[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20070130075325.GA591@elte.hu>
Date: Tue, 30 Jan 2007 08:53:25 +0100
From: Ingo Molnar <mingo@...e.hu>
To: Jeff Garzik <jeff@...zik.org>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,
Stephen Hemminger <shemminger@...ux-foundation.org>,
Thomas Gleixner <tglx@...utronix.de>,
Andrew Morton <akpm@...l.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: Linux 2.6.20-rc6 - sky2 resume breakage
* Jeff Garzik <jeff@...zik.org> wrote:
> Ingo Molnar wrote:
> >i'm wondering, could we go with Thomas' temporary patch that disables
> >sky2 MSI if CONFIG_PM is enabled - we could revert that after 2.6.20.
> >It's not like MSI is a life and death feature. On IO-APIC systems
> >vectors are abundant and in any case we share irqs just fine. The
> >true advantage of MSI is minimal. (MSI-X has the potential to be
> >better by being message based, but in reality it still goes through
> >the full IRQ layer.) MSI might be useful on really, really large
> >systems - but i really hope those really large systems dont rely on
> >CONFIG_PM. Meanwhile Thomas' patch maximizes the amount of working
> >hardware (it has the chance to produce working systems in 100% of the
> >cases) - which is a few orders of magnitude more important than IRQ
> >management micro-costs. Am i missing anything?
>
>
> Sharing irqs /sucks/. I routinely have to fight a USB device dying,
> because the ATA device is causing an interrupt storm, or vice versa.
> /Very/ common headache.
Yeah. Admittedly, ATA is very special because it is still edge-triggered
most of the time (for legacy reasons):
14: 389907 0 IO-APIC-edge ide0
so if it shares an irq with a device that has level-triggered
assumptions, those two dont intermix very well. That's why i have the
delayed-disable patches (see the two patches below), which will unify
the two methods, and the irq flow handling method will be mostly a
'performance hint' not a correctness issue. This has been in -rt for
quite a few weeks now and it works well.
btw., it would be great if you could help us here: could you perhaps,
from a past example, outline a specific case of such an ATA/USB IRQ
storm and how it occured (precisely) - and what the fix was? I'd like to
analyze a specific case to make sure the genirq layer recovers from such
cases more gracefully. In general, i think the IRQ subsystem needs to
become more failure-resilient and needs to become more auto-learning
(and these two dont stand in the way of good performance). This problem
of shared IRQs will be with us for at least another 10 years, if not
more. (for example ISA is /still/ not dead everywhere and it was already
legacy technology 15 years ago when Linux was started.)
Ingo
------------------->
Subject: irq: do not mask interrupts by default
From: Ingo Molnar <mingo@...e.hu>
never mask interrupts immediately upon request. Disabling interrupts in
high-performance codepaths is rare, and on the other hand this change
could recover lost edges (or even other types of lost interrupts) by
conservatively only masking interrupts after they happen. (NOTE: with
this change the highlevel irq-disable code still soft-disables this IRQ
line - and if such an interrupt happens then the IRQ flow handler keeps
the IRQ masked.)
mark i8529A controllers as 'never loses an edge'.
Signed-off-by: Ingo Molnar <mingo@...e.hu>
---
arch/i386/kernel/i8259.c | 1 +
arch/x86_64/kernel/i8259.c | 1 +
kernel/irq/chip.c | 17 ++++++++++-------
3 files changed, 12 insertions(+), 7 deletions(-)
Index: linux/arch/i386/kernel/i8259.c
===================================================================
--- linux.orig/arch/i386/kernel/i8259.c
+++ linux/arch/i386/kernel/i8259.c
@@ -41,6 +41,7 @@ static void mask_and_ack_8259A(unsigned
static struct irq_chip i8259A_chip = {
.name = "XT-PIC",
.mask = disable_8259A_irq,
+ .disable = disable_8259A_irq,
.unmask = enable_8259A_irq,
.mask_ack = mask_and_ack_8259A,
};
Index: linux/arch/x86_64/kernel/i8259.c
===================================================================
--- linux.orig/arch/x86_64/kernel/i8259.c
+++ linux/arch/x86_64/kernel/i8259.c
@@ -103,6 +103,7 @@ static void mask_and_ack_8259A(unsigned
static struct irq_chip i8259A_chip = {
.name = "XT-PIC",
.mask = disable_8259A_irq,
+ .disable = disable_8259A_irq,
.unmask = enable_8259A_irq,
.mask_ack = mask_and_ack_8259A,
};
Index: linux/kernel/irq/chip.c
===================================================================
--- linux.orig/kernel/irq/chip.c
+++ linux/kernel/irq/chip.c
@@ -202,10 +202,6 @@ static void default_enable(unsigned int
*/
static void default_disable(unsigned int irq)
{
- struct irq_desc *desc = irq_desc + irq;
-
- if (!(desc->status & IRQ_DELAYED_DISABLE))
- desc->chip->mask(irq);
}
/*
@@ -270,13 +266,18 @@ handle_simple_irq(unsigned int irq, stru
if (unlikely(desc->status & IRQ_INPROGRESS))
goto out_unlock;
- desc->status &= ~(IRQ_REPLAY | IRQ_WAITING);
kstat_cpu(cpu).irqs[irq]++;
action = desc->action;
- if (unlikely(!action || (desc->status & IRQ_DISABLED)))
+ if (unlikely(!action || (desc->status & IRQ_DISABLED))) {
+ if (desc->chip->mask)
+ desc->chip->mask(irq);
+ desc->status &= ~(IRQ_REPLAY | IRQ_WAITING);
+ desc->status |= IRQ_PENDING;
goto out_unlock;
+ }
+ desc->status &= ~(IRQ_REPLAY | IRQ_WAITING | IRQ_PENDING);
desc->status |= IRQ_INPROGRESS;
spin_unlock(&desc->lock);
@@ -368,11 +369,13 @@ handle_fasteoi_irq(unsigned int irq, str
/*
* If its disabled or no action available
- * keep it masked and get out of here
+ * then mask it and get out of here:
*/
action = desc->action;
if (unlikely(!action || (desc->status & IRQ_DISABLED))) {
desc->status |= IRQ_PENDING;
+ if (desc->chip->mask)
+ desc->chip->mask(irq);
goto out;
}
---------------------->
Subject: genirq: remove IRQ_DISABLED
From: Ingo Molnar <mingo@...e.hu>
now that disable_irq() defaults to delayed-disable semantics, the
IRQ_DISABLED flag is not needed anymore.
Signed-off-by: Ingo Molnar <mingo@...e.hu>
---
arch/arm/kernel/irq.c | 3 +--
arch/i386/kernel/io_apic.c | 4 +---
arch/powerpc/platforms/powermac/pic.c | 2 --
arch/x86_64/kernel/io_apic.c | 4 +---
include/linux/irq.h | 7 +++----
5 files changed, 6 insertions(+), 14 deletions(-)
Index: linux/arch/arm/kernel/irq.c
===================================================================
--- linux.orig/arch/arm/kernel/irq.c
+++ linux/arch/arm/kernel/irq.c
@@ -159,8 +159,7 @@ void __init init_IRQ(void)
int irq;
for (irq = 0; irq < NR_IRQS; irq++)
- irq_desc[irq].status |= IRQ_NOREQUEST | IRQ_DELAYED_DISABLE |
- IRQ_NOPROBE;
+ irq_desc[irq].status |= IRQ_NOREQUEST | IRQ_NOPROBE;
#ifdef CONFIG_SMP
bad_irq_desc.affinity = CPU_MASK_ALL;
Index: linux/arch/i386/kernel/io_apic.c
===================================================================
--- linux.orig/arch/i386/kernel/io_apic.c
+++ linux/arch/i386/kernel/io_apic.c
@@ -1275,11 +1275,9 @@ static void ioapic_register_intr(int irq
trigger == IOAPIC_LEVEL)
set_irq_chip_and_handler_name(irq, &ioapic_chip,
handle_fasteoi_irq, "fasteoi");
- else {
- irq_desc[irq].status |= IRQ_DELAYED_DISABLE;
+ else
set_irq_chip_and_handler_name(irq, &ioapic_chip,
handle_edge_irq, "edge");
- }
set_intr_gate(vector, interrupt[irq]);
}
Index: linux/arch/powerpc/platforms/powermac/pic.c
===================================================================
--- linux.orig/arch/powerpc/platforms/powermac/pic.c
+++ linux/arch/powerpc/platforms/powermac/pic.c
@@ -305,8 +305,6 @@ static int pmac_pic_host_map(struct irq_
level = !!(level_mask[hw >> 5] & (1UL << (hw & 0x1f)));
if (level)
desc->status |= IRQ_LEVEL;
- else
- desc->status |= IRQ_DELAYED_DISABLE;
set_irq_chip_and_handler(virq, &pmac_pic, level ?
handle_level_irq : handle_edge_irq);
return 0;
Index: linux/arch/x86_64/kernel/io_apic.c
===================================================================
--- linux.orig/arch/x86_64/kernel/io_apic.c
+++ linux/arch/x86_64/kernel/io_apic.c
@@ -810,11 +810,9 @@ static void ioapic_register_intr(int irq
trigger == IOAPIC_LEVEL)
set_irq_chip_and_handler_name(irq, &ioapic_chip,
handle_fasteoi_irq, "fasteoi");
- else {
- irq_desc[irq].status |= IRQ_DELAYED_DISABLE;
+ else
set_irq_chip_and_handler_name(irq, &ioapic_chip,
handle_edge_irq, "edge");
- }
}
static void __init setup_IO_APIC_irq(int apic, int pin, int idx, int irq)
{
Index: linux/include/linux/irq.h
===================================================================
--- linux.orig/include/linux/irq.h
+++ linux/include/linux/irq.h
@@ -57,10 +57,9 @@ typedef void fastcall (*irq_flow_handler
#define IRQ_NOPROBE 0x00020000 /* IRQ is not valid for probing */
#define IRQ_NOREQUEST 0x00040000 /* IRQ cannot be requested */
#define IRQ_NOAUTOEN 0x00080000 /* IRQ will not be enabled on request irq */
-#define IRQ_DELAYED_DISABLE 0x00100000 /* IRQ disable (masking) happens delayed. */
-#define IRQ_WAKEUP 0x00200000 /* IRQ triggers system wakeup */
-#define IRQ_MOVE_PENDING 0x00400000 /* need to re-target IRQ destination */
-#define IRQ_NO_BALANCING 0x00800000 /* IRQ is excluded from balancing */
+#define IRQ_WAKEUP 0x00100000 /* IRQ triggers system wakeup */
+#define IRQ_MOVE_PENDING 0x00200000 /* need to re-target IRQ destination */
+#define IRQ_NO_BALANCING 0x00400000 /* IRQ is excluded from balancing */
#ifdef CONFIG_IRQ_PER_CPU
# define CHECK_IRQ_PER_CPU(var) ((var) & IRQ_PER_CPU)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists