Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1257736 > unrolled thread
| Started by | David Woodhouse <dwmw2@infradead.org> |
|---|---|
| First post | 2015-10-28 08:20 +0100 |
| Last post | 2015-10-30 10:20 +0100 |
| Articles | 2 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH] Document that IRQ_NONE should be returned when IRQ not actually *handled* David Woodhouse <dwmw2@infradead.org> - 2015-10-28 08:20 +0100
[tip:irq/core] Document that IRQ_NONE should be returned when IRQ not actually handled tip-bot for David Woodhouse <tipbot@zytor.com> - 2015-10-30 10:20 +0100
| From | David Woodhouse <dwmw2@infradead.org> |
|---|---|
| Date | 2015-10-28 08:20 +0100 |
| Subject | [PATCH] Document that IRQ_NONE should be returned when IRQ not actually *handled* |
| Message-ID | <qotbA-7YX-17@gated-at.bofh.it> |
[Multipart message — attachments visible in raw view] — view raw
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". Signed-off-by: David Woodhouse <David.Woodhouse@intel.com> --- 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 diff --git a/include/linux/irqreturn.h b/include/linux/irqreturn.h index e374e36..eb1bdcf 100644 --- a/include/linux/irqreturn.h +++ b/include/linux/irqreturn.h @@ -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 */ -- David Woodhouse Open Source Technology Centre David.Woodhouse@intel.com Intel Corporation
[toc] | [next] | [standalone]
| From | tip-bot for David Woodhouse <tipbot@zytor.com> |
|---|---|
| Date | 2015-10-30 10:20 +0100 |
| Subject | [tip:irq/core] Document that IRQ_NONE should be returned when IRQ not actually handled |
| Message-ID | <qpe0O-3Z0-7@gated-at.bofh.it> |
| In reply to | #1257736 |
Commit-ID: d9e4ad5badf4ccbfddee208c898fb8fd0c8836b1 Gitweb: http://git.kernel.org/tip/d9e4ad5badf4ccbfddee208c898fb8fd0c8836b1 Author: David Woodhouse <dwmw2@infradead.org> AuthorDate: Wed, 28 Oct 2015 16:14:31 +0900 Committer: Thomas Gleixner <tglx@linutronix.de> 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". Signed-off-by: David Woodhouse <David.Woodhouse@intel.com> Cc: davem@davemloft.net Link: http://lkml.kernel.org/r/1446016471.3405.201.camel@infradead.org Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- include/linux/irqreturn.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/irqreturn.h b/include/linux/irqreturn.h index e374e36..eb1bdcf 100644 --- a/include/linux/irqreturn.h +++ b/include/linux/irqreturn.h @@ -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 */ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web