Document that IRQ_NONE should be returned when IRQ not actually *handled*
PATCH 1/1 Document that IRQ_NONE should be returned when IRQ not actually *handled*
Our IRQ storm detection works when an interrupt handler returns IRQ_NONE for thousands of consecutive interrupts in a second. It doesn't hurt to occasionally return IRQ_NONE when the interrupt is actually genuine.
Drivers should only be returning IRQ_HANDLED if they have actually *done* something to stop an interrupt from happening — it doesn't just mean "this really *was* my device".
---
See recent discussion about the 8139cp Ethernet driver¹. It developed (OK, I introduced) a bug where it would re-enable the RX IRQ when handling a TX timeout and resetting the hardware.
This leads to an IRQ storm with cp_interrupt() *not* doing anything about the RX IRQ, because NAPI was already scheduled. And then returning IRQ_HANDLED anyway. And complete death of the machine.
Our IRQ storm detection should handle that kind of thing — it's designed to catch both hardware *and* software screwups. But because of the cp_interrupt() return value, it didn't.
I tried to fix cp_interrupt(), and submitted a patch which made the failure mode much saner — the offending IRQ got disabled and the machine continued happily, with the network even *working* in polling mode. It met with resistance.
To overcome that resistance, we should clearly document the expectation that device drivers should return IRQ_NONE in that kind of case. Let's start by at least fixing the *wrong* text in irqreturn.h, which says that IRQ_NONE means "not my device"...
¹ http://www.spinics.net/lists/netdev/msg343991.html
http://www.spinics.net/lists/netdev/msg343995.html
http://www.spinics.net/lists/netdev/msg344265.html
include/linux/irqreturn.h +1 -1
@@ -3,7 +3,7 @@/** * enum irqreturn -* @IRQ_NONE interrupt was not from this device +* @IRQ_NONE interrupt was not from this device or was not handled * @IRQ_HANDLED interrupt was handled by this device * @IRQ_WAKE_THREAD handler requests to wake the handler thread */
Signature · 2 lines
David Woodhouse Open Source Technology Centre [email protected] Intel Corporation
tip-bot for David Woodhouse
merge bot[tip:irq/core] Document that IRQ_NONE should be returned when IRQ not actually handled
Document that IRQ_NONE should be returned when IRQ not actually handled
Commit-ID: d9e4ad5badf4ccbfddee208c898fb8fd0c8836b1 Gitweb: http://git.kernel.org/tip/d9e4ad5badf4ccbfddee208c898fb8fd0c8836b1 Author: David Woodhouse <[email protected]> AuthorDate: Wed, 28 Oct 2015 16:14:31 +0900 Committer: Thomas Gleixner <[email protected]> CommitDate: Fri, 30 Oct 2015 10:13:26 +0100
Document that IRQ_NONE should be returned when IRQ not actually handled
Our IRQ storm detection works when an interrupt handler returns IRQ_NONE for thousands of consecutive interrupts in a second. It doesn't hurt to occasionally return IRQ_NONE when the interrupt is actually genuine.
Drivers should only be returning IRQ_HANDLED if they have actually *done* something to stop an interrupt from happening — it doesn't just mean "this really *was* my device".
--- include/linux/irqreturn.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/irqreturn.h +1 -1
@@ -3,7 +3,7 @@/** * enum irqreturn -* @IRQ_NONE interrupt was not from this device +* @IRQ_NONE interrupt was not from this device or was not handled * @IRQ_HANDLED interrupt was handled by this device * @IRQ_WAKE_THREAD handler requests to wake the handler thread */