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

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

Posted by Sebastian Andrzej Siewior 3 weeks, 5 days ago

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

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

Posted by Laurent Pinchart 3 weeks, 4 days ago

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

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

Posted by Stefan Klug 3 weeks, 3 days ago

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

[tip: irq/core] genirq: Warn about using IRQF_ONESHOT without a threaded handler

Posted by tip-bot2 for Sebastian Andrzej Siewior 3 weeks, 4 days ago

The following commit has been merged into the irq/core branch of tip:

Commit-ID:     aef30c8d569c0f31715447525640044c74feb26f
Gitweb:        https://git.kernel.org/tip/aef30c8d569c0f31715447525640044c74feb26f
Author:        Sebastian Andrzej Siewior <bigeasy@linutronix.de>
AuthorDate:    Mon, 12 Jan 2026 14:40:13 +01:00
Committer:     Thomas Gleixner <tglx@kernel.org>
CommitterDate: Tue, 13 Jan 2026 09:56:25 +01:00

genirq: Warn about using IRQF_ONESHOT without a threaded handler

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>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Link: https://patch.msgid.link/20260112134013.eQWyReHR@linutronix.de
---
 kernel/irq/manage.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index bc2d36b..dde1aa6 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1474,6 +1474,13 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
 		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.
 	 */