LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Jeff Garzik <jeff@garzik.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Stephen Hemminger <shemminger@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Andrew Morton <akpm@osdl.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: Linux 2.6.20-rc6 - sky2 resume breakage
Date: Tue, 30 Jan 2007 08:53:25 +0100	[thread overview]
Message-ID: <20070130075325.GA591@elte.hu> (raw)
In-Reply-To: <45BEF61F.7000400@garzik.org>


* Jeff Garzik <jeff@garzik.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@elte.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@elte.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@elte.hu>

now that disable_irq() defaults to delayed-disable semantics, the 
IRQ_DISABLED flag is not needed anymore.

Signed-off-by: Ingo Molnar <mingo@elte.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)

  reply	other threads:[~2007-01-30  7:54 UTC|newest]

Thread overview: 84+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-01-25  2:58 Linux 2.6.20-rc6 Linus Torvalds
2007-01-25 10:09 ` Sunil Naidu
2007-01-25 11:10 ` Linux 2.6.20-rc6 - build failure Eyal Lebedinsky
2007-01-26  2:22   ` Eyal Lebedinsky
2007-01-26 18:49     ` [2.6 patch] fix OCFS2 compile error Adrian Bunk
2007-01-26 19:47       ` Mark Fasheh
2007-01-26 19:53         ` Adrian Bunk
2007-01-26 18:46   ` Linux 2.6.20-rc6 - build failure Mark Fasheh
2007-01-25 17:50 ` Linux 2.6.20-rc6 Arkadiusz Patyk
2007-01-25 21:05 ` Michal Piotrowski
2007-01-25 21:12   ` David Miller
2007-01-26 16:52     ` Venkat Yekkirala
2007-01-26 18:10 ` 2.6.20-rc6: known unfixed regressions (part 1) Adrian Bunk
2007-01-26 18:11 ` 2.6.20-rc6: known unfixed regressions (part 2) Adrian Bunk
2007-01-26 18:16   ` Malte Schröder
2007-01-27 17:28     ` Adrian Bunk
2007-01-27 17:39       ` Adrian Bunk
2007-01-27 17:58       ` Linus Torvalds
2007-01-26 19:04   ` Michal Piotrowski
2007-01-26 19:08     ` Venkat Yekkirala
2007-01-26 18:18 ` 2.6.20-rc6: known regressions with patches Adrian Bunk
2007-01-29  8:45   ` Ingo Molnar
2007-01-29 12:58     ` Dave Jones
2007-01-27 17:32 ` 2.6.20-rc6: known unfixed regressions (v2) (part 1) Adrian Bunk
2007-01-27 17:42 ` 2.6.20-rc6: known unfixed regressions (v2) (part 2) Adrian Bunk
2007-01-28 13:33   ` Uwe Bugla
2007-01-29  6:26     ` Mike Galbraith
2007-01-29  6:48       ` Andrew Morton
2007-01-29  7:08         ` Mike Galbraith
2007-01-29  7:13           ` Linus Torvalds
2007-01-27 17:44 ` 2.6.20-rc6: known regressions with patches (v2) Adrian Bunk
2007-01-27 20:47 ` Linux 2.6.20-rc6 - supend lockdep warning Thomas Gleixner
2007-01-27 20:55 ` Linux 2.6.20-rc6 - sky2 resume breakage Thomas Gleixner
2007-01-29 19:31   ` Stephen Hemminger
2007-01-29 20:10     ` Thomas Gleixner
2007-01-29 21:38       ` Stephen Hemminger
2007-01-29 22:23         ` Thomas Gleixner
2007-01-29 22:23           ` Stephen Hemminger
2007-01-29 22:31             ` Thomas Gleixner
2007-01-29 22:37           ` Linus Torvalds
2007-01-29 22:40             ` Stephen Hemminger
2007-01-29 23:04               ` Linus Torvalds
2007-01-29 23:45                 ` Stephen Hemminger
2007-01-30  0:12                   ` Linus Torvalds
2007-01-30  0:16                     ` Stephen Hemminger
2007-01-30  0:25                       ` Linus Torvalds
2007-01-30  0:26                         ` Stephen Hemminger
2007-01-30  6:54                         ` Ingo Molnar
2007-01-30  7:39                           ` Jeff Garzik
2007-01-30  7:53                             ` Ingo Molnar [this message]
2007-01-30  8:02                               ` Jeff Garzik
2007-01-30  8:08                                 ` Ingo Molnar
2007-01-30  8:13                                   ` Ingo Molnar
2007-01-31 15:27                                   ` Jeff Garzik
2007-01-31 17:38                                     ` Ingo Molnar
2007-01-31 17:52                                       ` Jeff Garzik
2007-01-31 20:13                                         ` Thomas Gleixner
2007-01-30  8:03                             ` Ingo Molnar
2007-02-01  6:15                             ` [LIBATA BUG] sr.c: TEST_UNIT_READY error Conke Hu
2007-02-07 12:40                               ` Jeff Garzik
2007-02-02  5:48                                 ` Conke Hu
2007-02-13  7:30                                   ` Conke Hu
2007-02-15  6:30                                     ` Conke Hu
2007-01-30  8:57                     ` Linux 2.6.20-rc6 - sky2 resume breakage Len Brown
2007-01-30 16:01                       ` Rafael J. Wysocki
2007-01-30 21:28                         ` Nigel Cunningham
2007-02-01 12:49                       ` Pavel Machek
2007-01-29 23:42             ` [PATCH] sky2: fix MSI related " Thomas Gleixner
2007-01-29 22:38           ` Linux 2.6.20-rc6 - sky2 " Frédéric Riss
2007-01-29 22:45             ` Thomas Gleixner
2007-01-29 22:50               ` Frédéric Riss
2007-01-29 22:57                 ` Thomas Gleixner
2007-01-29 23:26                   ` Frédéric Riss
2007-01-29 23:37                     ` Thomas Gleixner
2007-01-29 23:50                       ` [PATCH] block MSI on Sony Stephen Hemminger
2007-01-30  0:22                         ` Thomas Gleixner
2007-01-30  0:21                           ` Stephen Hemminger
2007-01-30  0:31                             ` Thomas Gleixner
2007-01-30  0:31                               ` Stephen Hemminger
2007-01-30  0:26                           ` Thomas Gleixner
2007-01-27 22:11 ` Linux 2.6.20-rc6 - suspend / resume ata_piix Thomas Gleixner
2007-01-27 22:40   ` Jeff Garzik
2007-01-27 22:44     ` Thomas Gleixner
2007-01-28 22:05       ` Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070130075325.GA591@elte.hu \
    --to=mingo@elte.hu \
    --cc=akpm@osdl.org \
    --cc=jeff@garzik.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=shemminger@linux-foundation.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --subject='Re: Linux 2.6.20-rc6 - sky2 resume breakage' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).