mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3] softirq: Preserve interrupt context during IRQ exit
@ 2026-09-24  4:15 Karl Mehltretter
  2026-09-24 16:34 ` Bradley Morgan
  2026-09-25  7:56 ` Sebastian Andrzej Siewior
  0 siblings, 2 replies; 3+ messages in thread
From: Karl Mehltretter @ 2026-09-24  4:15 UTC (permalink / raw)
  To: Peter Zijlstra, Thomas Gleixner
  Cc: Karl Mehltretter, Sebastian Andrzej Siewior, Frederic Weisbecker,
	Clark Williams, Steven Rostedt, Boqun Feng, Lyude Paul,
	Joel Fernandes, Alexander Potapenko, Marco Elver,
	Jonathan Corbet, linux-doc, linux-kernel, linux-rt-devel

__irq_exit_rcu() removes HARDIRQ_OFFSET first and then runs
hrtimer_rearm_deferred(), invoke_softirq() and wake_timersd(). This is
interrupt exit work with interrupts disabled, but preempt_count already
describes the interrupted task again. Everything which derives the
context from preempt_count gets it wrong in that window:

  - ftrace, perf and the ring buffer record task context and use the
    task recursion and context slots.
  - KCSAN attributes the accesses to the interrupted task, KMSAN uses
    and changes its state. KCOV and the printk caller id see a task.
  - On PREEMPT_RT can_spin_trylock() and local_trylock() reject hard
    interrupt context to avoid interfering with PI when the interrupted
    task is blocked on a lock. That check does not reject calls made in
    this window. BPF programs attached to sched_waking or sched_wakeup
    can reach it through kmalloc_nolock().
  - An oops kills the interrupted task instead of ending in "Fatal
    exception in interrupt".

Tracing and the sanitizers see the wrong context in this window. No
failure caused by this misclassification is known. The early removal of
HARDIRQ_OFFSET predates git. lockdep is not affected because
lockdep_hardirq_exit() is the last operation in irq_exit().

Keep HARDIRQ_OFFSET until right before tick_irq_exit(), which needs
in_hardirq() to be false for the outermost interrupt. Softirq handlers
must not run with HARDIRQ_OFFSET set, so softirq_handle_begin() replaces
it with SOFTIRQ_OFFSET and softirq_handle_end() reverts that, each in a
single raw preempt_count update. The raw operations keep the preemption
disable location recorded by irq_enter_rcu(), and lockdep is updated by
hand. softirq_handle_begin() detects the case with
in_hardirq() because __do_softirq() is reached through the stack switch
in do_softirq_own_stack() and cannot take an argument.

The checks run before HARDIRQ_OFFSET is removed. !in_interrupt() becomes
irq_count() == HARDIRQ_OFFSET, as in irq_enter_rcu(). The timer thread
check becomes !in_nmi() && hardirq_count() == HARDIRQ_OFFSET. It does
not test softirq_count(): the timer thread must also wake when the
interrupt hit softirq processing or a section with BHs disabled.
A softirq raised in the timer thread wakeup is handled by the timer
thread, which handles all pending softirqs.

A softirq raised in the final preempt_count_sub(), e.g. by a consumer of
the preempt_enable tracepoint, misses these checks. Wake ksoftirqd for
it as raise_softirq_irqoff() would. Otherwise it waits for the next
interrupt exit and a cpuidle driver reports it as pending at idle entry.

The number of preempt_count updates and the interrupt time accounting
are unchanged. The preemptoff tracer now reports the interrupt and the
softirq processing on top of it as one section, and function graph with
nofuncgraph-irqs also skips the interrupt exit work, including the
__do_softirq() frame.

Suggested-by: Peter Zijlstra <peterz@infradead.org>
Link: https://lore.kernel.org/r/20260813130826.GW687043@noisy.programming.kicks-ass.net
Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---

Notes:
    Thanks for the review, Sebastian.
    
    v3 also adds a pending softirq check after the final
    preempt_count_sub(). A preempt_enable tracepoint callback can raise a
    softirq there, after the softirq and timer thread checks. With an RCU
    reader in that callback I saw 10 NOHZ tick-stop warnings per boot in
    QEMU without the new check, and none on the base kernel or with the
    check. The check wakes ksoftirqd for newly pending softirqs. Does this
    case justify the extra check on every IRQ exit?
    
    Changes in v3:
    - Operands swapped, no casts, one irq_count() check per direction,
      lockdep_softirqs_off() before it (Sebastian).
    - Timer thread check without IRQ_EXIT_TIMERS, order of the exit work
      kept (Sebastian).
    - from_hardirq renamed to from_irq_exit, comment as Sebastian suggested.
    - ksoftirqd wakeup for a softirq raised in the final
      preempt_count_sub(), see above.
    - Documentation/core-api/entry.rst updated.
    - Rebased on tip/master c81f6d2398d0.
    
    I am not proposing this for stable.
    
    Testing: base against v3 in QEMU on x86-64 in separate configurations
    (non-RT, threadirqs, RT, KCSAN, KMSAN, rcutorture), on arm32, arm64,
    ppc64, s390x, parisc, sparc64, riscv64, loongarch64 and m68k, and on a
    SAM9X75 (also RT), a Raspberry Pi 500+ and a Raspberry Pi 400. No
    regressions observed in these runs.
    
    v2: https://lore.kernel.org/r/20260905023210.82853-1-kmehltretter@gmail.com

 Documentation/core-api/entry.rst | 16 ++++---
 kernel/softirq.c                 | 79 +++++++++++++++++++++++++++-----
 2 files changed, 77 insertions(+), 18 deletions(-)

diff --git a/Documentation/core-api/entry.rst b/Documentation/core-api/entry.rst
index 79fdaed954d9d..ff3df997b151f 100644
--- a/Documentation/core-api/entry.rst
+++ b/Documentation/core-api/entry.rst
@@ -197,8 +197,9 @@ return true, handles NOHZ tick state and interrupt time accounting. This
 means that up to the point where irq_enter_rcu() is invoked in_hardirq()
 returns false.
 
-irq_exit_rcu() handles interrupt time accounting, undoes the preemption
-count update and eventually handles soft interrupts and NOHZ tick state.
+irq_exit_rcu() handles interrupt time accounting, handles soft interrupts if
+possible, undoes the preemption count update and finally handles the NOHZ tick
+state.
 
 In theory, the preemption count could be updated in irqentry_enter(). In
 practice, deferring this update to irq_enter_rcu() allows the preemption-count
@@ -207,10 +208,13 @@ irqentry_exit(), which are described in the next paragraph. The only downside
 is that the early entry code up to irq_enter_rcu() must be aware that the
 preemption count has not yet been updated with the HARDIRQ_OFFSET state.
 
-Note that irq_exit_rcu() must remove HARDIRQ_OFFSET from the preemption count
-before it handles soft interrupts, whose handlers must run in BH context rather
-than irq-disabled context. In addition, irqentry_exit() might schedule, which
-also requires that HARDIRQ_OFFSET has been removed from the preemption count.
+Note that soft interrupt handlers must run in BH context rather than in hard
+interrupt context. irq_exit_rcu() therefore replaces HARDIRQ_OFFSET with
+SOFTIRQ_OFFSET in the preemption count while it handles soft interrupts and
+puts HARDIRQ_OFFSET back afterwards, so that the remaining interrupt exit work
+is still attributed to the interrupt. HARDIRQ_OFFSET is removed before
+irq_exit_rcu() returns because irqentry_exit() might schedule, which requires
+that HARDIRQ_OFFSET has been removed from the preemption count.
 
 Even though interrupt handlers are expected to run with local interrupts
 disabled, interrupt nesting is common from an entry/exit perspective. For
diff --git a/kernel/softirq.c b/kernel/softirq.c
index c3729c5b284b0..efa6707edc0e9 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -350,8 +350,8 @@ static inline void ksoftirqd_run_end(void)
 	local_irq_enable();
 }
 
-static inline void softirq_handle_begin(void) { }
-static inline void softirq_handle_end(void) { }
+static inline bool softirq_handle_begin(void) { return false; }
+static inline void softirq_handle_end(bool from_irq_exit) { }
 
 static inline bool should_wake_ksoftirqd(void)
 {
@@ -481,15 +481,40 @@ void __local_bh_enable_ip(unsigned long ip, unsigned int cnt)
 }
 EXPORT_SYMBOL(__local_bh_enable_ip);
 
-static inline void softirq_handle_begin(void)
+static inline bool softirq_handle_begin(void)
 {
-	__local_bh_disable_ip(_RET_IP_, SOFTIRQ_OFFSET);
+	bool from_irq_exit = in_hardirq();
+
+	if (!from_irq_exit) {
+		__local_bh_disable_ip(_RET_IP_, SOFTIRQ_OFFSET);
+		return false;
+	}
+
+	/*
+	 * Only reached from irq_exit(), with HARDIRQ_OFFSET still set.
+	 * Replace it with SOFTIRQ_OFFSET before handle_softirqs() enables
+	 * interrupts. Use the raw operation to preserve the preemption
+	 * disable location recorded by irq_enter_rcu(), and update lockdep
+	 * directly.
+	 */
+	__preempt_count_sub(HARDIRQ_OFFSET - SOFTIRQ_OFFSET);
+	lockdep_softirqs_off(_RET_IP_);
+	WARN_ON_ONCE(irq_count() != SOFTIRQ_OFFSET);
+
+	return true;
 }
 
-static inline void softirq_handle_end(void)
+static inline void softirq_handle_end(bool from_irq_exit)
 {
-	__local_bh_enable(SOFTIRQ_OFFSET);
-	WARN_ON_ONCE(in_interrupt());
+	if (!from_irq_exit) {
+		__local_bh_enable(SOFTIRQ_OFFSET);
+		WARN_ON_ONCE(in_interrupt());
+		return;
+	}
+
+	lockdep_softirqs_on(_RET_IP_);
+	__preempt_count_add(HARDIRQ_OFFSET - SOFTIRQ_OFFSET);
+	WARN_ON_ONCE(irq_count() != HARDIRQ_OFFSET);
 }
 
 static inline void ksoftirqd_run_begin(void)
@@ -605,6 +630,7 @@ static void handle_softirqs(bool ksirqd)
 	unsigned long old_flags = current->flags;
 	int max_restart = MAX_SOFTIRQ_RESTART;
 	struct softirq_action *h;
+	bool from_irq_exit;
 	bool in_hardirq;
 	__u32 pending;
 	int softirq_bit;
@@ -618,7 +644,7 @@ static void handle_softirqs(bool ksirqd)
 
 	pending = local_softirq_pending();
 
-	softirq_handle_begin();
+	from_irq_exit = softirq_handle_begin();
 	in_hardirq = lockdep_softirq_start();
 	account_softirq_enter(current);
 
@@ -670,7 +696,7 @@ static void handle_softirqs(bool ksirqd)
 
 	account_softirq_exit(current);
 	lockdep_softirq_end(in_hardirq);
-	softirq_handle_end();
+	softirq_handle_end(from_irq_exit);
 	current_restore_flags(old_flags, PF_MEMALLOC);
 }
 
@@ -742,14 +768,20 @@ static inline void wake_timersd(void) { }
 
 static inline void __irq_exit_rcu(void)
 {
+	u32 pending;
+
 #ifndef __ARCH_IRQ_EXIT_IRQS_DISABLED
 	local_irq_disable();
 #else
 	lockdep_assert_irqs_disabled();
 #endif
 	account_hardirq_exit(current);
-	preempt_count_sub(HARDIRQ_OFFSET);
-	if (!in_interrupt() && local_softirq_pending()) {
+
+	/*
+	 * HARDIRQ_OFFSET is still set. Only the outermost interrupt handles
+	 * softirqs, and only if it did not hit a softirq or BH disabled section.
+	 */
+	if (irq_count() == HARDIRQ_OFFSET && local_softirq_pending()) {
 		/*
 		 * If we left hrtimers unarmed, make sure to arm them now,
 		 * before enabling interrupts to run softirq.
@@ -758,10 +790,33 @@ static inline void __irq_exit_rcu(void)
 		invoke_softirq();
 	}
 
+	/*
+	 * Wake the timer thread even if the interrupt hit a softirq or a
+	 * section with BHs disabled. Only nested interrupts and NMIs are
+	 * excluded.
+	 */
 	if (IS_ENABLED(CONFIG_IRQ_FORCED_THREADING) && force_irqthreads() &&
-	    local_timers_pending_force_th() && !(in_nmi() | in_hardirq()))
+	    local_timers_pending_force_th() &&
+	    !in_nmi() && hardirq_count() == HARDIRQ_OFFSET)
 		wake_timersd();
 
+	pending = local_softirq_pending();
+
+	/*
+	 * tick_irq_exit() relies on in_hardirq() being false for the
+	 * outermost interrupt.
+	 */
+	preempt_count_sub(HARDIRQ_OFFSET);
+
+	/*
+	 * A softirq raised in preempt_count_sub(), e.g. by a tracepoint,
+	 * missed the checks above. Wake ksoftirqd as raise_softirq_irqoff()
+	 * would have done.
+	 */
+	if (unlikely(local_softirq_pending() & ~pending) && !in_interrupt() &&
+	    should_wake_ksoftirqd())
+		wakeup_softirqd();
+
 	tick_irq_exit();
 }
 

base-commit: c81f6d2398d063009cc9ad2c98f126daaa7669e2
-- 
2.53.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] softirq: Preserve interrupt context during IRQ exit
  2026-09-24  4:15 [PATCH v3] softirq: Preserve interrupt context during IRQ exit Karl Mehltretter
@ 2026-09-24 16:34 ` Bradley Morgan
  2026-09-25  7:56 ` Sebastian Andrzej Siewior
  1 sibling, 0 replies; 3+ messages in thread
From: Bradley Morgan @ 2026-09-24 16:34 UTC (permalink / raw)
  To: kmehltretter
  Cc: bigeasy, boqun, clrkwllms, corbet, elver, frederic, glider,
	joelagnelf, linux-doc, linux-kernel, linux-rt-devel, lyude,
	peterz, rostedt, tglx

On 24 September 2026 05:15:38 BST, Karl Mehltretter
<kmehltretter@gmail.com> wrote:
>__irq_exit_rcu() removes HARDIRQ_OFFSET first and then runs
>hrtimer_rearm_deferred(), invoke_softirq() and wake_timersd(). This is
>interrupt exit work with interrupts disabled, but preempt_count already
>describes the interrupted task again. Everything which derives the
>context from preempt_count gets it wrong in that window:
>
>  - ftrace, perf and the ring buffer record task context and use the
>    task recursion and context slots.
>  - KCSAN attributes the accesses to the interrupted task, KMSAN uses
>    and changes its state. KCOV and the printk caller id see a task.
>  - On PREEMPT_RT can_spin_trylock() and local_trylock() reject hard
>    interrupt context to avoid interfering with PI when the interrupted
>    task is blocked on a lock. That check does not reject calls made in
>    this window. BPF programs attached to sched_waking or sched_wakeup
>    can reach it through kmalloc_nolock().
>  - An oops kills the interrupted task instead of ending in "Fatal
>    exception in interrupt".
>
>Tracing and the sanitizers see the wrong context in this window. No
>failure caused by this misclassification is known. The early removal of
>HARDIRQ_OFFSET predates git. lockdep is not affected because
>lockdep_hardirq_exit() is the last operation in irq_exit().
>
>Keep HARDIRQ_OFFSET until right before tick_irq_exit(), which needs
>in_hardirq() to be false for the outermost interrupt. Softirq handlers
>must not run with HARDIRQ_OFFSET set, so softirq_handle_begin() replaces
>it with SOFTIRQ_OFFSET and softirq_handle_end() reverts that, each in a
>single raw preempt_count update. The raw operations keep the preemption
>disable location recorded by irq_enter_rcu(), and lockdep is updated by
>hand. softirq_handle_begin() detects the case with
>in_hardirq() because __do_softirq() is reached through the stack switch
>in do_softirq_own_stack() and cannot take an argument.
>
>The checks run before HARDIRQ_OFFSET is removed. !in_interrupt() becomes
>irq_count() == HARDIRQ_OFFSET, as in irq_enter_rcu(). The timer thread
>check becomes !in_nmi() && hardirq_count() == HARDIRQ_OFFSET. It does
>not test softirq_count(): the timer thread must also wake when the
>interrupt hit softirq processing or a section with BHs disabled.
>A softirq raised in the timer thread wakeup is handled by the timer
>thread, which handles all pending softirqs.
>
>A softirq raised in the final preempt_count_sub(), e.g. by a consumer of
>the preempt_enable tracepoint, misses these checks. Wake ksoftirqd for
>it as raise_softirq_irqoff() would. Otherwise it waits for the next
>interrupt exit and a cpuidle driver reports it as pending at idle entry.
>
>The number of preempt_count updates and the interrupt time accounting
>are unchanged. The preemptoff tracer now reports the interrupt and the
>softirq processing on top of it as one section, and function graph with
>nofuncgraph-irqs also skips the interrupt exit work, including the
>__do_softirq() frame.
>
>Suggested-by: Peter Zijlstra <peterz@infradead.org>
>Link: https://lore.kernel.org/r/20260813130826.GW687043@noisy.programming.kicks-ass.net
>Assisted-by: LLM
>Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
>---
>
>Notes:
>    Thanks for the review, Sebastian.
>    
>    v3 also adds a pending softirq check after the final
>    preempt_count_sub(). A preempt_enable tracepoint callback can raise a
>    softirq there, after the softirq and timer thread checks. With an RCU
>    reader in that callback I saw 10 NOHZ tick-stop warnings per boot in
>    QEMU without the new check, and none on the base kernel or with the
>    check. The check wakes ksoftirqd for newly pending softirqs. Does this
>    case justify the extra check on every IRQ exit?
>    
>    Changes in v3:
>    - Operands swapped, no casts, one irq_count() check per direction,
>      lockdep_softirqs_off() before it (Sebastian).
>    - Timer thread check without IRQ_EXIT_TIMERS, order of the exit work
>      kept (Sebastian).
>    - from_hardirq renamed to from_irq_exit, comment as Sebastian suggested.
>    - ksoftirqd wakeup for a softirq raised in the final
>      preempt_count_sub(), see above.
>    - Documentation/core-api/entry.rst updated.
>    - Rebased on tip/master c81f6d2398d0.
>    
>    I am not proposing this for stable.
>    
>    Testing: base against v3 in QEMU on x86-64 in separate configurations
>    (non-RT, threadirqs, RT, KCSAN, KMSAN, rcutorture), on arm32, arm64,
>    ppc64, s390x, parisc, sparc64, riscv64, loongarch64 and m68k, and on a
>    SAM9X75 (also RT), a Raspberry Pi 500+ and a Raspberry Pi 400. No
>    regressions observed in these runs.
>    
>    v2: https://lore.kernel.org/r/20260905023210.82853-1-kmehltretter@gmail.com
>
> Documentation/core-api/entry.rst | 16 ++++---
> kernel/softirq.c                 | 79 +++++++++++++++++++++++++++-----
> 2 files changed, 77 insertions(+), 18 deletions(-)
>
>diff --git a/Documentation/core-api/entry.rst b/Documentation/core-api/entry.rst
>index 79fdaed954d9d..ff3df997b151f 100644
>--- a/Documentation/core-api/entry.rst
>+++ b/Documentation/core-api/entry.rst
>@@ -197,8 +197,9 @@ return true, handles NOHZ tick state and interrupt time accounting. This
> means that up to the point where irq_enter_rcu() is invoked in_hardirq()
> returns false.
> 
>-irq_exit_rcu() handles interrupt time accounting, undoes the preemption
>-count update and eventually handles soft interrupts and NOHZ tick state.
>+irq_exit_rcu() handles interrupt time accounting, handles soft interrupts if
>+possible, undoes the preemption count update and finally handles the NOHZ tick
>+state.
> 
> In theory, the preemption count could be updated in irqentry_enter(). In
> practice, deferring this update to irq_enter_rcu() allows the
> preemption-count
>@@ -207,10 +208,13 @@ irqentry_exit(), which are described in the next paragraph. The only downside
> is that the early entry code up to irq_enter_rcu() must be aware that the
> preemption count has not yet been updated with the HARDIRQ_OFFSET state.
> 
>-Note that irq_exit_rcu() must remove HARDIRQ_OFFSET from the preemption count
>-before it handles soft interrupts, whose handlers must run in BH context rather
>-than irq-disabled context. In addition, irqentry_exit() might schedule, which
>-also requires that HARDIRQ_OFFSET has been removed from the preemption count.
>+Note that soft interrupt handlers must run in BH context rather than in hard
>+interrupt context. irq_exit_rcu() therefore replaces HARDIRQ_OFFSET with
>+SOFTIRQ_OFFSET in the preemption count while it handles soft interrupts and
>+puts HARDIRQ_OFFSET back afterwards, so that the remaining interrupt exit work
>+is still attributed to the interrupt. HARDIRQ_OFFSET is removed before
>+irq_exit_rcu() returns because irqentry_exit() might schedule, which requires
>+that HARDIRQ_OFFSET has been removed from the preemption count.
> 
> Even though interrupt handlers are expected to run with local interrupts
> disabled, interrupt nesting is common from an entry/exit perspective. For
>diff --git a/kernel/softirq.c b/kernel/softirq.c
>index c3729c5b284b0..efa6707edc0e9 100644
>--- a/kernel/softirq.c
>+++ b/kernel/softirq.c
>@@ -350,8 +350,8 @@ static inline void ksoftirqd_run_end(void)
> 	local_irq_enable();
> }
> 
>-static inline void softirq_handle_begin(void) { }
>-static inline void softirq_handle_end(void) { }
>+static inline bool softirq_handle_begin(void) { return false; }
>+static inline void softirq_handle_end(bool from_irq_exit) { }
> 
> static inline bool should_wake_ksoftirqd(void)
> {
>@@ -481,15 +481,40 @@ void __local_bh_enable_ip(unsigned long ip, unsigned int cnt)
> }
> EXPORT_SYMBOL(__local_bh_enable_ip);
> 
>-static inline void softirq_handle_begin(void)
>+static inline bool softirq_handle_begin(void)
> {
>-	__local_bh_disable_ip(_RET_IP_, SOFTIRQ_OFFSET);
>+	bool from_irq_exit = in_hardirq();
>+
>+	if (!from_irq_exit) {
>+		__local_bh_disable_ip(_RET_IP_, SOFTIRQ_OFFSET);
>+		return false;
>+	}
>+
>+	/*
>+	 * Only reached from irq_exit(), with HARDIRQ_OFFSET still set.
>+	 * Replace it with SOFTIRQ_OFFSET before handle_softirqs() enables
>+	 * interrupts. Use the raw operation to preserve the preemption
>+	 * disable location recorded by irq_enter_rcu(), and update lockdep
>+	 * directly.
>+	 */
>+	__preempt_count_sub(HARDIRQ_OFFSET - SOFTIRQ_OFFSET);
>+	lockdep_softirqs_off(_RET_IP_);
>+	WARN_ON_ONCE(irq_count() != SOFTIRQ_OFFSET);
>+
>+	return true;
> }
> 
>-static inline void softirq_handle_end(void)
>+static inline void softirq_handle_end(bool from_irq_exit)
> {
>-	__local_bh_enable(SOFTIRQ_OFFSET);
>-	WARN_ON_ONCE(in_interrupt());
>+	if (!from_irq_exit) {
>+		__local_bh_enable(SOFTIRQ_OFFSET);
>+		WARN_ON_ONCE(in_interrupt());
>+		return;
>+	}
>+
>+	lockdep_softirqs_on(_RET_IP_);
>+	__preempt_count_add(HARDIRQ_OFFSET - SOFTIRQ_OFFSET);
>+	WARN_ON_ONCE(irq_count() != HARDIRQ_OFFSET);
> }
> 
> static inline void ksoftirqd_run_begin(void)
>@@ -605,6 +630,7 @@ static void handle_softirqs(bool ksirqd)
> 	unsigned long old_flags = current->flags;
> 	int max_restart = MAX_SOFTIRQ_RESTART;
> 	struct softirq_action *h;
>+	bool from_irq_exit;
> 	bool in_hardirq;
> 	__u32 pending;
> 	int softirq_bit;
>@@ -618,7 +644,7 @@ static void handle_softirqs(bool ksirqd)
> 
> 	pending = local_softirq_pending();
> 
>-	softirq_handle_begin();
>+	from_irq_exit = softirq_handle_begin();
> 	in_hardirq = lockdep_softirq_start();
> 	account_softirq_enter(current);
> 
>@@ -670,7 +696,7 @@ static void handle_softirqs(bool ksirqd)
> 
> 	account_softirq_exit(current);
> 	lockdep_softirq_end(in_hardirq);
>-	softirq_handle_end();
>+	softirq_handle_end(from_irq_exit);
> 	current_restore_flags(old_flags, PF_MEMALLOC);
> }
> 
>@@ -742,14 +768,20 @@ static inline void wake_timersd(void) { }
> 
> static inline void __irq_exit_rcu(void)
> {
>+	u32 pending;
>+
> #ifndef __ARCH_IRQ_EXIT_IRQS_DISABLED
> 	local_irq_disable();
> #else
> 	lockdep_assert_irqs_disabled();
> #endif
> 	account_hardirq_exit(current);
>-	preempt_count_sub(HARDIRQ_OFFSET);
>-	if (!in_interrupt() && local_softirq_pending()) {
>+
>+	/*
>+	 * HARDIRQ_OFFSET is still set. Only the outermost interrupt handles
>+	 * softirqs, and only if it did not hit a softirq or BH disabled section.
>+	 */
>+	if (irq_count() == HARDIRQ_OFFSET && local_softirq_pending()) {
> 		/*
> 		 * If we left hrtimers unarmed, make sure to arm them now,
> 		 * before enabling interrupts to run softirq.
>@@ -758,10 +790,33 @@ static inline void __irq_exit_rcu(void)
> 		invoke_softirq();
> 	}
> 
>+	/*
>+	 * Wake the timer thread even if the interrupt hit a softirq or a
>+	 * section with BHs disabled. Only nested interrupts and NMIs are
>+	 * excluded.
>+	 */
> 	if (IS_ENABLED(CONFIG_IRQ_FORCED_THREADING) && force_irqthreads() &&
>-	    local_timers_pending_force_th() && !(in_nmi() | in_hardirq()))
>+	    local_timers_pending_force_th() &&
>+	    !in_nmi() && hardirq_count() == HARDIRQ_OFFSET)
> 		wake_timersd();
> 
>+	pending = local_softirq_pending();
>+
>+	/*
>+	 * tick_irq_exit() relies on in_hardirq() being false for the
>+	 * outermost interrupt.
>+	 */
>+	preempt_count_sub(HARDIRQ_OFFSET);
>+
>+	/*
>+	 * A softirq raised in preempt_count_sub(), e.g. by a tracepoint,
>+	 * missed the checks above. Wake ksoftirqd as raise_softirq_irqoff()
>+	 * would have done.
>+	 */
>+	if (unlikely(local_softirq_pending() & ~pending) && !in_interrupt() &&
>+	    should_wake_ksoftirqd())
>+		wakeup_softirqd();
>+
> 	tick_irq_exit();
> }
> 
>
>base-commit: c81f6d2398d063009cc9ad2c98f126daaa7669e2
>
Nice! LGTM, thanks

Reviewed-by: Bradley Morgan <brads@mainlining.org>


(Note this review is done by correctness!)
--- Thanks!
"I'm not a very positive person" - Linus torvalds

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] softirq: Preserve interrupt context during IRQ exit
  2026-09-24  4:15 [PATCH v3] softirq: Preserve interrupt context during IRQ exit Karl Mehltretter
  2026-09-24 16:34 ` Bradley Morgan
@ 2026-09-25  7:56 ` Sebastian Andrzej Siewior
  1 sibling, 0 replies; 3+ messages in thread
From: Sebastian Andrzej Siewior @ 2026-09-25  7:56 UTC (permalink / raw)
  To: Karl Mehltretter
  Cc: Peter Zijlstra, Thomas Gleixner, Frederic Weisbecker,
	Clark Williams, Steven Rostedt, Boqun Feng, Lyude Paul,
	Joel Fernandes, Alexander Potapenko, Marco Elver,
	Jonathan Corbet, linux-doc, linux-kernel, linux-rt-devel

On 2026-09-24 06:15:38 [+0200], Karl Mehltretter wrote:
> __irq_exit_rcu() removes HARDIRQ_OFFSET first and then runs
> hrtimer_rearm_deferred(), invoke_softirq() and wake_timersd(). This is
> interrupt exit work with interrupts disabled, but preempt_count already
> describes the interrupted task again. Everything which derives the
> context from preempt_count gets it wrong in that window:

On the return from interrupt path __irq_exit_rcu() removes
HARDIRQ_OFFSET from the preemption counter at the very top of the
function. Everything after that will get the current context reported as
TASK instead of HARDIRQ. Everything in that function, such as
invoke_softirq(), is aware of the situation and does not rely on the
counter. There are some exceptions:

>   - ftrace, perf and the ring buffer record task context and use the
…
> Notes:
>     v3 also adds a pending softirq check after the final
>     preempt_count_sub(). A preempt_enable tracepoint callback can raise a
>     softirq there, after the softirq and timer thread checks. With an RCU
>     reader in that callback I saw 10 NOHZ tick-stop warnings per boot in
>     QEMU without the new check, and none on the base kernel or with the
>     check. The check wakes ksoftirqd for newly pending softirqs. Does this
>     case justify the extra check on every IRQ exit?

No, I don't think so. This is not the normal path. I would rather prefer
of not being able to attach a tracepoint to preempt_count_sub() within
__irq_exit_rcu() than this. Also one can attach a tracepoint to
tick_irq_exit() leaving you with the exact same problem. So I would
rather have __irq_exit_rcu() in a section where tracepoints can not be
attached or have the consequences that someone did that. Or maybe have
tracepoints/BPF taught to not do this here.

We may legitimately leave invoke_softirq() with pending softirqs. So you
probably for that reason mask everything out and have a few other
conditions for that reason. Does not justify it.

…
>     - Rebased on tip/master c81f6d2398d0.

The hash here is not really helpful because tip/master is getting thrown
away. tip/master as of today, better yet the branch that it will be
probably applied to.

> --- a/Documentation/core-api/entry.rst
> +++ b/Documentation/core-api/entry.rst
> @@ -197,8 +197,9 @@ return true, handles NOHZ tick state and interrupt time accounting. This
>  means that up to the point where irq_enter_rcu() is invoked in_hardirq()
>  returns false.
>  
> -irq_exit_rcu() handles interrupt time accounting, undoes the preemption
> -count update and eventually handles soft interrupts and NOHZ tick state.
> +irq_exit_rcu() handles interrupt time accounting, handles soft interrupts if
> +possible, undoes the preemption count update and finally handles the NOHZ tick
> +state.

Changed the ordering to the code flow. Okay.

  irq_exit_rcu() handles interrupt time accounting, eventually handles
  soft interrupts, undoes the preemption count update and handles NOHZ
  tick state.

Or leave it.

>  In theory, the preemption count could be updated in irqentry_enter(). In
>  practice, deferring this update to irq_enter_rcu() allows the preemption-count
> @@ -207,10 +208,13 @@ irqentry_exit(), which are described in the next paragraph. The only downside
>  is that the early entry code up to irq_enter_rcu() must be aware that the
>  preemption count has not yet been updated with the HARDIRQ_OFFSET state.
>  
> -Note that irq_exit_rcu() must remove HARDIRQ_OFFSET from the preemption count
> -before it handles soft interrupts, whose handlers must run in BH context rather
> -than irq-disabled context. In addition, irqentry_exit() might schedule, which
> -also requires that HARDIRQ_OFFSET has been removed from the preemption count.

> +Note that soft interrupt handlers must run in BH context rather than in hard
> +interrupt context. irq_exit_rcu() therefore replaces HARDIRQ_OFFSET with
> +SOFTIRQ_OFFSET in the preemption count while it handles soft interrupts and
> +puts HARDIRQ_OFFSET back afterwards, so that the remaining interrupt exit work
> +is still attributed to the interrupt. HARDIRQ_OFFSET is removed before
> +irq_exit_rcu() returns because irqentry_exit() might schedule, which requires
> +that HARDIRQ_OFFSET has been removed from the preemption count.

Good.

>  Even though interrupt handlers are expected to run with local interrupts
>  disabled, interrupt nesting is common from an entry/exit perspective. For
> diff --git a/kernel/softirq.c b/kernel/softirq.c
> index c3729c5b284b0..efa6707edc0e9 100644
> --- a/kernel/softirq.c
> +++ b/kernel/softirq.c
> @@ -350,8 +350,8 @@ static inline void ksoftirqd_run_end(void)

As mentioned, the changes below look good except for the check
in __irq_exit_rcu() which feels a bit much.

Sebastian

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-25  7:56 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-24  4:15 [PATCH v3] softirq: Preserve interrupt context during IRQ exit Karl Mehltretter
2026-09-24 16:34 ` Bradley Morgan
2026-09-25  7:56 ` Sebastian Andrzej Siewior

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®