linux-kernel - [patch] irq: do not mask interrupts by default

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1163492040.28401.76.camel@earth>
Date:	Tue, 14 Nov 2006 09:14:00 +0100
From:	Ingo Molnar <mingo@...hat.com>
To:	"Eric W. Biederman" <ebiederm@...ssion.com>
Cc:	Linus Torvalds <torvalds@...l.org>,
	Komuro <komurojun-mbn@...ty.com>, tglx@...utronix.de,
	Adrian Bunk <bunk@...sta.de>, Andrew Morton <akpm@...l.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: [patch] irq: do not mask interrupts by default

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

Signed-off-by: Ingo Molnar <mingo@...e.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;
 	}

-
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/