generic_handle_irq() must be used from primary IRQ-handler (chain handler/ interrupt controller entry). It is low level code and the function expects that interrupts are disabled at entry point.
This isn't the case for invocations from tasklets, workqueues or the primary interrupt handler on PREEMPT_RT. Once this gets noticed a "local_irq_disable|safe()" is added. To avoid further confusion this series adds generic_handle_irq_safe() which can be used from any context and adds a few user.
v2…v1: - Redo kernel-doc for generic_handle_irq_safe() in #1. - Use generic_handle_irq_safe() instead of generic_handle_irq() in the patch description where I accidently used the wrong one.
v1: https://lore.kernel.org/all/20220127113303.3012207-1-bigeasy@linutronix.de/
Sebastian
Provide generic_handle_irq_safe() which can used from any context.
Suggested-by: Thomas Gleixner tglx@linutronix.de Signed-off-by: Sebastian Andrzej Siewior bigeasy@linutronix.de Reviewed-by: Hans de Goede hdegoede@redhat.com Reviewed-by: Oleksandr Natalenko oleksandr@natalenko.name --- include/linux/irqdesc.h | 1 + kernel/irq/irqdesc.c | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+)
diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h index 93d270ca0c567..a77584593f7d1 100644 --- a/include/linux/irqdesc.h +++ b/include/linux/irqdesc.h @@ -160,6 +160,7 @@ static inline void generic_handle_irq_desc(struct irq_desc *desc)
int handle_irq_desc(struct irq_desc *desc); int generic_handle_irq(unsigned int irq); +int generic_handle_irq_safe(unsigned int irq);
#ifdef CONFIG_IRQ_DOMAIN /* diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c index 2267e6527db3c..4c58fa940a61c 100644 --- a/kernel/irq/irqdesc.c +++ b/kernel/irq/irqdesc.c @@ -662,6 +662,29 @@ int generic_handle_irq(unsigned int irq) } EXPORT_SYMBOL_GPL(generic_handle_irq);
+/** + * generic_handle_irq_safe - Invoke the handler for a particular irq from any + * context. + * @irq: The irq number to handle + * + * Returns: 0 on success, a negative value on error. + * + * This function can be called any context (IRQ or process context). It will + * report an error if not invoked from IRQ context and the irq has been marked + * to enforce IRQ-contex only. + */ +int generic_handle_irq_safe(unsigned int irq) +{ + unsigned long flags; + int ret; + + local_irq_save(flags); + ret = handle_irq_desc(irq_to_desc(irq)); + local_irq_restore(flags); + return ret; +} +EXPORT_SYMBOL_GPL(generic_handle_irq_safe); + #ifdef CONFIG_IRQ_DOMAIN /** * generic_handle_domain_irq - Invoke the handler for a HW irq belonging
- This function can be called any context (IRQ or process context). It will
"from any context"
- report an error if not invoked from IRQ context and the irq has been marked
- to enforce IRQ-contex only.
"IRQ-context"
Other than that, the paragraph is really helpful now IMO. So with the above fixed, you may add:
Reviewed-by: Wolfram Sang wsa+renesas@sang-engineering.com
Provide generic_handle_irq_safe() which can used from any context.
Suggested-by: Thomas Gleixner tglx@linutronix.de Signed-off-by: Sebastian Andrzej Siewior bigeasy@linutronix.de Reviewed-by: Hans de Goede hdegoede@redhat.com Reviewed-by: Oleksandr Natalenko oleksandr@natalenko.name Reviewed-by: Wolfram Sang wsa+renesas@sang-engineering.com ---
v2…v3: Correct kernel doc for generic_handle_irq_safe() as per Wolfram Sang.
include/linux/irqdesc.h | 1 + kernel/irq/irqdesc.c | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+)
diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h index 93d270ca0c567..a77584593f7d1 100644 --- a/include/linux/irqdesc.h +++ b/include/linux/irqdesc.h @@ -160,6 +160,7 @@ static inline void generic_handle_irq_desc(struct irq_desc *desc)
int handle_irq_desc(struct irq_desc *desc); int generic_handle_irq(unsigned int irq); +int generic_handle_irq_safe(unsigned int irq);
#ifdef CONFIG_IRQ_DOMAIN /* diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c index 2267e6527db3c..346d283d2da14 100644 --- a/kernel/irq/irqdesc.c +++ b/kernel/irq/irqdesc.c @@ -662,6 +662,29 @@ int generic_handle_irq(unsigned int irq) } EXPORT_SYMBOL_GPL(generic_handle_irq);
+/** + * generic_handle_irq_safe - Invoke the handler for a particular irq from any + * context. + * @irq: The irq number to handle + * + * Returns: 0 on success, a negative value on error. + * + * This function can be called from any context (IRQ or process context). It + * will report an error if not invoked from IRQ context and the irq has been + * marked to enforce IRQ-context only. + */ +int generic_handle_irq_safe(unsigned int irq) +{ + unsigned long flags; + int ret; + + local_irq_save(flags); + ret = handle_irq_desc(irq_to_desc(irq)); + local_irq_restore(flags); + return ret; +} +EXPORT_SYMBOL_GPL(generic_handle_irq_safe); + #ifdef CONFIG_IRQ_DOMAIN /** * generic_handle_domain_irq - Invoke the handler for a HW irq belonging
The i2c-i801 driver invokes i2c_handle_smbus_host_notify() from his interrupt service routine. On PREEMPT_RT i2c-i801's handler is forced threaded with enabled interrupts which leads to a warning by handle_irq_event_percpu() assuming that irq_default_primary_handler() enabled interrupts.
i2c-i801's interrupt handler can't be made non-threaded because the interrupt line is shared with other devices.
Use generic_handle_irq_safe() which can invoked with disabled and enabled interrupts.
Reported-by: Michael Below below@judiz.de Link: https://bugs.debian.org/1002537 Cc: Salvatore Bonaccorso carnil@debian.org Signed-off-by: Sebastian Andrzej Siewior bigeasy@linutronix.de Reviewed-by: Oleksandr Natalenko oleksandr@natalenko.name Acked-by: Wolfram Sang wsa@kernel.org --- drivers/i2c/i2c-core-base.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c index 2c59dd748a49f..3f9e5303b6163 100644 --- a/drivers/i2c/i2c-core-base.c +++ b/drivers/i2c/i2c-core-base.c @@ -1424,7 +1424,7 @@ int i2c_handle_smbus_host_notify(struct i2c_adapter *adap, unsigned short addr) if (irq <= 0) return -ENXIO;
- generic_handle_irq(irq); + generic_handle_irq_safe(irq);
return 0; }
Instead of manually disabling interrupts before invoking use generic_handle_irq_safe() which can be invoked with enabled and disabled interrupts.
Signed-off-by: Sebastian Andrzej Siewior bigeasy@linutronix.de Reviewed-by: Hans de Goede hdegoede@redhat.com Acked-by: Wolfram Sang wsa@kernel.org --- drivers/i2c/busses/i2c-cht-wc.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/busses/i2c-cht-wc.c b/drivers/i2c/busses/i2c-cht-wc.c index 1cf68f85b2e11..8ccf0c928bb44 100644 --- a/drivers/i2c/busses/i2c-cht-wc.c +++ b/drivers/i2c/busses/i2c-cht-wc.c @@ -99,15 +99,8 @@ static irqreturn_t cht_wc_i2c_adap_thread_handler(int id, void *data) * interrupt handler as well, so running the client irq handler from * this thread will cause things to lock up. */ - if (reg & CHT_WC_EXTCHGRIRQ_CLIENT_IRQ) { - /* - * generic_handle_irq expects local IRQs to be disabled - * as normally it is called from interrupt context. - */ - local_irq_disable(); - generic_handle_irq(adap->client_irq); - local_irq_enable(); - } + if (reg & CHT_WC_EXTCHGRIRQ_CLIENT_IRQ) + generic_handle_irq_safe(adap->client_irq);
return IRQ_HANDLED; }
generic_handle_irq() is invoked from a regular interrupt service routine. This handler will become a forced-threaded handler on PREEMPT_RT and will be invoked with enabled interrupts. The generic_handle_irq() must be invoked with disabled interrupts in order to avoid deadlocks.
Instead of manually disabling interrupts before invoking use generic_handle_irq_safe() which can be invoked with enabled and disabled interrupts.
Signed-off-by: Sebastian Andrzej Siewior bigeasy@linutronix.de --- drivers/misc/hi6421v600-irq.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/hi6421v600-irq.c b/drivers/misc/hi6421v600-irq.c index 1c763796cf1fa..caa3de37698b0 100644 --- a/drivers/misc/hi6421v600-irq.c +++ b/drivers/misc/hi6421v600-irq.c @@ -117,8 +117,8 @@ static irqreturn_t hi6421v600_irq_handler(int irq, void *__priv) * If both powerkey down and up IRQs are received, * handle them at the right order */ - generic_handle_irq(priv->irqs[POWERKEY_DOWN]); - generic_handle_irq(priv->irqs[POWERKEY_UP]); + generic_handle_irq_safe(priv->irqs[POWERKEY_DOWN]); + generic_handle_irq_safe(priv->irqs[POWERKEY_UP]); pending &= ~HISI_IRQ_POWERKEY_UP_DOWN; }
@@ -126,7 +126,7 @@ static irqreturn_t hi6421v600_irq_handler(int irq, void *__priv) continue;
for_each_set_bit(offset, &pending, BITS_PER_BYTE) { - generic_handle_irq(priv->irqs[offset + i * BITS_PER_BYTE]); + generic_handle_irq_safe(priv->irqs[offset + i * BITS_PER_BYTE]); } }
On Mon, 31 Jan 2022, Sebastian Andrzej Siewior wrote:
generic_handle_irq() is invoked from a regular interrupt service routine. This handler will become a forced-threaded handler on PREEMPT_RT and will be invoked with enabled interrupts. The generic_handle_irq() must be invoked with disabled interrupts in order to avoid deadlocks.
Instead of manually disabling interrupts before invoking use generic_handle_irq_safe() which can be invoked with enabled and disabled interrupts.
Signed-off-by: Sebastian Andrzej Siewior bigeasy@linutronix.de
drivers/misc/hi6421v600-irq.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
The subject line should be "misc".
diff --git a/drivers/misc/hi6421v600-irq.c b/drivers/misc/hi6421v600-irq.c index 1c763796cf1fa..caa3de37698b0 100644 --- a/drivers/misc/hi6421v600-irq.c +++ b/drivers/misc/hi6421v600-irq.c @@ -117,8 +117,8 @@ static irqreturn_t hi6421v600_irq_handler(int irq, void *__priv) * If both powerkey down and up IRQs are received, * handle them at the right order */
generic_handle_irq(priv->irqs[POWERKEY_DOWN]);generic_handle_irq(priv->irqs[POWERKEY_UP]);
generic_handle_irq_safe(priv->irqs[POWERKEY_DOWN]);generic_handle_irq_safe(priv->irqs[POWERKEY_UP]); pending &= ~HISI_IRQ_POWERKEY_UP_DOWN;@@ -126,7 +126,7 @@ static irqreturn_t hi6421v600_irq_handler(int irq, void *__priv) continue; for_each_set_bit(offset, &pending, BITS_PER_BYTE) {
generic_handle_irq(priv->irqs[offset + i * BITS_PER_BYTE]);
generic_handle_irq_safe(priv->irqs[offset + i * BITS_PER_BYTE]);
generic_handle_irq() is invoked from a regular interrupt service routine. This handler will become a forced-threaded handler on PREEMPT_RT and will be invoked with enabled interrupts. The generic_handle_irq() must be invoked with disabled interrupts in order to avoid deadlocks.
Instead of manually disabling interrupts before invoking use generic_handle_irq_safe() which can be invoked with enabled and disabled interrupts.
Signed-off-by: Sebastian Andrzej Siewior bigeasy@linutronix.de --- drivers/misc/hi6421v600-irq.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/hi6421v600-irq.c b/drivers/misc/hi6421v600-irq.c index 1c763796cf1fa..caa3de37698b0 100644 --- a/drivers/misc/hi6421v600-irq.c +++ b/drivers/misc/hi6421v600-irq.c @@ -117,8 +117,8 @@ static irqreturn_t hi6421v600_irq_handler(int irq, void *__priv) * If both powerkey down and up IRQs are received, * handle them at the right order */ - generic_handle_irq(priv->irqs[POWERKEY_DOWN]); - generic_handle_irq(priv->irqs[POWERKEY_UP]); + generic_handle_irq_safe(priv->irqs[POWERKEY_DOWN]); + generic_handle_irq_safe(priv->irqs[POWERKEY_UP]); pending &= ~HISI_IRQ_POWERKEY_UP_DOWN; }
@@ -126,7 +126,7 @@ static irqreturn_t hi6421v600_irq_handler(int irq, void *__priv) continue;
for_each_set_bit(offset, &pending, BITS_PER_BYTE) { - generic_handle_irq(priv->irqs[offset + i * BITS_PER_BYTE]); + generic_handle_irq_safe(priv->irqs[offset + i * BITS_PER_BYTE]); } }
Instead of manually disabling interrupts before invoking use generic_handle_irq_safe() which can be invoked with enabled and disabled interrupts.
Signed-off-by: Sebastian Andrzej Siewior bigeasy@linutronix.de --- drivers/mfd/ezx-pcap.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/mfd/ezx-pcap.c b/drivers/mfd/ezx-pcap.c index 70fa18b04ad2b..b14d3f98e1ebd 100644 --- a/drivers/mfd/ezx-pcap.c +++ b/drivers/mfd/ezx-pcap.c @@ -193,13 +193,11 @@ static void pcap_isr_work(struct work_struct *work) ezx_pcap_write(pcap, PCAP_REG_MSR, isr | msr); ezx_pcap_write(pcap, PCAP_REG_ISR, isr);
- local_irq_disable(); service = isr & ~msr; for (irq = pcap->irq_base; service; service >>= 1, irq++) { if (service & 1) - generic_handle_irq(irq); + generic_handle_irq_safe(irq); } - local_irq_enable(); ezx_pcap_write(pcap, PCAP_REG_MSR, pcap->msr); } while (gpio_get_value(pdata->gpio)); }
Instead of manually disabling interrupts before invoking use generic_handle_irq_safe() which can be invoked with enabled and disabled interrupts.
Signed-off-by: Sebastian Andrzej Siewior bigeasy@linutronix.de --- drivers/net/usb/lan78xx.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c index b8e20a3f2b84e..415f16662f88e 100644 --- a/drivers/net/usb/lan78xx.c +++ b/drivers/net/usb/lan78xx.c @@ -1537,11 +1537,8 @@ static void lan78xx_status(struct lan78xx_net *dev, struct urb *urb) netif_dbg(dev, link, dev->net, "PHY INTR: 0x%08x\n", intdata); lan78xx_defer_kevent(dev, EVENT_LINK_RESET);
- if (dev->domain_data.phyirq > 0) { - local_irq_disable(); - generic_handle_irq(dev->domain_data.phyirq); - local_irq_enable(); - } + if (dev->domain_data.phyirq > 0) + generic_handle_irq_safe(dev->domain_data.phyirq); } else { netdev_warn(dev->net, "unexpected interrupt: 0x%08x\n", intdata);
Instead of manually disabling interrupts before invoking use generic_handle_irq_safe() which can be invoked with enabled and disabled interrupts.
Signed-off-by: Sebastian Andrzej Siewior bigeasy@linutronix.de --- drivers/staging/greybus/gpio.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/staging/greybus/gpio.c b/drivers/staging/greybus/gpio.c index 7e6347fe93f99..8a7cf1d0e9688 100644 --- a/drivers/staging/greybus/gpio.c +++ b/drivers/staging/greybus/gpio.c @@ -391,10 +391,7 @@ static int gb_gpio_request_handler(struct gb_operation *op) return -EINVAL; }
- local_irq_disable(); - ret = generic_handle_irq(irq); - local_irq_enable(); - + ret = generic_handle_irq_safe(irq); if (ret) dev_err(dev, "failed to invoke irq handler\n");
On Mon, Jan 31, 2022 at 01:34:04PM +0100, Sebastian Andrzej Siewior wrote:
Instead of manually disabling interrupts before invoking use generic_handle_irq_safe() which can be invoked with enabled and disabled interrupts.
Signed-off-by: Sebastian Andrzej Siewior bigeasy@linutronix.de
Acked-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
On Mon, Jan 31, 2022 at 01:34:04PM +0100, Sebastian Andrzej Siewior wrote:
Instead of manually disabling interrupts before invoking use generic_handle_irq_safe() which can be invoked with enabled and disabled interrupts.
Signed-off-by: Sebastian Andrzej Siewior bigeasy@linutronix.de
Acked-by: Johan Hovold johan@kernel.org
-
Greg Kroah-Hartman -
Johan Hovold -
Lee Jones -
Sebastian Andrzej Siewior -
Wolfram Sang