linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@redhat.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Linus Torvalds <torvalds@osdl.org>,
	Komuro <komurojun-mbn@nifty.com>,
	tglx@linutronix.de, Adrian Bunk <bunk@stusta.de>,
	Andrew Morton <akpm@osdl.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: [patch] irq: do not mask interrupts by default
Date: Tue, 14 Nov 2006 09:14:00 +0100	[thread overview]
Message-ID: <1163492040.28401.76.camel@earth> (raw)
In-Reply-To: <m1bqnboxv5.fsf@ebiederm.dsl.xmission.com>

On Mon, 2006-11-13 at 14:11 -0700, Eric W. Biederman wrote:
> -       else
> +       else {
> +               irq_desc[irq].status |= IRQ_DELAYED_DISABLE;
>                 set_irq_chip_and_handler_name(irq, &ioapic_chip,
>                                               handle_edge_irq,
> "edge");
> +       }
>  } 

yeah. Komuro, could you try my patch below - Eric's patch only updates
x86_64 while your failure was on the i386 kernel.

Note, i also took another approach to fix this problem, that should
cover both the case found by Komuro, and some other cases as well. The
theory is this:

1) disable_irq() is relatively rare (used in about 10% of drivers, but
there it's overwhelmingly used in some slowpath) so it's performance
uncritical.

2) missing an IRQ while the line is masked is often a lethal regression
to the user. An IRQ could be missed even if we think that the IRQ line
is 'level-triggered'.

so my patch changes the default irq-disable logic of /all/ controllers
to "delayed disable". (IRQ chips can still override this by providing a
different chip->disable method that just clones their ->mask method, if
it is absolutely sure that no IRQs can be lost while masked)

So this patch has the worst-case effect of getting at most one 'extra'
interrupt after the IRQ line has been 'disabled' - at which point the
line will be masked for real (by the flow handler). (I updated the
fasteoi and the simple irq flow handlers to mask the IRQ for real if an
IRQ triggers and the line was disabled.)

It's a bit late in the -rc cycle for a change like this, but i'm fairly
positive about it. I booted it on a couple of boxes and saw no badness.
(neither did i see any increase in IRQ rates)

	Ingo

NOTE: this also means that the old IRQ_DELAYED_DISABLE bit can probably
be scrapped - i'll do that later on in a separate mail, if this patch
works out fine.

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

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/irq/chip.c |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

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);
 }
 
 /*
@@ -272,8 +268,11 @@ handle_simple_irq(unsigned int irq, stru
 	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);
 		goto out_unlock;
+	}
 
 	desc->status |= IRQ_INPROGRESS;
 	spin_unlock(&desc->lock);
@@ -366,11 +365,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;
 	}
 



  reply	other threads:[~2006-11-14  8:16 UTC|newest]

Thread overview: 91+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-11-08  2:33 Linux 2.6.19-rc5 Linus Torvalds
     [not found] ` <20061108085235.GT4729@stusta.de>
2006-11-08  9:29   ` [discuss] 2.6.19-rc5: known regressions Jan Beulich
2006-11-08 10:21     ` Adrian Bunk
2006-11-08  9:34   ` Jens Axboe
2006-11-08 19:09     ` Alex Romosan
2006-11-08 19:29       ` Jens Axboe
2006-11-08 19:38         ` Alex Romosan
2006-11-08 19:45           ` Jens Axboe
2006-11-08 21:40             ` Alex Romosan
2006-11-08 20:03         ` Arjan van de Ven
2006-11-08 20:19           ` Jens Axboe
2006-11-08 11:04   ` Eric W. Biederman
2006-11-08 11:32   ` Thomas Gleixner
     [not found]   ` <7813413.118221162987983254.komurojun-mbn@nifty.com>
2006-11-08 16:00     ` Linus Torvalds
2006-11-10 12:42       ` Re: Re: 2.6.19-rc5: known regressions :SMP kernel can not generate ISA irq Komuro
2006-11-13 16:02         ` Linus Torvalds
2006-11-13 17:11           ` Eric W. Biederman
2006-11-13 20:44             ` Ingo Molnar
2006-11-13 21:11               ` Eric W. Biederman
2006-11-14  8:14                 ` Ingo Molnar [this message]
2006-11-14  8:20                   ` [patch] irq: do not mask interrupts by default Arjan van de Ven
2006-11-14 12:43                   ` Komuro
2006-11-14 16:10                   ` Linus Torvalds
2006-11-14 17:52                     ` [PATCH] Use delayed disable mode of ioapic edge triggered interrupts Eric W. Biederman
2006-11-14 23:35                       ` Linus Torvalds
2006-11-15  1:17                       ` Linus Torvalds
2006-11-15  5:14                         ` Eric W. Biederman
2006-11-15 16:06                           ` Linus Torvalds
2006-11-15 16:58                             ` Eric W. Biederman
2006-11-15 12:40                       ` Komuro
     [not found]                     ` <20061115090427.GA16173@elte.hu>
2006-11-15 16:13                       ` [patch] genirq: do not mask interrupts by default Linus Torvalds
2006-11-15 17:46                         ` Ingo Molnar
     [not found]   ` <m1y7qm425l.fsf@ebiederm.dsl.xmission.com>
     [not found]     ` <Pine.LNX.4.64.0611080745150.3667@g5.osdl.org>
2006-11-08 16:22       ` 2.6.19-rc5: known regressions Adrian Bunk
2006-11-08 23:11         ` Tim Chen
2006-11-09  2:49           ` Tim Chen
2006-11-09  5:10             ` Eric W. Biederman
2006-11-13 22:46               ` Tim Chen
2006-11-14  0:03                 ` Eric W. Biederman
2006-11-08  9:43 ` Linux 2.6.19-rc5 Nigel Cunningham
2006-11-08  9:59   ` Alessandro Suardi
2006-11-08 10:04     ` Nigel Cunningham
2006-11-08 14:19     ` Gene Heskett
2006-11-08 15:43   ` Linus Torvalds
     [not found] ` <20061111015035.GU4729@stusta.de>
2006-11-11  9:08   ` [discuss] 2.6.19-rc5: known regressions (v2) Rafael J. Wysocki
2006-11-11  9:25     ` Paolo Ornati
2006-11-11 10:49       ` Rafael J. Wysocki
2006-11-11 12:29         ` Paolo Ornati
2006-11-14 16:44           ` Paolo Ornati
2006-11-29 10:10             ` [SOLVED] " Paolo Ornati
2006-11-13 22:14 ` 2.6.19-rc5: known regressions with patches Adrian Bunk
2006-11-13 22:56   ` Brian King
2006-11-13 23:15     ` Linus Torvalds
2006-11-14  2:35       ` Jeff Garzik
2006-11-15 10:21 ` 2.6.19-rc5: known regressions (v3) Adrian Bunk
2006-11-15 10:35   ` Jens Axboe
2006-11-15 10:53     ` Adrian Bunk
2006-11-15 10:35   ` Eric Dumazet
2006-11-15 10:50     ` Andi Kleen
2006-11-15 16:40       ` William Cohen
2006-11-15 16:48         ` [discuss] " Andi Kleen
2006-11-15 18:39           ` Andrew Morton
2006-11-15 18:45             ` Andi Kleen
2006-11-15 19:07               ` Linus Torvalds
2006-11-15 19:23                 ` Andi Kleen
2006-11-15 20:21                   ` Andrew Morton
2006-11-15 21:18                     ` Eric W. Biederman
2006-11-15 21:31                       ` Andrew Morton
2006-11-16 10:55                         ` Mikael Pettersson
2006-11-16 20:23                           ` Andrew Morton
2006-11-17  9:59                             ` Mikael Pettersson
2006-11-17 10:13                               ` Andrew Morton
2006-11-19  3:05                                 ` Bill Davidsen
2006-11-17 10:29                               ` Andi Kleen
2006-11-16  3:21                     ` Andi Kleen
2006-11-16  5:05                       ` Andrew Morton
2006-11-16  7:04                         ` Andi Kleen
2006-11-16 15:34                           ` William Cohen
2006-11-16 15:47                             ` Andi Kleen
2006-11-16 21:32                             ` Stephane Eranian
2006-11-22 10:28     ` Eric Dumazet
2006-11-22 10:36       ` Andi Kleen
2006-11-22 18:42         ` Andrew Morton
2006-12-16 11:20           ` Ray Lee
2006-11-22 17:59       ` William Cohen
2006-11-22 18:05       ` William Cohen
2006-11-22 18:26         ` Eric Dumazet
2006-11-15 11:06   ` Brice Goglin
2006-11-15 22:32     ` Adrian Bunk
2006-11-15 12:07   ` Alan
2006-11-15 15:52   ` Stephen Hemminger
2006-11-15 16:35     ` Eric W. Biederman

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=1163492040.28401.76.camel@earth \
    --to=mingo@redhat.com \
    --cc=akpm@osdl.org \
    --cc=bunk@stusta.de \
    --cc=ebiederm@xmission.com \
    --cc=komurojun-mbn@nifty.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@osdl.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 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).