counter: interrupt-cnt: Drop IRQF_NO_THREAD flag

[PATCH] counter: interrupt-cnt: Drop IRQF_NO_THREAD flag

Posted by A. Sverdlin 1 week, 6 days ago

From: Alexander Sverdlin <alexander.sverdlin@siemens.com>

An IRQ handler can either be IRQF_NO_THREAD or acquire spinlock_t, as
CONFIG_PROVE_RAW_LOCK_NESTING warns:
=============================
[ BUG: Invalid wait context ]
6.18.0-rc1+git... #1
-----------------------------
some-user-space-process/1251 is trying to lock:
(&counter->events_list_lock){....}-{3:3}, at: counter_push_event [counter]
other info that might help us debug this:
context-{2:2}
no locks held by some-user-space-process/....
stack backtrace:
CPU: 0 UID: 0 PID: 1251 Comm: some-user-space-process 6.18.0-rc1+git... #1 PREEMPT
Call trace:
 show_stack (C)
 dump_stack_lvl
 dump_stack
 __lock_acquire
 lock_acquire
 _raw_spin_lock_irqsave
 counter_push_event [counter]
 interrupt_cnt_isr [interrupt_cnt]
 __handle_irq_event_percpu
 handle_irq_event
 handle_simple_irq
 handle_irq_desc
 generic_handle_domain_irq
 gpio_irq_handler
 handle_irq_desc
 generic_handle_domain_irq
 gic_handle_irq
 call_on_irq_stack
 do_interrupt_handler
 el0_interrupt
 __el0_irq_handler_common
 el0t_64_irq_handler
 el0t_64_irq

... and Sebastian correctly points out. Remove IRQF_NO_THREAD as an
alternative to switching to raw_spinlock_t, because the latter would limit
all potential nested locks to raw_spinlock_t only.

Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/all/20251117151314.xwLAZrWY@linutronix.de/
Fixes: a55ebd47f21f ("counter: add IRQ or GPIO based counter")
Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
---
 drivers/counter/interrupt-cnt.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/counter/interrupt-cnt.c b/drivers/counter/interrupt-cnt.c
index 6c0c1d2d7027d..e6100b5fb082e 100644
--- a/drivers/counter/interrupt-cnt.c
+++ b/drivers/counter/interrupt-cnt.c
@@ -229,8 +229,7 @@ static int interrupt_cnt_probe(struct platform_device *pdev)
 
 	irq_set_status_flags(priv->irq, IRQ_NOAUTOEN);
 	ret = devm_request_irq(dev, priv->irq, interrupt_cnt_isr,
-			       IRQF_TRIGGER_RISING | IRQF_NO_THREAD,
-			       dev_name(dev), counter);
+			       IRQF_TRIGGER_RISING, dev_name(dev), counter);
 	if (ret)
 		return ret;
 
-- 
2.51.1

Re: [PATCH] counter: interrupt-cnt: Drop IRQF_NO_THREAD flag

Posted by William Breathitt Gray 1 day, 12 hours ago

On Tue, 18 Nov 2025 09:35:48 +0100, A. Sverdlin wrote:
> An IRQ handler can either be IRQF_NO_THREAD or acquire spinlock_t, as
> CONFIG_PROVE_RAW_LOCK_NESTING warns:
> =============================
> [ BUG: Invalid wait context ]
> 6.18.0-rc1+git... #1
> -----------------------------
> some-user-space-process/1251 is trying to lock:
> (&counter->events_list_lock){....}-{3:3}, at: counter_push_event [counter]
> other info that might help us debug this:
> context-{2:2}
> no locks held by some-user-space-process/....
> stack backtrace:
> CPU: 0 UID: 0 PID: 1251 Comm: some-user-space-process 6.18.0-rc1+git... #1 PREEMPT
> Call trace:
>  show_stack (C)
>  dump_stack_lvl
>  dump_stack
>  __lock_acquire
>  lock_acquire
>  _raw_spin_lock_irqsave
>  counter_push_event [counter]
>  interrupt_cnt_isr [interrupt_cnt]
>  __handle_irq_event_percpu
>  handle_irq_event
>  handle_simple_irq
>  handle_irq_desc
>  generic_handle_domain_irq
>  gpio_irq_handler
>  handle_irq_desc
>  generic_handle_domain_irq
>  gic_handle_irq
>  call_on_irq_stack
>  do_interrupt_handler
>  el0_interrupt
>  __el0_irq_handler_common
>  el0t_64_irq_handler
>  el0t_64_irq
> 
> [...]

Applied, thanks!

[1/1] counter: interrupt-cnt: Drop IRQF_NO_THREAD flag
      commit: 1ea0a54c0a1fac796b133253804e392cb44068c8

Best regards,
-- 
William Breathitt Gray <wbg@kernel.org>

Re: [PATCH] counter: interrupt-cnt: Drop IRQF_NO_THREAD flag

Posted by Sebastian Andrzej Siewior 1 week, 6 days ago

On 2025-11-18 09:35:48 [+0100], A. Sverdlin wrote:
> From: Alexander Sverdlin <alexander.sverdlin@siemens.com>
> 
> An IRQ handler can either be IRQF_NO_THREAD or acquire spinlock_t, as
> CONFIG_PROVE_RAW_LOCK_NESTING warns:
> =============================
> [ BUG: Invalid wait context ]
> 6.18.0-rc1+git... #1
> -----------------------------
> some-user-space-process/1251 is trying to lock:
> (&counter->events_list_lock){....}-{3:3}, at: counter_push_event [counter]
> other info that might help us debug this:
> context-{2:2}
> no locks held by some-user-space-process/....
> stack backtrace:
> CPU: 0 UID: 0 PID: 1251 Comm: some-user-space-process 6.18.0-rc1+git... #1 PREEMPT
> Call trace:
>  show_stack (C)
>  dump_stack_lvl
>  dump_stack
>  __lock_acquire
>  lock_acquire
>  _raw_spin_lock_irqsave
>  counter_push_event [counter]
>  interrupt_cnt_isr [interrupt_cnt]
>  __handle_irq_event_percpu
>  handle_irq_event
>  handle_simple_irq
>  handle_irq_desc
>  generic_handle_domain_irq
>  gpio_irq_handler
>  handle_irq_desc
>  generic_handle_domain_irq
>  gic_handle_irq
>  call_on_irq_stack
>  do_interrupt_handler
>  el0_interrupt
>  __el0_irq_handler_common
>  el0t_64_irq_handler
>  el0t_64_irq

I would recommend to trim the commit message to what is required in
terms of describing the problem you faced. This backtrace contains a lot
noise and is not relevant.
The problem is that IRQF_NO_THREAD does not allow threading the
interrupt handler. Using spinlock_t in non-threaded (atomic) context is
not allowed and is reported by lockdep.

> ... and Sebastian correctly points out. Remove IRQF_NO_THREAD as an
> alternative to switching to raw_spinlock_t, because the latter would limit
> all potential nested locks to raw_spinlock_t only.
Correct.

Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Cc: stable@vger.kernel.org
> Link: https://lore.kernel.org/all/20251117151314.xwLAZrWY@linutronix.de/
> Fixes: a55ebd47f21f ("counter: add IRQ or GPIO based counter")
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>

Sebastian

Re: [PATCH] counter: interrupt-cnt: Drop IRQF_NO_THREAD flag

Posted by Oleksij Rempel 1 week, 6 days ago

Hi Alexander,

On Tue, Nov 18, 2025 at 09:35:48AM +0100, A. Sverdlin wrote:
> From: Alexander Sverdlin <alexander.sverdlin@siemens.com>
> 
> An IRQ handler can either be IRQF_NO_THREAD or acquire spinlock_t, as
> CONFIG_PROVE_RAW_LOCK_NESTING warns:
> =============================
> [ BUG: Invalid wait context ]
> 6.18.0-rc1+git... #1
> -----------------------------
> some-user-space-process/1251 is trying to lock:
> (&counter->events_list_lock){....}-{3:3}, at: counter_push_event [counter]
> other info that might help us debug this:
> context-{2:2}
> no locks held by some-user-space-process/....
> stack backtrace:
> CPU: 0 UID: 0 PID: 1251 Comm: some-user-space-process 6.18.0-rc1+git... #1 PREEMPT
> Call trace:
>  show_stack (C)
>  dump_stack_lvl
>  dump_stack
>  __lock_acquire
>  lock_acquire
>  _raw_spin_lock_irqsave
>  counter_push_event [counter]
>  interrupt_cnt_isr [interrupt_cnt]
>  __handle_irq_event_percpu
>  handle_irq_event
>  handle_simple_irq
>  handle_irq_desc
>  generic_handle_domain_irq
>  gpio_irq_handler
>  handle_irq_desc
>  generic_handle_domain_irq
>  gic_handle_irq
>  call_on_irq_stack
>  do_interrupt_handler
>  el0_interrupt
>  __el0_irq_handler_common
>  el0t_64_irq_handler
>  el0t_64_irq
> 
> ... and Sebastian correctly points out. Remove IRQF_NO_THREAD as an
> alternative to switching to raw_spinlock_t, because the latter would limit
> all potential nested locks to raw_spinlock_t only.
> 
> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Cc: stable@vger.kernel.org
> Link: https://lore.kernel.org/all/20251117151314.xwLAZrWY@linutronix.de/
> Fixes: a55ebd47f21f ("counter: add IRQ or GPIO based counter")
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
> ---
>  drivers/counter/interrupt-cnt.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/counter/interrupt-cnt.c b/drivers/counter/interrupt-cnt.c
> index 6c0c1d2d7027d..e6100b5fb082e 100644
> --- a/drivers/counter/interrupt-cnt.c
> +++ b/drivers/counter/interrupt-cnt.c
> @@ -229,8 +229,7 @@ static int interrupt_cnt_probe(struct platform_device *pdev)
>  
>  	irq_set_status_flags(priv->irq, IRQ_NOAUTOEN);
>  	ret = devm_request_irq(dev, priv->irq, interrupt_cnt_isr,
> -			       IRQF_TRIGGER_RISING | IRQF_NO_THREAD,
> -			       dev_name(dev), counter);
> +			       IRQF_TRIGGER_RISING, dev_name(dev), counter);
>  	if (ret)
>  		return ret;
>  

Hm, I guess it will break the requirement to handle at least 10kHz
interrupts. May be we should move only counter_push_event() to the
thread? or using delayed worker?

Right now I do not have needed system for testing to come with better
proposal.

Best Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] counter: interrupt-cnt: Drop IRQF_NO_THREAD flag

Posted by Sebastian Andrzej Siewior 1 week, 6 days ago

On 2025-11-18 10:12:56 [+0100], Oleksij Rempel wrote:
> Hi Alexander,
Hi,

> > --- a/drivers/counter/interrupt-cnt.c
> > +++ b/drivers/counter/interrupt-cnt.c
> > @@ -229,8 +229,7 @@ static int interrupt_cnt_probe(struct platform_device *pdev)
> >  
> >  	irq_set_status_flags(priv->irq, IRQ_NOAUTOEN);
> >  	ret = devm_request_irq(dev, priv->irq, interrupt_cnt_isr,
> > -			       IRQF_TRIGGER_RISING | IRQF_NO_THREAD,
> > -			       dev_name(dev), counter);
> > +			       IRQF_TRIGGER_RISING, dev_name(dev), counter);
> >  	if (ret)
> >  		return ret;
> >  
> 
> Hm, I guess it will break the requirement to handle at least 10kHz
> interrupts. May be we should move only counter_push_event() to the
> thread? or using delayed worker?

IRQF_NO_THREAD only prohibits threading of interrupts on !RT if
threadirqs was specified on the boot command line. This should not
effect you general use case.
As the threaded interrupt runs as SCHED_FIFO-50 it will be preferred
over any SCHED_OTHER so it still should the most prefer task in the
system. 10kHz interrupt sounds like one interrupt every 100us. This
sounds like a lot if the CPU is also doing other things.
Anyway.

> Right now I do not have needed system for testing to come with better
> proposal.
> 
> Best Regards,
> Oleksij

Sebastian

Re: [PATCH] counter: interrupt-cnt: Drop IRQF_NO_THREAD flag

Posted by Sverdlin, Alexander 1 week, 6 days ago

Hi Oleksij!

On Tue, 2025-11-18 at 10:12 +0100, Oleksij Rempel wrote:
> > An IRQ handler can either be IRQF_NO_THREAD or acquire spinlock_t, as
> > CONFIG_PROVE_RAW_LOCK_NESTING warns:
> > =============================
> > [ BUG: Invalid wait context ]
> > 6.18.0-rc1+git... #1
> > -----------------------------
> > some-user-space-process/1251 is trying to lock:
> > (&counter->events_list_lock){....}-{3:3}, at: counter_push_event [counter]
> > other info that might help us debug this:
> > context-{2:2}
> > no locks held by some-user-space-process/....
> > stack backtrace:
> > CPU: 0 UID: 0 PID: 1251 Comm: some-user-space-process 6.18.0-rc1+git... #1 PREEMPT
> > Call trace:
> >   show_stack (C)
> >   dump_stack_lvl
> >   dump_stack
> >   __lock_acquire
> >   lock_acquire
> >   _raw_spin_lock_irqsave
> >   counter_push_event [counter]
> >   interrupt_cnt_isr [interrupt_cnt]
> >   __handle_irq_event_percpu
> >   handle_irq_event
> >   handle_simple_irq
> >   handle_irq_desc
> >   generic_handle_domain_irq
> >   gpio_irq_handler
> >   handle_irq_desc
> >   generic_handle_domain_irq
> >   gic_handle_irq
> >   call_on_irq_stack
> >   do_interrupt_handler
> >   el0_interrupt
> >   __el0_irq_handler_common
> >   el0t_64_irq_handler
> >   el0t_64_irq
> > 
> > ... and Sebastian correctly points out. Remove IRQF_NO_THREAD as an
> > alternative to switching to raw_spinlock_t, because the latter would limit
> > all potential nested locks to raw_spinlock_t only.
> > 
> > Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > Cc: stable@vger.kernel.org
> > Link: https://lore.kernel.org/all/20251117151314.xwLAZrWY@linutronix.de/
> > Fixes: a55ebd47f21f ("counter: add IRQ or GPIO based counter")
> > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
> > ---
> >   drivers/counter/interrupt-cnt.c | 3 +--
> >   1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/counter/interrupt-cnt.c b/drivers/counter/interrupt-cnt.c
> > index 6c0c1d2d7027d..e6100b5fb082e 100644
> > --- a/drivers/counter/interrupt-cnt.c
> > +++ b/drivers/counter/interrupt-cnt.c
> > @@ -229,8 +229,7 @@ static int interrupt_cnt_probe(struct platform_device *pdev)
> >   
> >   	irq_set_status_flags(priv->irq, IRQ_NOAUTOEN);
> >   	ret = devm_request_irq(dev, priv->irq, interrupt_cnt_isr,
> > -			       IRQF_TRIGGER_RISING | IRQF_NO_THREAD,
> > -			       dev_name(dev), counter);
> > +			       IRQF_TRIGGER_RISING, dev_name(dev), counter);
> >   	if (ret)
> >   		return ret;
> >   
> 
> Hm, I guess it will break the requirement to handle at least 10kHz
> interrupts. May be we should move only counter_push_event() to the
> thread? or using delayed worker?
> 
> Right now I do not have needed system for testing to come with better
> proposal.

I thought about possible performance implications of the patch.
But the performance regression would happen only with PREEMPT_RT.
However, it must have been broken (and by that I mean really broken, like
"scheduling in atomic") from the very beginning in PREEMPT_RT and
I suppose your initial tests were performed not with PREEMPT_RT kernel.

So overall there shall be no possible performance regression in reality.

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com

Re: [PATCH] counter: interrupt-cnt: Drop IRQF_NO_THREAD flag

Posted by Oleksij Rempel 1 week, 6 days ago

On Tue, Nov 18, 2025 at 09:51:02AM +0000, Sverdlin, Alexander wrote:
> Hi Oleksij!
> 
> On Tue, 2025-11-18 at 10:12 +0100, Oleksij Rempel wrote:
> > > An IRQ handler can either be IRQF_NO_THREAD or acquire spinlock_t, as
> > > CONFIG_PROVE_RAW_LOCK_NESTING warns:
> > > =============================
> > > [ BUG: Invalid wait context ]
> > > 6.18.0-rc1+git... #1
> > > -----------------------------
> > > some-user-space-process/1251 is trying to lock:
> > > (&counter->events_list_lock){....}-{3:3}, at: counter_push_event [counter]
> > > other info that might help us debug this:
> > > context-{2:2}
> > > no locks held by some-user-space-process/....
> > > stack backtrace:
> > > CPU: 0 UID: 0 PID: 1251 Comm: some-user-space-process 6.18.0-rc1+git... #1 PREEMPT
> > > Call trace:
> > >   show_stack (C)
> > >   dump_stack_lvl
> > >   dump_stack
> > >   __lock_acquire
> > >   lock_acquire
> > >   _raw_spin_lock_irqsave
> > >   counter_push_event [counter]
> > >   interrupt_cnt_isr [interrupt_cnt]
> > >   __handle_irq_event_percpu
> > >   handle_irq_event
> > >   handle_simple_irq
> > >   handle_irq_desc
> > >   generic_handle_domain_irq
> > >   gpio_irq_handler
> > >   handle_irq_desc
> > >   generic_handle_domain_irq
> > >   gic_handle_irq
> > >   call_on_irq_stack
> > >   do_interrupt_handler
> > >   el0_interrupt
> > >   __el0_irq_handler_common
> > >   el0t_64_irq_handler
> > >   el0t_64_irq
> > > 
> > > ... and Sebastian correctly points out. Remove IRQF_NO_THREAD as an
> > > alternative to switching to raw_spinlock_t, because the latter would limit
> > > all potential nested locks to raw_spinlock_t only.
> > > 
> > > Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > > Cc: stable@vger.kernel.org
> > > Link: https://lore.kernel.org/all/20251117151314.xwLAZrWY@linutronix.de/
> > > Fixes: a55ebd47f21f ("counter: add IRQ or GPIO based counter")
> > > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
> > > ---
> > >   drivers/counter/interrupt-cnt.c | 3 +--
> > >   1 file changed, 1 insertion(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/counter/interrupt-cnt.c b/drivers/counter/interrupt-cnt.c
> > > index 6c0c1d2d7027d..e6100b5fb082e 100644
> > > --- a/drivers/counter/interrupt-cnt.c
> > > +++ b/drivers/counter/interrupt-cnt.c
> > > @@ -229,8 +229,7 @@ static int interrupt_cnt_probe(struct platform_device *pdev)
> > >   
> > >   	irq_set_status_flags(priv->irq, IRQ_NOAUTOEN);
> > >   	ret = devm_request_irq(dev, priv->irq, interrupt_cnt_isr,
> > > -			       IRQF_TRIGGER_RISING | IRQF_NO_THREAD,
> > > -			       dev_name(dev), counter);
> > > +			       IRQF_TRIGGER_RISING, dev_name(dev), counter);
> > >   	if (ret)
> > >   		return ret;
> > >   
> > 
> > Hm, I guess it will break the requirement to handle at least 10kHz
> > interrupts. May be we should move only counter_push_event() to the
> > thread? or using delayed worker?
> > 
> > Right now I do not have needed system for testing to come with better
> > proposal.
> 
> I thought about possible performance implications of the patch.
> But the performance regression would happen only with PREEMPT_RT.
> However, it must have been broken (and by that I mean really broken, like
> "scheduling in atomic") from the very beginning in PREEMPT_RT and
> I suppose your initial tests were performed not with PREEMPT_RT kernel.

Ack.

> So overall there shall be no possible performance regression in reality.

Ok, thank you!

Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de>

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |