From: Russ Anderson <rja@sgi.com>
To: "Luck, Tony" <tony.luck@intel.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
akpm@linux-foundation.org, linux-ia64@vger.kernel.org
Subject: Re: [patch] __do_IRQ does not check IRQ_DISABLED when IRQ_PER_CPU is set
Date: Wed, 31 Oct 2007 20:00:01 +0000 [thread overview]
Message-ID: <20071031200000.GB22855@sgi.com> (raw)
In-Reply-To: <617E1C2C70743745A92448908E030B2A02D2142E@scsmsx411.amr.corp.intel.com>
On Wed, Oct 31, 2007 at 09:20:27AM -0700, Luck, Tony wrote:
> > 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.
>
> Presumably we are in this situation because there are still some
> pending CPE interrupts on some cpus when we disable CPE? Or is
> there a more serious problem that we aren't manage to disable CPE
> on all cpus properly?
The latter. If IRQ_PER_CPU is set, IRQ_DISABLED is not checked
in __do_IRQ(), so the handler is always called. It is not a race
condition type thing where a few pended interrupts get handled after
IRQ_DISABLED is set.
My assumption is that setting IRQ_PER_CPU should not change the
behavior of IRQ_DISABLED.
disable_irq_nosync() does call chip->disable() to provide a chipset
specific interface for disabling the interrupt. That avoids
the issue by having the chipset not issue the interrupt. If a
disable handler is required to disable the interrupt, then setting
IRQ_DISABLED is not necessary (and misleading).
I think the intended behavior is for chip->disable() to
disable the interrupt in the chipset. If, for some reason,
the interrupt cannot be disabled in the hardware, the IRQ_DISABLED
would prevent the interrupt handler from being called.
--
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: "Luck, Tony" <tony.luck@intel.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
akpm@linux-foundation.org, linux-ia64@vger.kernel.org
Subject: Re: [patch] __do_IRQ does not check IRQ_DISABLED when IRQ_PER_CPU is set
Date: Wed, 31 Oct 2007 15:00:01 -0500 [thread overview]
Message-ID: <20071031200000.GB22855@sgi.com> (raw)
In-Reply-To: <617E1C2C70743745A92448908E030B2A02D2142E@scsmsx411.amr.corp.intel.com>
On Wed, Oct 31, 2007 at 09:20:27AM -0700, Luck, Tony wrote:
> > 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.
>
> Presumably we are in this situation because there are still some
> pending CPE interrupts on some cpus when we disable CPE? Or is
> there a more serious problem that we aren't manage to disable CPE
> on all cpus properly?
The latter. If IRQ_PER_CPU is set, IRQ_DISABLED is not checked
in __do_IRQ(), so the handler is always called. It is not a race
condition type thing where a few pended interrupts get handled after
IRQ_DISABLED is set.
My assumption is that setting IRQ_PER_CPU should not change the
behavior of IRQ_DISABLED.
disable_irq_nosync() does call chip->disable() to provide a chipset
specific interface for disabling the interrupt. That avoids
the issue by having the chipset not issue the interrupt. If a
disable handler is required to disable the interrupt, then setting
IRQ_DISABLED is not necessary (and misleading).
I think the intended behavior is for chip->disable() to
disable the interrupt in the chipset. If, for some reason,
the interrupt cannot be disabled in the hardware, the IRQ_DISABLED
would prevent the interrupt handler from being called.
--
Russ Anderson, OS RAS/Partitioning Project Lead
SGI - Silicon Graphics Inc rja@sgi.com
next prev parent reply other threads:[~2007-10-31 20:00 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-10-30 16:26 [patch] __do_IRQ does not check IRQ_DISABLED when IRQ_PER_CPU is set Russ Anderson
2007-10-30 16:26 ` 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 [this message]
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=20071031200000.GB22855@sgi.com \
--to=rja@sgi.com \
--cc=akpm@linux-foundation.org \
--cc=linux-ia64@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=tony.luck@intel.com \
/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.