mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] interrupt: Disable interrupt before modifying hardirq_disable counter
@ 2026-08-27 18:10 Boqun Feng
  2026-08-27 19:48 ` [PATCH] preempt: Remove hardirq_disable_count() Boqun Feng
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Boqun Feng @ 2026-08-27 18:10 UTC (permalink / raw)
  To: tglx
  Cc: Boqun Feng, Peter Zijlstra (Intel),
	Lyude Paul, Sebastian Andrzej Siewior, Joel Fernandes,
	linux-kernel

Currently a softirq may be pending longer then expected if the
triggering interrupt happens in-between hardirq_disable_enter() and
_local_interrupt_disable() in local_interrupt_disable():

    local_interrupt_disable():
      hardirq_disable_enter();
      <interrupt>
      ...
      __irq_exit_rcu():
        // false because hardirq_disable_count() is not 0
        if (.. && !hardirq_disable_count() && ..) {
	  invoke_softirq();
	}
      _local_interrupt_disable();

, it'll defer the softirq to the next interrupt which can be forever.

The order between hardirq_disable_enter() and _local_interrupt_disable()
is to optimize re-disabling interrupts if they are already disabled, but
as 1) local_interrupt_disable() is not widely used yet and 2) the proper
way to achieve this optimization may need fixing up the counter at
entry/exit time [1], so reverse the order for now to avoid the softirq
pending issue.

Since we are doing this, the part of saving the current state is
separated from irq disabling, and we basically do the following in
local_interrupt_disable():

    local_irq_save(flags);
    if (counter++ == 0) {
      this_cpu(local_interrupt_disable_state) = flags;
    }

Change _local_interrupt_disable() to _local_interrupt_save_state().

Link: https://lore.kernel.org/lkml/87v78wezid.ffs@fw13/ [1]
Reported-by: Thomas Gleixner <tglx@kernel.org>
Fixes: e901c1510e24 ("irq,spin_lock: Add counted interrupt disabling/enabling")
Signed-off-by: Boqun Feng <boqun@kernel.org>
---
 include/linux/interrupt_rc.h | 19 ++++++++-----------
 kernel/softirq.c             | 17 ++++-------------
 2 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/include/linux/interrupt_rc.h b/include/linux/interrupt_rc.h
index b9a7f05ecf42..e68e1bedba66 100644
--- a/include/linux/interrupt_rc.h
+++ b/include/linux/interrupt_rc.h
@@ -20,11 +20,8 @@
 /* Per-CPU interrupt disabling state for local_interrupt_{disable,enable}(). */
 DECLARE_PER_CPU(unsigned long, local_interrupt_disable_state);
 
-static __always_inline void __local_interrupt_disable(void)
+static __always_inline void __local_interrupt_save_state(unsigned long flags)
 {
-	unsigned long flags;
-
-	local_irq_save(flags);
 	raw_cpu_write(local_interrupt_disable_state, flags);
 }
 
@@ -36,9 +33,9 @@ static __always_inline void __local_interrupt_enable(void)
 }
 
 #ifndef INSTANTIATE_EXPORTED_INTERRUPT_DISABLE
-static __always_inline void _local_interrupt_disable(void)
+static __always_inline void _local_interrupt_save_state(unsigned long flags)
 {
-	__local_interrupt_disable();
+	__local_interrupt_save_state(flags);
 }
 
 static __always_inline void _local_interrupt_enable(void)
@@ -46,27 +43,27 @@ static __always_inline void _local_interrupt_enable(void)
 	__local_interrupt_enable();
 }
 #else
-extern void _local_interrupt_disable(void);
+extern void _local_interrupt_save_state(unsigned long flags);
 extern void _local_interrupt_enable(void);
 #endif
 
 #else /* !MODULE */
-extern void _local_interrupt_disable(void);
+extern void _local_interrupt_save_state(unsigned long flags);
 extern void _local_interrupt_enable(void);
 #endif /* !MODULE */
 
 static inline void local_interrupt_disable(void)
 {
 	int new_count;
+	unsigned long flags;
 
 	WARN_ON_ONCE(in_nmi());
 
+	local_irq_save(flags);
 	new_count = hardirq_disable_enter();
 
-	/* Interrupts can happen here, but it's OK, see __irq_exit_rcu(). */
-
 	if ((new_count & HARDIRQ_DISABLE_MASK) == HARDIRQ_DISABLE_OFFSET)
-		_local_interrupt_disable();
+		_local_interrupt_save_state(flags);
 }
 
 static inline void local_interrupt_enable(void)
diff --git a/kernel/softirq.c b/kernel/softirq.c
index 7980a4a232f9..5d02c36c40e3 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -91,11 +91,11 @@ EXPORT_PER_CPU_SYMBOL_GPL(hardirq_context);
 
 DEFINE_PER_CPU(unsigned long, local_interrupt_disable_state);
 
-void _local_interrupt_disable(void)
+void _local_interrupt_save_state(unsigned long flags)
 {
-	__local_interrupt_disable();
+	__local_interrupt_save_state(flags);
 }
-EXPORT_SYMBOL(_local_interrupt_disable);
+EXPORT_SYMBOL(_local_interrupt_save_state);
 
 void _local_interrupt_enable(void)
 {
@@ -749,16 +749,7 @@ static inline void __irq_exit_rcu(void)
 #endif
 	account_hardirq_exit(current);
 	preempt_count_sub(HARDIRQ_OFFSET);
-	/*
-	 * Interrupts may happen between hardirq_disable_enter() and
-	 * local_irq_save() in local_interrupt_disable(), if irq_exit() invokes
-	 * softirq here, we may have a softirq handler calling
-	 * local_interrupt_disable() but it won't disable the IRQ because
-	 * hardirq disabling count is already 1, hence we need to prevent
-	 * invoking softirq when a local_interrupt_disable() is ongoing.
-	 */
-	if (!in_interrupt() && !hardirq_disable_count() &&
-	    local_softirq_pending()) {
+	if (!in_interrupt() && local_softirq_pending()) {
 		/*
 		 * If we left hrtimers unarmed, make sure to arm them now,
 		 * before enabling interrupts to run softirq.
-- 
2.50.1 (Apple Git-155)


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

* [PATCH] preempt: Remove hardirq_disable_count()
  2026-08-27 18:10 [PATCH] interrupt: Disable interrupt before modifying hardirq_disable counter Boqun Feng
@ 2026-08-27 19:48 ` Boqun Feng
  2026-08-30  6:44   ` [tip: locking/urgent] " tip-bot2 for Boqun Feng
  2026-08-27 20:07 ` [PATCH] interrupt: Disable interrupt before modifying hardirq_disable counter Bradley Morgan
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Boqun Feng @ 2026-08-27 19:48 UTC (permalink / raw)
  To: tglx
  Cc: Peter Zijlstra (Intel),
	Lyude Paul, Sebastian Andrzej Siewior, Joel Fernandes,
	linux-kernel, Boqun Feng

It turns out the previous usage of hardirq_disable_count() in
__irq_exit_rcu() would cause softirq pending issues. Without that usage,
hardirq_disable_count() doesn't need to exist, so remove it.

Signed-off-by: Boqun Feng <boqun@kernel.org>
---
 include/linux/preempt.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/linux/preempt.h b/include/linux/preempt.h
index 8299657f0f86..6d2fedbd6758 100644
--- a/include/linux/preempt.h
+++ b/include/linux/preempt.h
@@ -168,7 +168,6 @@ static __always_inline unsigned char interrupt_context_level(void)
 #define in_softirq()		(softirq_count())
 #define in_interrupt()		(irq_count())
 
-#define hardirq_disable_count()	((preempt_count() & HARDIRQ_DISABLE_MASK) >> HARDIRQ_DISABLE_SHIFT)
 #define hardirq_disable_enter()	__preempt_count_add_return(HARDIRQ_DISABLE_OFFSET)
 #define hardirq_disable_exit()	__preempt_count_sub_return(HARDIRQ_DISABLE_OFFSET)
 
-- 
2.50.1 (Apple Git-155)


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

* Re: [PATCH] interrupt: Disable interrupt before modifying hardirq_disable counter
  2026-08-27 18:10 [PATCH] interrupt: Disable interrupt before modifying hardirq_disable counter Boqun Feng
  2026-08-27 19:48 ` [PATCH] preempt: Remove hardirq_disable_count() Boqun Feng
@ 2026-08-27 20:07 ` Bradley Morgan
  2026-08-29 21:02 ` Thomas Gleixner
  2026-08-29 21:34 ` [PATCH v2] " Boqun Feng
  3 siblings, 0 replies; 8+ messages in thread
From: Bradley Morgan @ 2026-08-27 20:07 UTC (permalink / raw)
  To: boqun; +Cc: bigeasy, joelagnelf, linux-kernel, lyude, peterz, tglx

On 27 August 2026 19:10:48 BST, Boqun Feng <boqun@kernel.org> wrote:
>Currently a softirq may be pending longer then expected if the
>triggering interrupt happens in-between hardirq_disable_enter() and
>_local_interrupt_disable() in local_interrupt_disable():
>
>    local_interrupt_disable():
>      hardirq_disable_enter();
>      <interrupt>
>      ...
>      __irq_exit_rcu():
>        // false because hardirq_disable_count() is not 0
>        if (.. && !hardirq_disable_count() && ..) {
>	  invoke_softirq();
>	}
>      _local_interrupt_disable();
>
>, it'll defer the softirq to the next interrupt which can be forever.
>
>The order between hardirq_disable_enter() and _local_interrupt_disable()
>is to optimize re-disabling interrupts if they are already disabled, but
>as 1) local_interrupt_disable() is not widely used yet and 2) the proper
>way to achieve this optimization may need fixing up the counter at
>entry/exit time [1], so reverse the order for now to avoid the softirq
>pending issue.
>
>Since we are doing this, the part of saving the current state is
>separated from irq disabling, and we basically do the following in
>local_interrupt_disable():
>
>    local_irq_save(flags);
>    if (counter++ == 0) {
>      this_cpu(local_interrupt_disable_state) = flags;
>    }
>
>Change _local_interrupt_disable() to _local_interrupt_save_state().
>
>Link: https://lore.kernel.org/lkml/87v78wezid.ffs@fw13/ [1]
>Reported-by: Thomas Gleixner <tglx@kernel.org>
>Fixes: e901c1510e24 ("irq,spin_lock: Add counted interrupt disabling/enabling")

LGTM, thanks 

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


>Signed-off-by: Boqun Feng <boqun@kernel.org>
>---
> include/linux/interrupt_rc.h | 19 ++++++++-----------
> kernel/softirq.c             | 17 ++++-------------
> 2 files changed, 12 insertions(+), 24 deletions(-)
>
>diff --git a/include/linux/interrupt_rc.h b/include/linux/interrupt_rc.h
>index b9a7f05ecf42..e68e1bedba66 100644
>--- a/include/linux/interrupt_rc.h
>+++ b/include/linux/interrupt_rc.h
>@@ -20,11 +20,8 @@
> /* Per-CPU interrupt disabling state for
> local_interrupt_{disable,enable}(). */
> DECLARE_PER_CPU(unsigned long, local_interrupt_disable_state);
> 
>-static __always_inline void __local_interrupt_disable(void)
>+static __always_inline void __local_interrupt_save_state(unsigned long flags)
> {
>-	unsigned long flags;
>-
>-	local_irq_save(flags);
> 	raw_cpu_write(local_interrupt_disable_state, flags);
> }
> 
>@@ -36,9 +33,9 @@ static __always_inline void __local_interrupt_enable(void)
> }
> 
> #ifndef INSTANTIATE_EXPORTED_INTERRUPT_DISABLE
>-static __always_inline void _local_interrupt_disable(void)
>+static __always_inline void _local_interrupt_save_state(unsigned long flags)
> {
>-	__local_interrupt_disable();
>+	__local_interrupt_save_state(flags);
> }
> 
> static __always_inline void _local_interrupt_enable(void)
>@@ -46,27 +43,27 @@ static __always_inline void _local_interrupt_enable(void)
> 	__local_interrupt_enable();
> }
> #else
>-extern void _local_interrupt_disable(void);
>+extern void _local_interrupt_save_state(unsigned long flags);
> extern void _local_interrupt_enable(void);
> #endif
> 
> #else /* !MODULE */
>-extern void _local_interrupt_disable(void);
>+extern void _local_interrupt_save_state(unsigned long flags);
> extern void _local_interrupt_enable(void);
> #endif /* !MODULE */
> 
> static inline void local_interrupt_disable(void)
> {
> 	int new_count;
>+	unsigned long flags;
> 
> 	WARN_ON_ONCE(in_nmi());
> 
>+	local_irq_save(flags);
> 	new_count = hardirq_disable_enter();
> 
>-	/* Interrupts can happen here, but it's OK, see __irq_exit_rcu(). */
>-
> 	if ((new_count & HARDIRQ_DISABLE_MASK) == HARDIRQ_DISABLE_OFFSET)
>-		_local_interrupt_disable();
>+		_local_interrupt_save_state(flags);
> }
> 
> static inline void local_interrupt_enable(void)
>diff --git a/kernel/softirq.c b/kernel/softirq.c
>index 7980a4a232f9..5d02c36c40e3 100644
>--- a/kernel/softirq.c
>+++ b/kernel/softirq.c
>@@ -91,11 +91,11 @@ EXPORT_PER_CPU_SYMBOL_GPL(hardirq_context);
> 
> DEFINE_PER_CPU(unsigned long, local_interrupt_disable_state);
> 
>-void _local_interrupt_disable(void)
>+void _local_interrupt_save_state(unsigned long flags)
> {
>-	__local_interrupt_disable();
>+	__local_interrupt_save_state(flags);
> }
>-EXPORT_SYMBOL(_local_interrupt_disable);
>+EXPORT_SYMBOL(_local_interrupt_save_state);
> 
> void _local_interrupt_enable(void)
> {
>@@ -749,16 +749,7 @@ static inline void __irq_exit_rcu(void)
> #endif
> 	account_hardirq_exit(current);
> 	preempt_count_sub(HARDIRQ_OFFSET);
>-	/*
>-	 * Interrupts may happen between hardirq_disable_enter() and
>-	 * local_irq_save() in local_interrupt_disable(), if irq_exit() invokes
>-	 * softirq here, we may have a softirq handler calling
>-	 * local_interrupt_disable() but it won't disable the IRQ because
>-	 * hardirq disabling count is already 1, hence we need to prevent
>-	 * invoking softirq when a local_interrupt_disable() is ongoing.
>-	 */
>-	if (!in_interrupt() && !hardirq_disable_count() &&
>-	    local_softirq_pending()) {
>+	if (!in_interrupt() && local_softirq_pending()) {
> 		/*
> 		 * If we left hrtimers unarmed, make sure to arm them now,
> 		 * before enabling interrupts to run softirq.
>

--- Thanks!
https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/

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

* Re: [PATCH] interrupt: Disable interrupt before modifying hardirq_disable counter
  2026-08-27 18:10 [PATCH] interrupt: Disable interrupt before modifying hardirq_disable counter Boqun Feng
  2026-08-27 19:48 ` [PATCH] preempt: Remove hardirq_disable_count() Boqun Feng
  2026-08-27 20:07 ` [PATCH] interrupt: Disable interrupt before modifying hardirq_disable counter Bradley Morgan
@ 2026-08-29 21:02 ` Thomas Gleixner
  2026-08-29 21:15   ` Boqun Feng
  2026-08-29 21:34 ` [PATCH v2] " Boqun Feng
  3 siblings, 1 reply; 8+ messages in thread
From: Thomas Gleixner @ 2026-08-29 21:02 UTC (permalink / raw)
  To: Boqun Feng
  Cc: Boqun Feng, Peter Zijlstra (Intel),
	Lyude Paul, Sebastian Andrzej Siewior, Joel Fernandes,
	linux-kernel

On Thu, Aug 27 2026 at 11:10, Boqun Feng wrote:
> Currently a softirq may be pending longer then expected if the
> triggering interrupt happens in-between hardirq_disable_enter() and
> _local_interrupt_disable() in local_interrupt_disable():
>
>     local_interrupt_disable():
>       hardirq_disable_enter();
>       <interrupt>
>       ...
>       __irq_exit_rcu():
>         // false because hardirq_disable_count() is not 0
>         if (.. && !hardirq_disable_count() && ..) {
> 	  invoke_softirq();
> 	}
>       _local_interrupt_disable();
>
> , it'll defer the softirq to the next interrupt which can be forever.
>
> The order between hardirq_disable_enter() and _local_interrupt_disable()
> is to optimize re-disabling interrupts if they are already disabled, but
> as 1) local_interrupt_disable() is not widely used yet and 2) the proper
> way to achieve this optimization may need fixing up the counter at
> entry/exit time [1], so reverse the order for now to avoid the softirq
> pending issue.
>
> Since we are doing this, the part of saving the current state is
> separated from irq disabling, and we basically do the following in
> local_interrupt_disable():

We do nothing. See

https://docs.kernel.org/process/maintainer-tip.html#changelog

>     local_irq_save(flags);
>     if (counter++ == 0) {
>       this_cpu(local_interrupt_disable_state) = flags;
>     }
>
> Change _local_interrupt_disable() to _local_interrupt_save_state().
>
> Link: https://lore.kernel.org/lkml/87v78wezid.ffs@fw13/ [1]

  https://lore.kernel.org/lkml/87jypbfu1t.ffs@fw13/

is the mail where I explained that softirq issue.


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

* Re: [PATCH] interrupt: Disable interrupt before modifying hardirq_disable counter
  2026-08-29 21:02 ` Thomas Gleixner
@ 2026-08-29 21:15   ` Boqun Feng
  0 siblings, 0 replies; 8+ messages in thread
From: Boqun Feng @ 2026-08-29 21:15 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Peter Zijlstra (Intel),
	Lyude Paul, Sebastian Andrzej Siewior, Joel Fernandes,
	linux-kernel

On Sat, Aug 29, 2026 at 11:02:10PM +0200, Thomas Gleixner wrote:
> On Thu, Aug 27 2026 at 11:10, Boqun Feng wrote:
> > Currently a softirq may be pending longer then expected if the
> > triggering interrupt happens in-between hardirq_disable_enter() and
> > _local_interrupt_disable() in local_interrupt_disable():
> >
> >     local_interrupt_disable():
> >       hardirq_disable_enter();
> >       <interrupt>
> >       ...
> >       __irq_exit_rcu():
> >         // false because hardirq_disable_count() is not 0
> >         if (.. && !hardirq_disable_count() && ..) {
> > 	  invoke_softirq();
> > 	}
> >       _local_interrupt_disable();
> >
> > , it'll defer the softirq to the next interrupt which can be forever.
> >
> > The order between hardirq_disable_enter() and _local_interrupt_disable()
> > is to optimize re-disabling interrupts if they are already disabled, but
> > as 1) local_interrupt_disable() is not widely used yet and 2) the proper
> > way to achieve this optimization may need fixing up the counter at
> > entry/exit time [1], so reverse the order for now to avoid the softirq
> > pending issue.
> >
> > Since we are doing this, the part of saving the current state is
> > separated from irq disabling, and we basically do the following in
> > local_interrupt_disable():
> 
> We do nothing. See
> 
> https://docs.kernel.org/process/maintainer-tip.html#changelog
> 

Will fix this.

> >     local_irq_save(flags);
> >     if (counter++ == 0) {
> >       this_cpu(local_interrupt_disable_state) = flags;
> >     }
> >
> > Change _local_interrupt_disable() to _local_interrupt_save_state().
> >
> > Link: https://lore.kernel.org/lkml/87v78wezid.ffs@fw13/ [1]
> 
>   https://lore.kernel.org/lkml/87jypbfu1t.ffs@fw13/
> 
> is the mail where I explained that softirq issue.
> 

The link [1] here is for "the proper way to achieve this optimization
may need fixing up the counter at entry/exit time [1]", so I think
that's correct because it contains your explanation on how the fixup
would work. I will add a "Closes" tag to the other email that you
explained the issue. Sounds good?

Regards,
Boqun

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

* [PATCH v2] interrupt: Disable interrupt before modifying hardirq_disable counter
  2026-08-27 18:10 [PATCH] interrupt: Disable interrupt before modifying hardirq_disable counter Boqun Feng
                   ` (2 preceding siblings ...)
  2026-08-29 21:02 ` Thomas Gleixner
@ 2026-08-29 21:34 ` Boqun Feng
  2026-08-30  6:44   ` [tip: locking/urgent] " tip-bot2 for Boqun Feng
  3 siblings, 1 reply; 8+ messages in thread
From: Boqun Feng @ 2026-08-29 21:34 UTC (permalink / raw)
  To: tglx
  Cc: Peter Zijlstra (Intel),
	Lyude Paul, Sebastian Andrzej Siewior, Joel Fernandes,
	linux-kernel, Boqun Feng, Bradley Morgan

Currently a softirq may be pending longer then expected if the
triggering interrupt happens in-between hardirq_disable_enter() and
_local_interrupt_disable() in local_interrupt_disable():

    local_interrupt_disable():
      hardirq_disable_enter();
      <interrupt>
      ...
      __irq_exit_rcu():
        // false because hardirq_disable_count() is not 0
        if (.. && !hardirq_disable_count() && ..) {
	  invoke_softirq();
	}
      _local_interrupt_disable();

, it'll defer the softirq to the next interrupt which can be forever.

The order between hardirq_disable_enter() and _local_interrupt_disable()
is to optimize re-disabling interrupts if they are already disabled, but
as 1) local_interrupt_disable() is not widely used yet and 2) the proper
way to achieve this optimization may need fixing up the counter at
entry/exit time [1], so reverse the order for now to avoid the softirq
pending issue.

Because of this fix, the part of saving the current state is separated
from irq disabling, and the logic of local_interrupt_disable() becomes:

    local_irq_save(flags);
    if (counter++ == 0) {
      this_cpu(local_interrupt_disable_state) = flags;
    }

Therefore change the helper function _local_interrupt_disable() to
_local_interrupt_save_state() which only saves the current irqflags
(when interrupts get disabled the first time).

Link: https://lore.kernel.org/lkml/87v78wezid.ffs@fw13/ [1]
Reported-by: Thomas Gleixner <tglx@kernel.org>
Closes: https://lore.kernel.org/lkml/87jypbfu1t.ffs@fw13/
Fixes: e901c1510e24 ("irq,spin_lock: Add counted interrupt disabling/enabling")
Reviewed-by: Bradley Morgan <brads@mainlining.org>
Signed-off-by: Boqun Feng <boqun@kernel.org>
---
v1 -> v2:

* Use imperative mood in the last paragraph of the change log.
* Add "Closes" tag to the email of the explanation of the issue.
* Apply the RoB tag from Bradley Morgan

 include/linux/interrupt_rc.h | 19 ++++++++-----------
 kernel/softirq.c             | 17 ++++-------------
 2 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/include/linux/interrupt_rc.h b/include/linux/interrupt_rc.h
index b9a7f05ecf42..e68e1bedba66 100644
--- a/include/linux/interrupt_rc.h
+++ b/include/linux/interrupt_rc.h
@@ -20,11 +20,8 @@
 /* Per-CPU interrupt disabling state for local_interrupt_{disable,enable}(). */
 DECLARE_PER_CPU(unsigned long, local_interrupt_disable_state);
 
-static __always_inline void __local_interrupt_disable(void)
+static __always_inline void __local_interrupt_save_state(unsigned long flags)
 {
-	unsigned long flags;
-
-	local_irq_save(flags);
 	raw_cpu_write(local_interrupt_disable_state, flags);
 }
 
@@ -36,9 +33,9 @@ static __always_inline void __local_interrupt_enable(void)
 }
 
 #ifndef INSTANTIATE_EXPORTED_INTERRUPT_DISABLE
-static __always_inline void _local_interrupt_disable(void)
+static __always_inline void _local_interrupt_save_state(unsigned long flags)
 {
-	__local_interrupt_disable();
+	__local_interrupt_save_state(flags);
 }
 
 static __always_inline void _local_interrupt_enable(void)
@@ -46,27 +43,27 @@ static __always_inline void _local_interrupt_enable(void)
 	__local_interrupt_enable();
 }
 #else
-extern void _local_interrupt_disable(void);
+extern void _local_interrupt_save_state(unsigned long flags);
 extern void _local_interrupt_enable(void);
 #endif
 
 #else /* !MODULE */
-extern void _local_interrupt_disable(void);
+extern void _local_interrupt_save_state(unsigned long flags);
 extern void _local_interrupt_enable(void);
 #endif /* !MODULE */
 
 static inline void local_interrupt_disable(void)
 {
 	int new_count;
+	unsigned long flags;
 
 	WARN_ON_ONCE(in_nmi());
 
+	local_irq_save(flags);
 	new_count = hardirq_disable_enter();
 
-	/* Interrupts can happen here, but it's OK, see __irq_exit_rcu(). */
-
 	if ((new_count & HARDIRQ_DISABLE_MASK) == HARDIRQ_DISABLE_OFFSET)
-		_local_interrupt_disable();
+		_local_interrupt_save_state(flags);
 }
 
 static inline void local_interrupt_enable(void)
diff --git a/kernel/softirq.c b/kernel/softirq.c
index 7980a4a232f9..5d02c36c40e3 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -91,11 +91,11 @@ EXPORT_PER_CPU_SYMBOL_GPL(hardirq_context);
 
 DEFINE_PER_CPU(unsigned long, local_interrupt_disable_state);
 
-void _local_interrupt_disable(void)
+void _local_interrupt_save_state(unsigned long flags)
 {
-	__local_interrupt_disable();
+	__local_interrupt_save_state(flags);
 }
-EXPORT_SYMBOL(_local_interrupt_disable);
+EXPORT_SYMBOL(_local_interrupt_save_state);
 
 void _local_interrupt_enable(void)
 {
@@ -749,16 +749,7 @@ static inline void __irq_exit_rcu(void)
 #endif
 	account_hardirq_exit(current);
 	preempt_count_sub(HARDIRQ_OFFSET);
-	/*
-	 * Interrupts may happen between hardirq_disable_enter() and
-	 * local_irq_save() in local_interrupt_disable(), if irq_exit() invokes
-	 * softirq here, we may have a softirq handler calling
-	 * local_interrupt_disable() but it won't disable the IRQ because
-	 * hardirq disabling count is already 1, hence we need to prevent
-	 * invoking softirq when a local_interrupt_disable() is ongoing.
-	 */
-	if (!in_interrupt() && !hardirq_disable_count() &&
-	    local_softirq_pending()) {
+	if (!in_interrupt() && local_softirq_pending()) {
 		/*
 		 * If we left hrtimers unarmed, make sure to arm them now,
 		 * before enabling interrupts to run softirq.
-- 
2.50.1 (Apple Git-155)


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

* [tip: locking/urgent] preempt: Remove hardirq_disable_count()
  2026-08-27 19:48 ` [PATCH] preempt: Remove hardirq_disable_count() Boqun Feng
@ 2026-08-30  6:44   ` tip-bot2 for Boqun Feng
  0 siblings, 0 replies; 8+ messages in thread
From: tip-bot2 for Boqun Feng @ 2026-08-30  6:44 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Boqun Feng, Thomas Gleixner, x86, linux-kernel

The following commit has been merged into the locking/urgent branch of tip:

Commit-ID:     95a5037ec202adb579f532afb6b35680001b564e
Gitweb:        https://git.kernel.org/tip/95a5037ec202adb579f532afb6b35680001b564e
Author:        Boqun Feng <boqun@kernel.org>
AuthorDate:    Thu, 27 Aug 2026 12:48:35 -07:00
Committer:     Thomas Gleixner <tglx@kernel.org>
CommitterDate: Sun, 30 Aug 2026 08:39:04 +02:00

preempt: Remove hardirq_disable_count()

It turns out the previous usage of hardirq_disable_count() in
__irq_exit_rcu() would cause softirq pending issues. Without that usage,
hardirq_disable_count() doesn't need to exist, so remove it.

Signed-off-by: Boqun Feng <boqun@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Link: https://patch.msgid.link/20260827194835.38968-1-boqun@kernel.org
---
 include/linux/preempt.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/linux/preempt.h b/include/linux/preempt.h
index 8299657..6d2fedb 100644
--- a/include/linux/preempt.h
+++ b/include/linux/preempt.h
@@ -168,7 +168,6 @@ static __always_inline unsigned char interrupt_context_level(void)
 #define in_softirq()		(softirq_count())
 #define in_interrupt()		(irq_count())
 
-#define hardirq_disable_count()	((preempt_count() & HARDIRQ_DISABLE_MASK) >> HARDIRQ_DISABLE_SHIFT)
 #define hardirq_disable_enter()	__preempt_count_add_return(HARDIRQ_DISABLE_OFFSET)
 #define hardirq_disable_exit()	__preempt_count_sub_return(HARDIRQ_DISABLE_OFFSET)
 

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

* [tip: locking/urgent] interrupt: Disable interrupt before modifying hardirq_disable counter
  2026-08-29 21:34 ` [PATCH v2] " Boqun Feng
@ 2026-08-30  6:44   ` tip-bot2 for Boqun Feng
  0 siblings, 0 replies; 8+ messages in thread
From: tip-bot2 for Boqun Feng @ 2026-08-30  6:44 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Thomas Gleixner, Boqun Feng, Bradley Morgan, x86, linux-kernel

The following commit has been merged into the locking/urgent branch of tip:

Commit-ID:     a155ac8f0c523bd53f412196dcbb104ad1f4595f
Gitweb:        https://git.kernel.org/tip/a155ac8f0c523bd53f412196dcbb104ad1f4595f
Author:        Boqun Feng <boqun@kernel.org>
AuthorDate:    Sat, 29 Aug 2026 14:34:12 -07:00
Committer:     Thomas Gleixner <tglx@kernel.org>
CommitterDate: Sun, 30 Aug 2026 08:39:04 +02:00

interrupt: Disable interrupt before modifying hardirq_disable counter

Currently a softirq may be pending longer then expected if the
triggering interrupt happens in-between hardirq_disable_enter() and
_local_interrupt_disable() in local_interrupt_disable():

    local_interrupt_disable():
      hardirq_disable_enter();
      <interrupt>
      ...
      __irq_exit_rcu():
        // false because hardirq_disable_count() is not 0
        if (.. && !hardirq_disable_count() && ..) {
	  invoke_softirq();
	}
      _local_interrupt_disable();

, it'll defer the softirq to the next interrupt which can be forever.

The order between hardirq_disable_enter() and _local_interrupt_disable()
is to optimize re-disabling interrupts if they are already disabled, but
as 1) local_interrupt_disable() is not widely used yet and 2) the proper
way to achieve this optimization may need fixing up the counter at
entry/exit time [1], so reverse the order for now to avoid the softirq
pending issue.

Because of this fix, the part of saving the current state is separated
from irq disabling, and the logic of local_interrupt_disable() becomes:

    local_irq_save(flags);
    if (counter++ == 0) {
      this_cpu(local_interrupt_disable_state) = flags;
    }

Therefore change the helper function _local_interrupt_disable() to
_local_interrupt_save_state() which only saves the current irqflags
(when interrupts get disabled the first time).

Fixes: e901c1510e24 ("irq,spin_lock: Add counted interrupt disabling/enabling")
Reported-by: Thomas Gleixner <tglx@kernel.org>
Signed-off-by: Boqun Feng <boqun@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Reviewed-by: Bradley Morgan <brads@mainlining.org>
Link: https://patch.msgid.link/20260829213412.14303-1-boqun@kernel.org
Link: https://lore.kernel.org/lkml/87v78wezid.ffs@fw13/ [1]
Closes: https://lore.kernel.org/lkml/87jypbfu1t.ffs@fw13/
---
 include/linux/interrupt_rc.h | 19 ++++++++-----------
 kernel/softirq.c             | 17 ++++-------------
 2 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/include/linux/interrupt_rc.h b/include/linux/interrupt_rc.h
index b9a7f05..e68e1be 100644
--- a/include/linux/interrupt_rc.h
+++ b/include/linux/interrupt_rc.h
@@ -20,11 +20,8 @@
 /* Per-CPU interrupt disabling state for local_interrupt_{disable,enable}(). */
 DECLARE_PER_CPU(unsigned long, local_interrupt_disable_state);
 
-static __always_inline void __local_interrupt_disable(void)
+static __always_inline void __local_interrupt_save_state(unsigned long flags)
 {
-	unsigned long flags;
-
-	local_irq_save(flags);
 	raw_cpu_write(local_interrupt_disable_state, flags);
 }
 
@@ -36,9 +33,9 @@ static __always_inline void __local_interrupt_enable(void)
 }
 
 #ifndef INSTANTIATE_EXPORTED_INTERRUPT_DISABLE
-static __always_inline void _local_interrupt_disable(void)
+static __always_inline void _local_interrupt_save_state(unsigned long flags)
 {
-	__local_interrupt_disable();
+	__local_interrupt_save_state(flags);
 }
 
 static __always_inline void _local_interrupt_enable(void)
@@ -46,27 +43,27 @@ static __always_inline void _local_interrupt_enable(void)
 	__local_interrupt_enable();
 }
 #else
-extern void _local_interrupt_disable(void);
+extern void _local_interrupt_save_state(unsigned long flags);
 extern void _local_interrupt_enable(void);
 #endif
 
 #else /* !MODULE */
-extern void _local_interrupt_disable(void);
+extern void _local_interrupt_save_state(unsigned long flags);
 extern void _local_interrupt_enable(void);
 #endif /* !MODULE */
 
 static inline void local_interrupt_disable(void)
 {
 	int new_count;
+	unsigned long flags;
 
 	WARN_ON_ONCE(in_nmi());
 
+	local_irq_save(flags);
 	new_count = hardirq_disable_enter();
 
-	/* Interrupts can happen here, but it's OK, see __irq_exit_rcu(). */
-
 	if ((new_count & HARDIRQ_DISABLE_MASK) == HARDIRQ_DISABLE_OFFSET)
-		_local_interrupt_disable();
+		_local_interrupt_save_state(flags);
 }
 
 static inline void local_interrupt_enable(void)
diff --git a/kernel/softirq.c b/kernel/softirq.c
index 7980a4a..5d02c36 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -91,11 +91,11 @@ EXPORT_PER_CPU_SYMBOL_GPL(hardirq_context);
 
 DEFINE_PER_CPU(unsigned long, local_interrupt_disable_state);
 
-void _local_interrupt_disable(void)
+void _local_interrupt_save_state(unsigned long flags)
 {
-	__local_interrupt_disable();
+	__local_interrupt_save_state(flags);
 }
-EXPORT_SYMBOL(_local_interrupt_disable);
+EXPORT_SYMBOL(_local_interrupt_save_state);
 
 void _local_interrupt_enable(void)
 {
@@ -749,16 +749,7 @@ static inline void __irq_exit_rcu(void)
 #endif
 	account_hardirq_exit(current);
 	preempt_count_sub(HARDIRQ_OFFSET);
-	/*
-	 * Interrupts may happen between hardirq_disable_enter() and
-	 * local_irq_save() in local_interrupt_disable(), if irq_exit() invokes
-	 * softirq here, we may have a softirq handler calling
-	 * local_interrupt_disable() but it won't disable the IRQ because
-	 * hardirq disabling count is already 1, hence we need to prevent
-	 * invoking softirq when a local_interrupt_disable() is ongoing.
-	 */
-	if (!in_interrupt() && !hardirq_disable_count() &&
-	    local_softirq_pending()) {
+	if (!in_interrupt() && local_softirq_pending()) {
 		/*
 		 * If we left hrtimers unarmed, make sure to arm them now,
 		 * before enabling interrupts to run softirq.

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

end of thread, other threads:[~2026-08-30  6:44 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-27 18:10 [PATCH] interrupt: Disable interrupt before modifying hardirq_disable counter Boqun Feng
2026-08-27 19:48 ` [PATCH] preempt: Remove hardirq_disable_count() Boqun Feng
2026-08-30  6:44   ` [tip: locking/urgent] " tip-bot2 for Boqun Feng
2026-08-27 20:07 ` [PATCH] interrupt: Disable interrupt before modifying hardirq_disable counter Bradley Morgan
2026-08-29 21:02 ` Thomas Gleixner
2026-08-29 21:15   ` Boqun Feng
2026-08-29 21:34 ` [PATCH v2] " Boqun Feng
2026-08-30  6:44   ` [tip: locking/urgent] " tip-bot2 for Boqun Feng

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®