* [PATCHv2 0/2] genirq: reliably replay pending edge-triggered irq (plus doc)
@ 2010-05-04 0:19 Guillaume Knispel
2010-05-04 0:21 ` [PATCHv2 1/2] genirq: reliably replay pending edge-triggered irq Guillaume Knispel
2010-05-04 0:23 ` [PATCHv2 2/2] genirq: update doc Guillaume Knispel
0 siblings, 2 replies; 3+ messages in thread
From: Guillaume Knispel @ 2010-05-04 0:19 UTC (permalink / raw)
To: linux-kernel, Linuxppc-dev
Cc: Randy Dunlap, Lars-Peter Clausen, Russell King,
Bartlomiej Zolnierkiewicz, Haavard Skinnemoen, Guillaume Knispel,
Peter Zijlstra, Ingo Molnar, Linus Torvalds, Thomas Gleixner
In the following series:
[1/2] implements the proposal I made at the end of the thread
http://lkml.org/lkml/2010/4/19/129 to reliably support replay
of edge-triggered interrupts on all architectures when using
disable_irq() / enable_irq().
Proper replays of pending edge-triggered interrupts was
depending on CONFIG_HARDIRQS_SW_RESEND which only seems to have
been noticed for plateforms of ARM and AVR32 architecture while
it should also have been used on other architectures to get the
correct behavior. So the patch unconditionally builds the
resend_irqs() tasklet and its scheduling
(CONFIG_HARDIRQS_SW_RESEND is not used anymore).
I only tested an equivalent patch for linux-2.6.22.18 on powerpc
for a board with an MPC8555E (using a portC line on the CPM2 PIC),
and build-tested this one for x86.
[2/2] updates Documentation/DocBook/genericirq.tmpl, taking
into account 1/2 other previous undocumented changes to genirq.
History:
[v2] - Don't touch archs Kconfig and defconfigs
(CONFIG_HARDIRQS_SW_RESEND is left unused)
- Don't make unrelated change to a comment in
arch/x86/kernel/apic/io_apic.c
- (hopefully) Better commit message for [1/2]
- Correct spelling errors in [2/2]
[v1] - initial patch
Guillaume Knispel (2):
genirq: reliably replay pending edge-triggered irq
genirq: update doc
Documentation/DocBook/genericirq.tmpl | 73 ++++++++++++++++++--------------
kernel/irq/resend.c | 6 ---
2 files changed, 41 insertions(+), 38 deletions(-)
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCHv2 1/2] genirq: reliably replay pending edge-triggered irq
2010-05-04 0:19 [PATCHv2 0/2] genirq: reliably replay pending edge-triggered irq (plus doc) Guillaume Knispel
@ 2010-05-04 0:21 ` Guillaume Knispel
2010-05-04 0:23 ` [PATCHv2 2/2] genirq: update doc Guillaume Knispel
1 sibling, 0 replies; 3+ messages in thread
From: Guillaume Knispel @ 2010-05-04 0:21 UTC (permalink / raw)
To: linux-kernel, Linuxppc-dev
Cc: Randy Dunlap, Lars-Peter Clausen, Russell King,
Bartlomiej Zolnierkiewicz, Haavard Skinnemoen, Guillaume Knispel,
Peter Zijlstra, Ingo Molnar, Linus Torvalds, Thomas Gleixner
The following can happen:
CPU 1 CPU 2
disable_irq(): handle_edge_irq():
LOCK desc->lock (irqsave)
desc->status |= IRQ_DISABLED;
desc->chip->disable(irq);/*1*/
UNLOCK desc->lock (irqrestore)
LOCK desc->lock
desc->status |= (IRQ_PENDING
| IRQ_MASKED);
mask_ack_irq(desc, irq);
UNLOCK desc->lock
NOTE /*1*/: ->disable can point to default_disable().
Since commit:
76d2160147f43f982dfe881404cfde9fd0a9da21
genirq: do not mask interrupts by default
the delayed interrupt disable mechanism has been activated for every
user of default_disable() -- which used to mask the interrupt at
controller level before and is now a noop. The sequence describing a
race above will now indeed happen if an interrupt event occurs at any
time between the effective disable_irq() and the next effective
enable_irq().
Also note that even if ->disable does a masking, a similar race
can indeed happen even on a monoprocessor system if an interrupt event
occurs before just before the masking.
In order to avoid interrupt loss, an IRQ_PENDING interrupt must be
replayed when enable_irq() is called (or immediately after).
This replay (implemented in kernel/irq/resend.c) used to be reliable
only if:
* the interrupt controller driver implements a reliable retrigger()
callback
or
* CONFIG_HARDIRQS_SW_RESEND is defined (in this case the flow handler
can be executed in a tasklet running resend_irqs() )
So CONFIG_HARDIRQS_SW_RESEND was meant to be set on plateforms where it
exists a risk that edge interrupts are used on an interrupt controller
that does not support hard retrigger (or at least not reliably).
But CONFIG_HARDIRQS_SW_RESEND was only defined on arm and avr32
architectures, and other architectures exist which can have controllers
without a reliable retrigger(). Some examples:
* arch/powerpc/sysdev/cpm2.c arch/powerpc/sysdev/ipic.c
* arch/blackfin/mach-common/ints-priority.c
* arch/mips/alchemy/common/irq.c
* ...
With the present change, resend_irqs() is unconditionally built, so
that edge-triggered interrupts can not be lost.
The CONFIG_HARDIRQS_SW_RESEND option is not used anymore.
See http://lkml.org/lkml/2010/4/19/129 for the first discussion about
this problem.
Signed-off-by: Guillaume Knispel <gknispel@proformatique•com>
CC: linux-kernel@vger•kernel.org
CC: Linuxppc-dev@lists•ozlabs.org
CC: Bartlomiej Zolnierkiewicz <bzolnier@gmail•com>
CC: Benjamin Herrenschmidt <benh@kernel•crashing.org>
CC: Haavard Skinnemoen <hskinnemoen@atmel•com>
CC: Ingo Molnar <mingo@elte•hu>
CC: Lars-Peter Clausen <lars@metafoo•de>
CC: Linus Torvalds <torvalds@linux-foundation•org>
CC: Peter Zijlstra <peterz@infradead•org>
CC: Randy Dunlap <randy.dunlap@oracle•com>
CC: Russell King <linux@arm•linux.org.uk>
CC: Thomas Gleixner <tglx@linutronix•de>
---
kernel/irq/resend.c | 6 ------
1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/kernel/irq/resend.c b/kernel/irq/resend.c
index 090c376..3b20ce1 100644
--- a/kernel/irq/resend.c
+++ b/kernel/irq/resend.c
@@ -20,8 +20,6 @@
#include "internals.h"
-#ifdef CONFIG_HARDIRQS_SW_RESEND
-
/* Bitmap to handle software resend of interrupts: */
static DECLARE_BITMAP(irqs_resend, NR_IRQS);
@@ -46,8 +44,6 @@ static void resend_irqs(unsigned long arg)
/* Tasklet to handle resend: */
static DECLARE_TASKLET(resend_tasklet, resend_irqs, 0);
-#endif
-
/*
* IRQ resend
*
@@ -71,11 +67,9 @@ void check_irq_resend(struct irq_desc *desc, unsigned int irq)
desc->status = (status & ~IRQ_PENDING) | IRQ_REPLAY;
if (!desc->chip->retrigger || !desc->chip->retrigger(irq)) {
-#ifdef CONFIG_HARDIRQS_SW_RESEND
/* Set it pending and activate the softirq: */
set_bit(irq, irqs_resend);
tasklet_schedule(&resend_tasklet);
-#endif
}
}
}
--
1.6.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCHv2 2/2] genirq: update doc
2010-05-04 0:19 [PATCHv2 0/2] genirq: reliably replay pending edge-triggered irq (plus doc) Guillaume Knispel
2010-05-04 0:21 ` [PATCHv2 1/2] genirq: reliably replay pending edge-triggered irq Guillaume Knispel
@ 2010-05-04 0:23 ` Guillaume Knispel
1 sibling, 0 replies; 3+ messages in thread
From: Guillaume Knispel @ 2010-05-04 0:23 UTC (permalink / raw)
To: linux-kernel, Linuxppc-dev
Cc: Randy Dunlap, Lars-Peter Clausen, Russell King,
Bartlomiej Zolnierkiewicz, Haavard Skinnemoen, Guillaume Knispel,
Peter Zijlstra, Ingo Molnar, Linus Torvalds, Thomas Gleixner
Following "genirq: always build resend_irqs" and previous changes, this
commit updates the genirq DocBook.
Signed-off-by: Guillaume Knispel <gknispel@proformatique•com>
CC: linux-kernel@vger•kernel.org
CC: Linuxppc-dev@lists•ozlabs.org
CC: Bartlomiej Zolnierkiewicz <bzolnier@gmail•com>
CC: Benjamin Herrenschmidt <benh@kernel•crashing.org>
CC: Haavard Skinnemoen <hskinnemoen@atmel•com>
CC: Ingo Molnar <mingo@elte•hu>
CC: Lars-Peter Clausen <lars@metafoo•de>
CC: Linus Torvalds <torvalds@linux-foundation•org>
CC: Randy Dunlap <randy.dunlap@oracle•com>
CC: Peter Zijlstra <peterz@infradead•org>
CC: Russell King <linux@arm•linux.org.uk>
CC: Thomas Gleixner <tglx@linutronix•de>
---
Documentation/DocBook/genericirq.tmpl | 73 ++++++++++++++++++--------------
1 files changed, 41 insertions(+), 32 deletions(-)
diff --git a/Documentation/DocBook/genericirq.tmpl b/Documentation/DocBook/genericirq.tmpl
index 1448b33..7df5274 100644
--- a/Documentation/DocBook/genericirq.tmpl
+++ b/Documentation/DocBook/genericirq.tmpl
@@ -240,8 +240,6 @@ default_enable(irq)
default_disable(irq)
{
- if (!delay_disable(irq))
- desc->chip->mask(irq);
}
default_ack(irq)
@@ -264,7 +262,11 @@ noop(irq)
}
</programlisting>
- </para>
+ Note: default_disable() is empty so that an edge-triggered
+ interrupt happening between a disable_irq() and an enable_irq()
+ is caught by the kernel for later replay. See
+ <xref linkend="Delayed_interrupt_disable"/> for details.
+ </para>
</sect3>
</sect2>
<sect2 id="Default_flow_handler_implementations">
@@ -278,9 +280,14 @@ noop(irq)
<para>
The following control flow is implemented (simplified excerpt):
<programlisting>
-desc->chip->start();
+desc->chip->mask_ack();
+if (desc->status & inprogress)
+ return;
+desc->status |= inprogress;
handle_IRQ_event(desc->action);
-desc->chip->end();
+desc->status &= inprogress;
+if (!(desc->status & disabled))
+ desc->chip->unmask_ack();
</programlisting>
</para>
</sect3>
@@ -293,21 +300,23 @@ desc->chip->end();
<para>
The following control flow is implemented (simplified excerpt):
<programlisting>
-if (desc->status & running) {
- desc->chip->hold();
+if (desc->status & (inprogress | disabled)) {
desc->status |= pending | masked;
+ desc->chip->mask_ack();
return;
}
-desc->chip->start();
-desc->status |= running;
+desc->chip->ack();
+desc->status |= inprogress;
do {
- if (desc->status & masked)
- desc->chip->enable();
+ if ((desc->status & (pending | masked | disabled))
+ == (pending | masked)) {
+ desc->chip->unmask();
+ desc->status &= ~masked;
+ }
desc->status &= ~pending;
handle_IRQ_event(desc->action);
-} while (status & pending);
-desc->status &= ~running;
-desc->chip->end();
+} while ((status & (pending | disabled)) == pending);
+desc->status &= inprogress;
</programlisting>
</para>
</sect3>
@@ -342,9 +351,9 @@ handle_IRQ_event(desc->action);
<para>
The following control flow is implemented (simplified excerpt):
<programlisting>
-desc->chip->start();
+desc->chip->ack();
handle_IRQ_event(desc->action);
-desc->chip->end();
+desc->chip->eoi();
</programlisting>
</para>
</sect3>
@@ -361,22 +370,20 @@ desc->chip->end();
<sect2 id="Delayed_interrupt_disable">
<title>Delayed interrupt disable</title>
<para>
- This per interrupt selectable feature, which was introduced by Russell
- King in the ARM interrupt implementation, does not mask an interrupt
- at the hardware level when disable_irq() is called. The interrupt is
- kept enabled and is masked in the flow handler when an interrupt event
- happens. This prevents losing edge interrupts on hardware which does
- not store an edge interrupt event while the interrupt is disabled at
- the hardware level. When an interrupt arrives while the IRQ_DISABLED
- flag is set, then the interrupt is masked at the hardware level and
- the IRQ_PENDING bit is set. When the interrupt is re-enabled by
- enable_irq() the pending bit is checked and if it is set, the
- interrupt is resent either via hardware or by a software resend
- mechanism. (It's necessary to enable CONFIG_HARDIRQS_SW_RESEND when
- you want to use the delayed interrupt disable feature and your
- hardware is not capable of retriggering an interrupt.)
- The delayed interrupt disable can be runtime enabled, per interrupt,
- by setting the IRQ_DELAYED_DISABLE flag in the irq_desc status field.
+ At least when using default_disable as the ->disable() callback of a
+ chip handler, an interrupt is not masked at the hardware level when
+ disable_irq() is called. The interrupt is kept enabled and is masked
+ in the flow handler when an interrupt event happens. This prevents
+ losing edge interrupts on hardware which does not store an edge
+ interrupt event while the interrupt is disabled at the hardware level.
+ This makes use of the replay mechanism that is mandatory anyway to
+ prevent potential race conditions between an interrupt event and a call
+ to disable_irq() at the same time. When an interrupt arrives in
+ handle_edge_irq() while the IRQ_DISABLED flag is set, then the
+ interrupt is masked at the hardware level and the IRQ_PENDING bit is
+ set. When the interrupt is re-enabled by enable_irq() the pending bit
+ is checked and if it is set, the interrupt is resent either via
+ hardware or by a software resend mechanism.
</para>
</sect2>
</sect1>
@@ -387,6 +394,8 @@ desc->chip->end();
contains all the direct chip relevant functions, which
can be utilized by the irq flow implementations.
<itemizedlist>
+ <listitem><para>disable() - Optional</para></listitem>
+ <listitem><para>enable() - Optional</para></listitem>
<listitem><para>ack()</para></listitem>
<listitem><para>mask_ack() - Optional, recommended for performance</para></listitem>
<listitem><para>mask()</para></listitem>
--
1.6.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-05-04 0:23 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-04 0:19 [PATCHv2 0/2] genirq: reliably replay pending edge-triggered irq (plus doc) Guillaume Knispel
2010-05-04 0:21 ` [PATCHv2 1/2] genirq: reliably replay pending edge-triggered irq Guillaume Knispel
2010-05-04 0:23 ` [PATCHv2 2/2] genirq: update doc Guillaume Knispel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox