[PATCHv2 2/2] genirq: update doc

Previous thread: by Western Union Transfer Award on Monday, May 3, 2010 - 4:32 pm. (1 message)

Next thread: [PATCH] mtd: nand_base: Extend NAND flash detection to new MLC chips by Kevin Cernekee on Monday, May 3, 2010 - 5:46 pm. (2 messages)
From: Guillaume Knispel
Date: Monday, May 3, 2010 - 5:19 pm

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(-)
--

From: Guillaume Knispel
Date: Monday, May 3, 2010 - 5:21 pm

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 ...
From: Guillaume Knispel
Date: Monday, May 3, 2010 - 5:23 pm

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();
 ...
Previous thread: by Western Union Transfer Award on Monday, May 3, 2010 - 4:32 pm. (1 message)

Next thread: [PATCH] mtd: nand_base: Extend NAND flash detection to new MLC chips by Kevin Cernekee on Monday, May 3, 2010 - 5:46 pm. (2 messages)