All of lore.kernel.org
 help / color / mirror / Atom feed
From: Russ Anderson <rja@sgi.com>
To: LKML <linux-kernel@vger.kernel.org>,
	akpm@linux-foundation.org, linux-ia64@vger.kernel.org
Subject: [patch] __do_IRQ does not check IRQ_DISABLED when IRQ_PER_CPU is set
Date: Tue, 30 Oct 2007 16:26:57 +0000	[thread overview]
Message-ID: <20071030162657.GA21728@sgi.com> (raw)

[patch] __do_IRQ does not check IRQ_DISABLED when IRQ_PER_CPU is set

In __do_IRQ(), the normal case is that IRQ_DISABLED is checked and if
set the handler (handle_IRQ_event()) is not called.  

Earlier in __do_IRQ(), if IRQ_PER_CPU is set the code does not check
IRQ_DISABLED and calls the handler even though IRQ_DISABLED is set.
This behavior seems unintentional.

One user encountering this behavior is the CPE handler (in 
arch/ia64/kernel/mca.c).  When the CPE handler encounters too many
CPEs (such as a solid single bit error), it sets up a polling timer
and disables the CPE interrupt (to avoid excessive overhead logging
the stream of single bit errors).  disable_irq_nosync() is called
which sets IRQ_DISABLED.  The IRQ_PER_CPU flag was previously set
(in ia64_mca_late_init()).  The net result is the CPE handler gets
called even though it is marked disabled.

If the behavior of not checking IRQ_DISABLED when IRQ_PER_CPU is
set is intentional, it would be worthy of a comment describing 
the intended behavior.  disable_irq_nosync() does call chip->disable()
to provide a chipset specifiec interface for disabling the interrupt,
which avoids this issue when used.

Comments???

Signed-off-by: Russ Anderson (rja@sgi.com)

--------------------------------------------------------------------
---
 kernel/irq/handle.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Index: linus/kernel/irq/handle.c
=================================--- linus.orig/kernel/irq/handle.c	2007-10-30 09:49:26.000000000 -0500
+++ linus/kernel/irq/handle.c	2007-10-30 10:23:52.436719688 -0500
@@ -178,9 +178,11 @@ fastcall unsigned int __do_IRQ(unsigned 
 		 */
 		if (desc->chip->ack)
 			desc->chip->ack(irq);
-		action_ret = handle_IRQ_event(irq, desc->action);
-		if (!noirqdebug)
-			note_interrupt(irq, desc, action_ret);
+		if (likely(!(desc->status & IRQ_DISABLED))) {
+			action_ret = handle_IRQ_event(irq, desc->action);
+			if (!noirqdebug)
+				note_interrupt(irq, desc, action_ret);
+		}
 		desc->chip->end(irq);
 		return 1;
 	}
-- 
Russ Anderson, OS RAS/Partitioning Project Lead  
SGI - Silicon Graphics Inc          rja@sgi.com

WARNING: multiple messages have this Message-ID (diff)
From: Russ Anderson <rja@sgi.com>
To: LKML <linux-kernel@vger.kernel.org>,
	akpm@linux-foundation.org, linux-ia64@vger.kernel.org
Subject: [patch] __do_IRQ does not check IRQ_DISABLED when IRQ_PER_CPU is set
Date: Tue, 30 Oct 2007 11:26:57 -0500	[thread overview]
Message-ID: <20071030162657.GA21728@sgi.com> (raw)

[patch] __do_IRQ does not check IRQ_DISABLED when IRQ_PER_CPU is set

In __do_IRQ(), the normal case is that IRQ_DISABLED is checked and if
set the handler (handle_IRQ_event()) is not called.  

Earlier in __do_IRQ(), if IRQ_PER_CPU is set the code does not check
IRQ_DISABLED and calls the handler even though IRQ_DISABLED is set.
This behavior seems unintentional.

One user encountering this behavior is the CPE handler (in 
arch/ia64/kernel/mca.c).  When the CPE handler encounters too many
CPEs (such as a solid single bit error), it sets up a polling timer
and disables the CPE interrupt (to avoid excessive overhead logging
the stream of single bit errors).  disable_irq_nosync() is called
which sets IRQ_DISABLED.  The IRQ_PER_CPU flag was previously set
(in ia64_mca_late_init()).  The net result is the CPE handler gets
called even though it is marked disabled.

If the behavior of not checking IRQ_DISABLED when IRQ_PER_CPU is
set is intentional, it would be worthy of a comment describing 
the intended behavior.  disable_irq_nosync() does call chip->disable()
to provide a chipset specifiec interface for disabling the interrupt,
which avoids this issue when used.

Comments???

Signed-off-by: Russ Anderson (rja@sgi.com)

--------------------------------------------------------------------
---
 kernel/irq/handle.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Index: linus/kernel/irq/handle.c
===================================================================
--- linus.orig/kernel/irq/handle.c	2007-10-30 09:49:26.000000000 -0500
+++ linus/kernel/irq/handle.c	2007-10-30 10:23:52.436719688 -0500
@@ -178,9 +178,11 @@ fastcall unsigned int __do_IRQ(unsigned 
 		 */
 		if (desc->chip->ack)
 			desc->chip->ack(irq);
-		action_ret = handle_IRQ_event(irq, desc->action);
-		if (!noirqdebug)
-			note_interrupt(irq, desc, action_ret);
+		if (likely(!(desc->status & IRQ_DISABLED))) {
+			action_ret = handle_IRQ_event(irq, desc->action);
+			if (!noirqdebug)
+				note_interrupt(irq, desc, action_ret);
+		}
 		desc->chip->end(irq);
 		return 1;
 	}
-- 
Russ Anderson, OS RAS/Partitioning Project Lead  
SGI - Silicon Graphics Inc          rja@sgi.com

             reply	other threads:[~2007-10-30 16:26 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-10-30 16:26 Russ Anderson [this message]
2007-10-30 16:26 ` [patch] __do_IRQ does not check IRQ_DISABLED when IRQ_PER_CPU is set Russ Anderson
2007-10-30 22:22 ` [patch] __do_IRQ does not check IRQ_DISABLED when IRQ_PER_CPU Andrew Morton
2007-10-30 22:22   ` [patch] __do_IRQ does not check IRQ_DISABLED when IRQ_PER_CPU is set Andrew Morton
2007-10-31 16:20 ` Luck, Tony
2007-10-31 16:20   ` Luck, Tony
2007-10-31 20:00   ` Russ Anderson
2007-10-31 20:00     ` Russ Anderson

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=20071030162657.GA21728@sgi.com \
    --to=rja@sgi.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-ia64@vger.kernel.org \
    --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.