All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Alan Cox <alan@lxorguk.ukuu.org.uk>, Andrew Morton <akpm@osdl.org>
Cc: arjan@linux.intel.com, linux-kernel@vger.kernel.org
Subject: [patch, -rc5-mm3] fix irqpoll some more
Date: Mon, 5 Jun 2006 10:49:38 +0200	[thread overview]
Message-ID: <20060605084938.GA31915@elte.hu> (raw)
In-Reply-To: <1149497459.23209.39.camel@localhost.localdomain>


* Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> Ar Sul, 2006-06-04 am 23:00 -0700, ysgrifennodd akpm@osdl.org:
> > irqpoll/irqfixup ignored IRQ_DISABLED but that could cause lockups.  So
> > listen to desc->depth to correctly honor disable_irq().  Also, when an
> > interrupt it screaming, set IRQ_DISABLED but do not touch ->depth.
> > 
> > Signed-off-by: Ingo Molnar <mingo@elte.hu>
> > Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
> 
> NAK
> 
> The moment you do this you as good as lose irqpoll support because the 
> IRQ being disabled is unrelated to the IRQ delivery line when dealing 
> with faults of this nature.

hm, agreed. I really sent it more as an RFC. The real fix is to do the 
disable_irq_handler()/enable_irq_handler() thing. [which is also a much 
nicer interface for more advanced MSI concepts]

> There is a saner fix - walk all the IRQs that are not disabled and 
> only if that produces no claim (ie the box is about to die otherwise) 
> then walk the disabled ones. Its a slow path at that point in error 
> recovery so the performance isn't critical.

yeah. How about the patch below? [builds and boots on x86, but no real 
irqpoll testing was done as i dont have such a problem-system.]

	Ingo

---------------
Subject: fix irqpoll some more
From: Ingo Molnar <mingo@elte.hu>

implement Alan's suggestion of irqpoll doing a second pass over
disabled irq lines if we didnt manage to handle the screaming
interrupt.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/irq/spurious.c |   24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

Index: linux/kernel/irq/spurious.c
===================================================================
--- linux.orig/kernel/irq/spurious.c
+++ linux/kernel/irq/spurious.c
@@ -18,8 +18,9 @@ static int irqfixup __read_mostly;
  */
 static int misrouted_irq(int irq, struct pt_regs *regs)
 {
-	int i, ok = 0, work = 0;
+	int i, ok = 0, work = 0, first_pass = 1;
 
+repeat:
 	for (i = 1; i < NR_IRQS; i++) {
 		struct irq_desc *desc = irq_desc + i;
 		struct irqaction *action;
@@ -61,7 +62,7 @@ static int misrouted_irq(int irq, struct
 		 * IRQ_DISABLED - which might have been set due to
 		 * a screaming interrupt.
 		 */
-		if (desc->depth) {
+		if (first_pass && desc->depth) {
 			spin_unlock(&desc->lock);
 			continue;
 		}
@@ -90,6 +91,25 @@ static int misrouted_irq(int irq, struct
 			desc->chip->end(i);
 		spin_unlock(&desc->lock);
 	}
+	/*
+	 * HACK:
+	 *
+	 * In the first pass we dont touch handlers that are behind
+	 * a disabled IRQ line. In the second pass (having no other
+	 * choice) we ignore the disabled state of IRQ lines. We've
+	 * got a screaming interrupt, so we have the choice between
+	 * a real lockup happening due to that screaming interrupt,
+	 * against a theoretical locking that becomes possible if we
+	 * ignore a disabled IRQ line.
+	 *
+	 * FIXME: proper disable_irq_handler() API would remove the
+	 * need for this hack.
+	 */
+	if (!ok && first_pass) {
+		first_pass = 0;
+		goto repeat;
+	}
+
 	/* So the caller can adjust the irq error counts */
 	return ok;
 }

       reply	other threads:[~2006-06-05  8:50 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <200606050600.k5560GdU002338@shell0.pdx.osdl.net>
     [not found] ` <1149497459.23209.39.camel@localhost.localdomain>
2006-06-05  8:49   ` Ingo Molnar [this message]
2006-06-05 13:05     ` [patch, -rc5-mm3] fix irqpoll some more Alan Cox
2006-06-05 12:52       ` Ingo Molnar
2006-06-06 13:25     ` Steven Rostedt
2006-06-06 13:53       ` Ingo Molnar
2006-06-06 14:40         ` Steven Rostedt
2006-06-06 14:59           ` Ingo Molnar
2006-06-06 15:32             ` Steven Rostedt

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=20060605084938.GA31915@elte.hu \
    --to=mingo@elte.hu \
    --cc=akpm@osdl.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=arjan@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.