* [PATCH] genirq: Warn about using IRQF_ONESHOT without a threaded handler
@ 2026-01-12 13:40 Sebastian Andrzej Siewior
2026-01-12 17:46 ` Laurent Pinchart
0 siblings, 1 reply; 3+ messages in thread
From: Sebastian Andrzej Siewior @ 2026-01-12 13:40 UTC (permalink / raw)
To: linux-kernel, linux-rt-devel
Cc: Stefan Klug, Laurent Pinchart, Steven Rostedt, Clark Williams,
Thomas Gleixner
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/
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.
--
2.51.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] genirq: Warn about using IRQF_ONESHOT without a threaded handler
2026-01-12 13:40 [PATCH] genirq: Warn about using IRQF_ONESHOT without a threaded handler Sebastian Andrzej Siewior
@ 2026-01-12 17:46 ` Laurent Pinchart
2026-01-14 10:29 ` Stefan Klug
0 siblings, 1 reply; 3+ messages in thread
From: Laurent Pinchart @ 2026-01-12 17:46 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: linux-kernel, linux-rt-devel, Stefan Klug, Steven Rostedt,
Clark Williams, Thomas Gleixner
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
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] genirq: Warn about using IRQF_ONESHOT without a threaded handler
2026-01-12 17:46 ` Laurent Pinchart
@ 2026-01-14 10:29 ` Stefan Klug
0 siblings, 0 replies; 3+ messages in thread
From: Stefan Klug @ 2026-01-14 10:29 UTC (permalink / raw)
To: Laurent Pinchart, Sebastian Andrzej Siewior
Cc: linux-kernel, linux-rt-devel, Steven Rostedt, Clark Williams,
Thomas Gleixner
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-01-14 10:29 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-12 13:40 [PATCH] genirq: Warn about using IRQF_ONESHOT without a threaded handler Sebastian Andrzej Siewior
2026-01-12 17:46 ` Laurent Pinchart
2026-01-14 10:29 ` Stefan Klug
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox