Skip to content

FROMLIST: Revert "wifi: ath12k: add panic handler"#712

Open
MilanoPipo wants to merge 1 commit into
qualcomm-linux:qcom-6.18.yfrom
MilanoPipo:ath-pr1357
Open

FROMLIST: Revert "wifi: ath12k: add panic handler"#712
MilanoPipo wants to merge 1 commit into
qualcomm-linux:qcom-6.18.yfrom
MilanoPipo:ath-pr1357

Conversation

@MilanoPipo

Copy link
Copy Markdown
Contributor

This reverts commit 8090556.

Call trace:
rcu_note_context_switch+0x4c4/0x508 (P)
__schedule+0xbc/0x1204
schedule+0x34/0x110
schedule_timeout+0x84/0x11c
__mhi_device_get_sync+0x164/0x228 [mhi]
mhi_device_get_sync+0x1c/0x3c [mhi]
ath12k_wifi7_pci_bus_wake_up+0x20/0x2c [ath12k_wifi7] ath12k_pci_read32+0x58/0x350 [ath12k]
ath12k_pci_clear_dbg_registers+0x28/0xb8 [ath12k]
ath12k_pci_panic_handler+0x20/0x44 [ath12k] ath12k_core_panic_handler+0x28/0x3c [ath12k] notifier_call_chain+0x78/0x1c0
atomic_notifier_call_chain+0x3c/0x5c

ath12k_core_panic_handler() is invoked via atomic_notifier_call_chain(), which runs inside an RCU read-side critical section. The current code calls ath12k_pci_sw_reset() synchronously from this context, which eventually reaches mhi_device_get_sync() and schedule_timeout(), triggering a voluntary context switch within RCU.

Revert change "wifi: ath12k: add panic handler" to avoid this issue.

Tested-on: WLAN.HMT.1.1.c7-00108-QCAHMTSWPL_V1.0_V2.0_SILICONZ_UPSTREAM-3
Link: https://lore.kernel.org/all/20260612032332.2278338-1-yingying.tang@oss.qualcomm.com/
CRs-Fixed: 4390354

This reverts commit 8090556.

Call trace:
rcu_note_context_switch+0x4c4/0x508 (P)
__schedule+0xbc/0x1204
schedule+0x34/0x110
schedule_timeout+0x84/0x11c
__mhi_device_get_sync+0x164/0x228 [mhi]
mhi_device_get_sync+0x1c/0x3c [mhi]
ath12k_wifi7_pci_bus_wake_up+0x20/0x2c [ath12k_wifi7]
ath12k_pci_read32+0x58/0x350 [ath12k]
ath12k_pci_clear_dbg_registers+0x28/0xb8 [ath12k]
ath12k_pci_panic_handler+0x20/0x44 [ath12k] ath12k_core_panic_handler+0x28/0x3c [ath12k]
notifier_call_chain+0x78/0x1c0
atomic_notifier_call_chain+0x3c/0x5c

ath12k_core_panic_handler() is invoked via atomic_notifier_call_chain(),
which runs inside an RCU read-side critical section. The current code calls
ath12k_pci_sw_reset() synchronously from this context, which eventually
reaches mhi_device_get_sync() and schedule_timeout(), triggering a voluntary
context switch within RCU.

Revert change "wifi: ath12k: add panic handler" to avoid this issue.

Tested-on: WLAN.HMT.1.1.c7-00108-QCAHMTSWPL_V1.0_V2.0_SILICONZ_UPSTREAM-3
Link: https://lore.kernel.org/all/20260612032332.2278338-1-yingying.tang@oss.qualcomm.com/
Signed-off-by: Yingying Tang <yingying.tang@oss.qualcomm.com>
@qlijarvis

Copy link
Copy Markdown

PR #712 — validate-patch

PR: #712

Verdict Issues Detailed Report
5 Full report

Final Summary

  1. Lore link present: Yes — Link: https://lore.kernel.org/all/20260612032332.2278338-1-yingying.tang@oss.qualcomm.com/
  2. Lore link matches PR commits: Cannot verify — network access restricted; unable to fetch upstream patch for byte-level comparison
  3. Upstream patch status: In review — FROMLIST prefix indicates patch has been submitted to mailing list but not yet merged into mainline
  4. PR present in qcom-next: Not checked — no kernel tree available per task constraints

Note: Due to network restrictions, I could not fetch the upstream patch from lore.kernel.org to perform byte-level diff comparison. However, the patch structure, commit message quality, authorship, and diff correctness are all excellent. The FROMLIST prefix and lore link indicate proper upstream submission workflow.

Verdict: ✅ — click to expand

🔍 Patch Validation

PR: #712
Title: FROMLIST: Revert "wifi: ath12k: add panic handler"
Upstream commit: https://lore.kernel.org/all/20260612032332.2278338-1-yingying.tang@oss.qualcomm.com/
Verdict: ✅ PASS (with network limitation caveat)

Commit Message

Check Status Note
Subject matches upstream FROMLIST prefix indicates pending upstream submission; subject follows kernel revert convention
Body preserves rationale Includes call trace, RCU context explanation, and clear problem statement
Fixes tag present/correct N/A Revert commits typically don't use Fixes: tag; uses "This reverts commit" instead
Authorship preserved Author: Yingying Tang yingying.tang@oss.qualcomm.com
Backport note (if applicable) N/A FROMLIST indicates this is being submitted upstream, not a backport

Diff

File Status Notes
drivers/net/wireless/ath/ath12k/core.c Removes panic handler registration/unregistration functions and call sites; cleans up error path
drivers/net/wireless/ath/ath12k/core.h Removes panic_notifier.h include and panic_nb field from ath12k_base struct
drivers/net/wireless/ath/ath12k/hif.h Removes panic_handler callback from hif_ops and ath12k_hif_panic_handler() wrapper
drivers/net/wireless/ath/ath12k/pci.c Removes ath12k_pci_panic_handler() implementation and callback registration

Issues

None detected. The patch is well-formed:

  • Revert structure: Correctly identifies the reverted commit SHA (8090556)
  • Commit message quality: Provides detailed technical rationale with call trace showing the RCU context violation
  • Diff completeness: Cleanly removes all panic handler infrastructure across 4 files (52 deletions, 1 insertion)
  • Error path cleanup: Properly simplifies error handling in ath12k_core_init() by removing the now-unnecessary err_unregister_notifier label
  • Metadata: Includes Tested-on tag, Link to lore, and proper Signed-off-by

Verdict

Merge as-is. This is a well-crafted revert patch that addresses a legitimate RCU context violation bug. The commit message clearly explains the problem (panic handler calling schedule_timeout() from atomic context), includes a full call trace for debugging, and follows kernel revert conventions.

Final Summary

  1. Lore link present: Yes — Link: https://lore.kernel.org/all/20260612032332.2278338-1-yingying.tang@oss.qualcomm.com/
  2. Lore link matches PR commits: Cannot verify — network access restricted; unable to fetch upstream patch for byte-level comparison
  3. Upstream patch status: In review — FROMLIST prefix indicates patch has been submitted to mailing list but not yet merged into mainline
  4. PR present in qcom-next: Not checked — no kernel tree available per task constraints

Note: Due to network restrictions, I could not fetch the upstream patch from lore.kernel.org to perform byte-level diff comparison. However, the patch structure, commit message quality, authorship, and diff correctness are all excellent. The FROMLIST prefix and lore link indicate proper upstream submission workflow.

@qlijarvis

Copy link
Copy Markdown

PR #712 — checker-log-analyzer

PR: #712
Checker run: https://github.com/qualcomm-linux/kernel-config/actions/runs/27533594265

Checker Result Summary
Checker Result Summary
checkpatch 1 warning: commit message line exceeds 75 chars
dt-binding-check ⏭️ No DT binding changes
dtb-check ⏭️ No devicetree changes
sparse-check Passed
check-uapi-headers No UAPI header changes
check-patch-compliance Passed
tag-check FROMLIST: prefix present
qcom-next-check FROMLIST commit with Link tag

Detailed report: Full report

Checker analysis — click to expand

🤖 CI Checker Analysis (checker-log-analyzer)

PR: #712 - FROMLIST: Revert "wifi: ath12k: add panic handler"
Source: https://github.com/qualcomm-linux/kernel-config/actions/runs/27533594265

Checker Result Summary
checkpatch 1 warning: commit message line exceeds 75 chars
dt-binding-check ⏭️ No DT binding changes
dtb-check ⏭️ No devicetree changes
sparse-check Passed
check-uapi-headers No UAPI header changes
check-patch-compliance Passed
tag-check FROMLIST: prefix present
qcom-next-check FROMLIST commit with Link tag

❌ checkpatch

Root cause: Line 18 of the commit message contains a call trace entry that is 88 characters long, exceeding checkpatch's preferred 75-character limit for commit description lines.

Failure details:

WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
#18: 
ath12k_pci_panic_handler+0x20/0x44 [ath12k] ath12k_core_panic_handler+0x28/0x3c [ath12k]

8c13eed524f96867fcb2c5c8abbafcb8ba2208fe total: 0 errors, 1 warnings, 0 checks, 119 lines checked

The problematic line in the commit message:

ath12k_pci_panic_handler+0x20/0x44 [ath12k] ath12k_core_panic_handler+0x28/0x3c [ath12k]

Fix:

This is a cosmetic warning that can be addressed by wrapping the call trace line. The line contains two function calls that should be split:

Option 1 (Recommended): Split the line into two separate lines:

ath12k_pci_panic_handler+0x20/0x44 [ath12k]
ath12k_core_panic_handler+0x28/0x3c [ath12k]

Option 2: Accept the warning as-is. Call traces often contain long symbol names, and maintainers typically accept this as a valid exception since breaking the line could reduce readability of the stack trace.

Reproduce locally:

./scripts/checkpatch.pl --strict --summary-file --ignore FILE_PATH_CHANGES --git 9740e45b352ca1e5d7f40f5754b977d3533dc8b4..8c13eed524f96867fcb2c5c8abbafcb8ba2208fe

Verdict

1 non-blocking cosmetic warning. The checkpatch warning is about commit message formatting in a call trace. This is commonly accepted by maintainers since call traces naturally contain long function names. The patch is functionally correct and all other checkers passed. The PR can proceed to merge, though the author may optionally reformat the call trace line for cleaner checkpatch output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants