Re: [PATCH] genirq: Warn about using IRQF_ONESHOT without a threaded handler

[Stefan Klug <stefan.klug@ideasonboard.com>]

Hi Sebastian,

Quoting Laurent Pinchart (2026-01-12 18:46:16)
> Hi Sebastian,
> 
> Thank you for the patch.
> 
> On Mon, Jan 12, 2026 at 02:40:13PM +0100, Sebastian Andrzej Siewior wrote:
> > IRQF_ONESHOT disables the interrupt source until after the threaded
> > handler completed its work. This is needed to allow the threaded handler
> > to run - otherwise the CPU will get back to the interrupt handler
> > because the interrupt source remains active and the threaded handler
> > will not able to do its work.
> > 
> > Specifying IRQF_ONESHOT without a threaded handler does not make sense.
> > It could be a leftover if the handler _was_ threaded and changed back to
> > primary and the flag was not removed. This can be problematic in the
> > `threadirqs' case because the handler is exempt from forced-threading.
> > This in turn can become a problem on a PREEMPT_RT system if the handler
> > attempts to acquire sleeping locks.
> > 
> > Warn about missing threaded handlers with the IRQF_ONESHOT flag.
> > 
> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > ---
> > 
> > This popped up after Stefan Klug posted
> >    https://lore.kernel.org/all/20260105-sklug-v6-16-topic-dw100-v3-1-dev-v1-3-65af34d04fd8@ideasonboard.com/
> 
> This patch would have saved us from trouble with the DW100 driver, so

I can only agree here :-)

Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>

Thanks,
Stefan

> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> I wonder if some people would object due to panic_on_warn, but I think
> the issue will be caught early when testing kernel updates way before it
> should hit users.
> 
> > There are a few drivers in tree which will trigger this warning such as
> > - arch/arm/mach-versatile/spc.c
> > - drivers/char/tpm/tpm_tis_spi_cr50.c
> > - drivers/edac/altera_edac.c
> > - drivers/i2c/busses/i2c-k1.c
> > - …
> > 
> > just to name a few. I *think* if IRQF_ONESHOT was on purpose and not
> > driven by copy/paste then the they wanted to use IRQF_NO_THREAD.
> > 
> >  kernel/irq/manage.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> > index 349ae7979da0e..18a8405cadb26 100644
> > --- a/kernel/irq/manage.c
> > +++ b/kernel/irq/manage.c
> > @@ -1473,6 +1473,13 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
> >       if (!(new->flags & IRQF_TRIGGER_MASK))
> >               new->flags |= irqd_get_trigger_type(&desc->irq_data);
> >  
> > +     /*
> > +      * IRQF_ONESHOT means the interrupt source in the IRQ chip will be
> > +      * masked until the threaded handled is done. If there is no thread
> > +      * handler then it makes no sense to have IRQF_ONESHOT.
> > +      */
> > +     WARN_ON_ONCE(new->flags & IRQF_ONESHOT && !new->thread_fn);
> > +
> >       /*
> >        * Check whether the interrupt nests into another interrupt
> >        * thread.
> 
> -- 
> Regards,
> 
> Laurent Pinchart