public inbox for linux-rt-devel@lists.linux.dev
 help / color / mirror / Atom feed
* [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