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

[Oleksij Rempel <o.rempel@pengutronix.de>]

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 |