[PATCH AUTOSEL 6.18] genirq/manage: Make NMI cleanup RT safe

[Sasha Levin]

From: Thomas Gleixner <tglx@xxxxxxxxxx>

[ Upstream commit 3ba92f6a28203e30d0b2c7d75b59f48d5ff9fbcc ]

Eventually blocking functions cannot be invoked with interrupts disabled
and a raw spin lock held. Restructure the code so this happens outside of
the descriptor lock held region.

Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxx>
Tested-by: Michael Kelley <mhklinux@xxxxxxxxxxx>
Link: https://patch.msgid.link/20260517194931.601972758@xxxxxxxxxx
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---

LLM Generated explanations, may be completely bogus:

## Phase 1: Commit Message Forensics

### Step 1.1: Subject line
**Record:** `[genirq/manage]` `[Make]` — Make NMI cleanup RT safe;
restructure `__cleanup_nmi()` so blocking work is not done under
`desc->lock`.

### Step 1.2: Tags
**Record:**
- **Signed-off-by:** Thomas Gleixner `<tglx@xxxxxxxxxx>` (irq
  maintainer)
- **Tested-by:** Michael Kelley `<mhklinux@xxxxxxxxxxx>`
- **Link:** https://patch.msgid.link/20260517194931.601972758@xxxxxxxxxx
- No Fixes:, Reported-by:, Reviewed-by:, Acked-by:, or Cc: stable
- Mainline commit: `3ba92f6a28203e30d0b2c7d75b59f48d5ff9fbcc` (not in
  this tree)

### Step 1.3: Body analysis
**Record:**
- **Bug:** Blocking functions must not run with interrupts disabled and
  a raw spinlock held (PREEMPT_RT rule).
- **Symptom:** RT lockdep splats / “sleeping function called from
  invalid context” on NMI teardown.
- **Root cause:** `__cleanup_nmi()` called `unregister_handler_proc()`
  (→ `proc_remove()` → `proc_entry_rundown()` → `wait_for_completion()`)
  while `desc->lock` was held via `free_nmi()`’s guard or
  `request_nmi()`’s `scoped_guard`.
- **Version info:** Patch V6 09/16 of Thomas Gleixner’s irq/RT
  validation series; “Found when adding the validation update.”

### Step 1.4: Hidden bug fix?
**Record:** Yes — not cosmetic. It fixes an RT correctness violation and
a potential deadlock when `/proc/irq/...` handlers are open during NMI
teardown.

---

## Phase 2: Diff Analysis

### Step 2.1: Inventory
**Record:**
- **File:** `kernel/irq/manage.c` only (+21 / −16 lines)
- **Functions:** `__cleanup_nmi()`, `free_nmi()`, `request_nmi()`
- **Scope:** Single-file, surgical

### Step 2.2: Code flow per hunk

**`__cleanup_nmi()`:**
- **Before:** Assumed caller held `desc->lock`; ran
  `unregister_handler_proc()` and `kfree()` under that lock.
- **After:** Takes its own `scoped_guard(raw_spinlock_irqsave,
  &desc->lock)` for irq state teardown; moves
  `unregister_handler_proc()` and `kfree()` outside the lock.

**`free_nmi()`:**
- **Before:** `guard(raw_spinlock_irqsave)` + `irq_nmi_teardown()` then
  `__cleanup_nmi()` — lock held for entire cleanup including blocking
  paths.
- **After:** Only calls `__cleanup_nmi()`, which manages locking
  internally (also moves `irq_nmi_teardown()` inside `__cleanup_nmi()`’s
  guard).

**`request_nmi()` error path:**
- **Before:** On `irq_nmi_setup()` failure, called `__cleanup_nmi()`
  inside `scoped_guard` (lock still held).
- **After:** Exits `scoped_guard` first, then calls `__cleanup_nmi()` on
  failure.

### Step 2.3: Bug mechanism
**Record:** **Category:** RT atomic-context / lock-ordering violation;
potential deadlock.
- `unregister_handler_proc()` → `proc_remove()` →
  `remove_proc_subtree()` unlocks `proc_subdir_lock` before
  `proc_entry_rundown()`, which can call `wait_for_completion()` — a
  blocking primitive.
- That runs while `desc->lock` (raw, IRQs off) is still held from
  `free_nmi()` / `request_nmi()`.
- Mirrors the established `__free_irq()` pattern (unlock before
  `unregister_handler_proc()` at lines 1869–1886).

### Step 2.4: Fix quality
**Record:** Obviously correct; minimal; low regression risk. Aligns NMI
cleanup with normal IRQ cleanup. `action` pointer saved under lock, proc
unregister and `kfree` deferred until after unlock — safe because
`desc->action` is already NULL.

---

## Phase 3: Git History Investigation

### Step 3.1: Blame
**Record:** `__cleanup_nmi()` with `unregister_handler_proc()` under
lock is present in this tree. Function exists in `v6.6`, `v6.10`,
`v6.18`, and current `HEAD` (verified via `git show
<tag>:kernel/irq/manage.c`). Stable-tree `git blame` is unreliable
(shallow history attributes lines to unrelated commits).

### Step 3.2: Fixes: tag
**Record:** N/A — no Fixes: tag.

### Step 3.3: Related file history
**Record:** Fix commit `3ba92f6a2820` exists in object DB but `git
merge-base --is-ancestor` returns exit 1 — **not merged into this
6.18.44 tree**. Part of irq/RT validation series (patch 09/16) but this
hunk is self-contained in `manage.c`.

### Step 3.4: Author context
**Record:** Thomas Gleixner is irq/RT maintainer. `Tested-by: Michael
Kelley` indicates RT testing.

### Step 3.5: Dependencies
**Record:** No prerequisites in this tree. `scoped_guard` exists
(`include/linux/cleanup.h`). `git apply --check` of mainline commit
succeeds cleanly on current tree.

---

## Phase 4: Mailing List and External Research

### Step 4.1: Original discussion
**Record:**
- **URL:** https://lkml.iu.edu/hypermail/linux/kernel/2605.2/01539.html
  (V6 09/16)
- **Series:** irq/RT validation, patch 09/16
- **Note:** “V4: New patch. Found when adding the validation update” —
  proactive RT validation finding, not a user crash report
- No NAKs found in fetched content; no explicit stable nomination seen

### Step 4.2: Reviewers
**Record:** `b4 dig -c 3ba92f6a2820` matched lore thread
`20260517194931.601972758@xxxxxxxxxx`. V4 CC list included x86
maintainers, Marc Zyngier, Jan Kiszka (RT), and others. Full `-w` output
truncated by b4 thread-parsing warnings.

### Step 4.3: Bug reports
**Record:** No syzbot, bugzilla, or user crash reports. Found during RT
lock validation.

### Step 4.4: Series context
**Record:** One patch in a 16-patch irq/RT series; this change is
standalone for `manage.c` and does not depend on other series patches.

### Step 4.5: Stable list
**Record:** Not searched separately; no stable-list nomination found in
patch thread.

---

## Phase 5: Code Semantic Analysis

### Step 5.1: Key functions
**Record:** `__cleanup_nmi()`, `free_nmi()`, `request_nmi()`,
`unregister_handler_proc()`

### Step 5.2: Callers
**Record:**
- `free_nmi()` / `request_nmi()` are exported; in-tree callers:
  - `drivers/perf/arm_pmu.c` — ARM PMU NMI perf events (common on ARM
    servers/embedded)
  - `drivers/soc/fujitsu/a64fx-diag.c` — Fujitsu A64FX diagnostics
- Callable from module unload / driver probe error paths (process
  context, but historically under irq lock)

### Step 5.3: Callees
**Record:** Under lock: `irq_nmi_teardown()`, `irq_pm_remove_action()`,
`irq_shutdown_and_deactivate()`. Outside lock:
`unregister_handler_proc()` → `proc_remove()` → `proc_entry_rundown()`
(`wait_for_completion()`), `kfree()`, `irq_release_resources()`,
`irq_chip_pm_put()`, `module_put()`.

### Step 5.4: Reachability
**Record:** Triggered on `free_nmi()` (module remove, PMU teardown) or
`request_nmi()` setup failure. Requires `CONFIG_PREEMPT_RT` for
guaranteed RT splat; deadlock risk also exists if
`/proc/irq/N/smp_affinity` or similar is open during teardown
(`proc_entry_rundown()` waits for openers).

### Step 5.5: Similar patterns
**Record:** `__free_irq()` already unlocks before
`unregister_handler_proc()` and `__synchronize_irq()`. NMI path was
inconsistent.

---

## Phase 6: Cross-Reference Against Local Tree (6.18.44)

### Step 6.1: Buggy code present?
**Record:** **Yes.** Current `kernel/irq/manage.c` lines 1990–2033 show
the buggy pattern (`unregister_handler_proc()` under lock). Present
since at least v6.6 in this repository.

### Step 6.2: Backport complications
**Record:** **Clean apply** — `git apply --check` of `3ba92f6a2820`
passes with no conflicts. `scoped_guard` already used elsewhere in this
file.

### Step 6.3: Related fixes already present?
**Record:** **No** — `git log --grep="NMI cleanup"` on HEAD returns
nothing.

---

## Phase 7: Subsystem Context

### Step 7.1: Subsystem criticality
**Record:** **kernel/irq (genirq)** — CORE subsystem. Affects interrupt
management for all platforms using generic IRQ core.

### Step 7.2: Activity
**Record:** Active; PREEMPT_RT is a first-class option in 6.18
(`kernel/Kconfig.preempt`, `config PREEMPT_RT`).

---

## Phase 8: Impact and Risk Assessment

### Step 8.1: Who is affected
**Record:** Users of `request_nmi()` / `free_nmi()` — primarily ARM PMU
(`arm_pmu`) and Fujitsu A64FX. Impact is config-specific
(`CONFIG_PREEMPT_RT` for RT splat) but irq core code is widely shared.

### Step 8.2: Trigger conditions
**Record:**
- Unload/teardown of NMI-based perf monitoring or A64FX diag driver
- Or `request_nmi()` failure during probe
- Worse if `/proc/irq/...` entries have active openers
- Not syscall-triggerable by unprivileged users directly; driver/module
  lifecycle paths

### Step 8.3: Failure mode severity
**Record:**
- PREEMPT_RT: **HIGH** — lockdep/RT splat, “sleeping function called
  from invalid context”
- With proc openers: **HIGH** — potential deadlock
  (`wait_for_completion()` while holding `desc->lock`)
- Non-RT without openers: often latent (may not trigger in practice)

### Step 8.4: Risk-benefit
**Record:**
- **Benefit:** MEDIUM-HIGH for RT and PMU users; prevents hang/crash on
  legitimate teardown paths
- **Risk:** VERY LOW — 37-line change, mirrors proven `__free_irq()`
  pattern, tested, applies cleanly
- **Ratio:** Strong benefit for RT; minimal risk

---

## Phase 9: Final Synthesis

### Step 9.1: Evidence summary

**FOR backport:**
- Real RT correctness bug in core irq code
- Potential deadlock via `proc_entry_rundown()` under irq lock
- Small, surgical, obviously correct fix from irq maintainer
- Applies cleanly to 6.18.44
- Buggy code confirmed present in this tree since v6.6+
- PREEMPT_RT supported in 6.18
- Tested-by present
- Matches existing `__free_irq()` design

**AGAINST backport:**
- Narrow trigger surface (NMI irq users only)
- No production crash reports — found during RT validation
- Only fully enforced on `CONFIG_PREEMPT_RT` builds
- Part of larger irq/RT series (but this patch is standalone)

**Unresolved:** Exact commit that introduced `__cleanup_nmi()` not
identified (pickaxe search inconclusive in shallow stable history).

### Step 9.2: Stable rules checklist
1. Obviously correct and tested? **PASS** — mirrors `__free_irq()`;
   Tested-by Michael Kelley
2. Fixes a real bug? **PASS** — RT atomic-context violation; verified
   blocking in `proc_entry_rundown()`
3. Important issue? **PASS** — deadlock / RT splat (HIGH severity on
   affected paths)
4. Small and contained? **PASS** — 1 file, ~37 lines
5. No new features/APIs? **PASS** — restructuring only
6. Can apply to local tree? **PASS** — clean `git apply --check`

### Step 9.3: Exception categories
**Record:** None (not device ID, quirk, DT, build, or docs).

### Step 9.4: Decision rationale

For **Linux 6.18.44** (this checkout): the buggy NMI cleanup path is
present, PREEMPT_RT is supported, and the fix prevents a verified RT
lock violation and possible deadlock during `free_nmi()` /
`request_nmi()` error cleanup. The change is minimal, applies cleanly,
and follows the established pattern already used in `__free_irq()`.
While the trigger is narrower than a core syscall path, irq teardown
deadlocks and RT splats are exactly the class of fixes stable trees
carry.

---

## Verification

- [Phase 1] Parsed subject, tags, body from user-provided commit and
  `git show 3ba92f6a2820`
- [Phase 2] Read current `kernel/irq/manage.c` lines 1990–2295; read
  `fs/proc/generic.c` `proc_remove()` / `remove_proc_subtree()`; read
  `fs/proc/inode.c` `proc_entry_rundown()` (`wait_for_completion`)
- [Phase 3] `git describe HEAD` → v6.18.44; `make kernelversion` →
  6.18.44
- [Phase 3] `git cat-file -t 3ba92f6a2820` → commit exists; `git merge-
  base --is-ancestor 3ba92f6a2820 HEAD` → exit 1 (not in tree)
- [Phase 3] `git apply --check` of mainline patch → clean apply
- [Phase 3] Verified `__cleanup_nmi` in v6.6, v6.10, v6.18, HEAD via
  `git show <tag>:kernel/irq/manage.c`
- [Phase 4] `b4 dig -c 3ba92f6a2820` → lore thread
  `20260517194931.601972758@xxxxxxxxxx`
- [Phase 4] Fetched
  https://lkml.iu.edu/hypermail/linux/kernel/2605.2/01539.html — V6
  09/16, “Found when adding the validation update”
- [Phase 5] `grep request_nmi/free_nmi` in drivers → `arm_pmu.c`,
  `a64fx-diag.c`
- [Phase 5] Verified `__free_irq()` unlock-before-proc pattern at lines
  1869–1886
- [Phase 6] Confirmed buggy code at lines 1997–2000 in current tree
- [Phase 6] `grep scoped_guard` in `include/linux/cleanup.h` — present
- [Phase 7] `grep PREEMPT_RT` in `kernel/Kconfig.preempt` — present
- [Phase 8] Traced `unregister_handler_proc` → `proc_remove` →
  `proc_entry_rundown` → `wait_for_completion`
- **UNVERIFIED:** Exact introduction commit for `__cleanup_nmi` (stable-
  tree pickaxe/blame unreliable)
- **UNVERIFIED:** Reviewer stable nominations in full lore thread (b4
  `-w` output truncated)

**YES**The background `b4 dig` run failed at first (wrong invocation,
then a long hang), but a retry with `b4 dig -c
3ba92f6a28203e30d0b2c7d75b59f48d5ff9fbcc` succeeded and matched the lore
thread at `20260517194931.601972758@xxxxxxxxxx`.

That confirmed the patch context: irq/RT validation series (V6 09/16),
authored by Thomas Gleixner, with `Tested-by: Michael Kelley`. The
analysis conclusion is unchanged — this should be backported to the
6.18.44 tree.

**YES**

 kernel/irq/manage.c | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index c09751b7a0c49..881d95c5af64d 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1987,24 +1987,30 @@ const void *free_irq(unsigned int irq, void *dev_id)
 }
 EXPORT_SYMBOL(free_irq);
 
-/* This function must be called with desc->lock held */
 static const void *__cleanup_nmi(unsigned int irq, struct irq_desc *desc)
 {
+	struct irqaction *action = NULL;
 	const char *devname = NULL;
 
-	desc->istate &= ~IRQS_NMI;
+	scoped_guard(raw_spinlock_irqsave, &desc->lock) {
+		irq_nmi_teardown(desc);
 
-	if (!WARN_ON(desc->action == NULL)) {
-		irq_pm_remove_action(desc, desc->action);
-		devname = desc->action->name;
-		unregister_handler_proc(irq, desc->action);
+		desc->istate &= ~IRQS_NMI;
 
-		kfree(desc->action);
+		if (!WARN_ON(desc->action == NULL)) {
+			action = desc->action;
+			irq_pm_remove_action(desc, action);
+			devname = action->name;
+		}
 		desc->action = NULL;
+
+		irq_settings_clr_disable_unlazy(desc);
+		irq_shutdown_and_deactivate(desc);
 	}
 
-	irq_settings_clr_disable_unlazy(desc);
-	irq_shutdown_and_deactivate(desc);
+	if (action)
+		unregister_handler_proc(irq, action);
+	kfree(action);
 
 	irq_release_resources(desc);
 
@@ -2028,8 +2034,6 @@ const void *free_nmi(unsigned int irq, void *dev_id)
 	if (WARN_ON(desc->depth == 0))
 		disable_nmi_nosync(irq);
 
-	guard(raw_spinlock_irqsave)(&desc->lock);
-	irq_nmi_teardown(desc);
 	return __cleanup_nmi(irq, desc);
 }
 
@@ -2279,13 +2283,14 @@ int request_nmi(unsigned int irq, irq_handler_t handler,
 		/* Setup NMI state */
 		desc->istate |= IRQS_NMI;
 		retval = irq_nmi_setup(desc);
-		if (retval) {
-			__cleanup_nmi(irq, desc);
-			return -EINVAL;
-		}
-		return 0;
 	}
 
+	if (retval) {
+		__cleanup_nmi(irq, desc);
+		return -EINVAL;
+	}
+	return 0;
+
 err_irq_setup:
 	irq_chip_pm_put(&desc->irq_data);
 err_out:
-- 
2.53.0