From: Don Zickus <dzickus@redhat.com>
To: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>,
Robert Richter <robert.richter@amd.com>,
ying.huang@intel.com, Andi Kleen <andi@firstfloor.org>,
gorcunov@gmail.com, LKML <linux-kernel@vger.kernel.org>,
Don Zickus <dzickus@redhat.com>
Subject: [PATCH 3/9] x86, NMI: Rewrite NMI handler
Date: Tue, 30 Nov 2010 17:27:24 -0500 [thread overview]
Message-ID: <1291156050-4482-4-git-send-email-dzickus@redhat.com> (raw)
In-Reply-To: <1291156050-4482-1-git-send-email-dzickus@redhat.com>
From: Huang Ying <ying.huang@intel.com>
The original NMI handler is quite outdated in many aspects. This patch
try to fix it.
The order to process the NMI sources are changed as follow:
notify_die(DIE_NMI_IPI);
notify_die(DIE_NMI);
/* process io port 0x61 */
nmi_watchdog_touch();
unknown_nmi();
DIE_NMI_IPI is used to process CPU specific NMI sources, such as perf
event, oprofile, crash IPI, etc. While DIE_NMI is used to process
non-CPU-specific NMI sources, such as APEI (ACPI Platform Error
Interface) GHES (Generic Hardware Error Source), etc. Non-CPU-specific
NMI sources can be processed on any CPU,
DIE_NMI_IPI must be processed before DIE_NMI. For example, perf event
trigger a NMI on CPU 1, at the same time, APEI GHES trigger another
NMI on CPU 0. If DIE_NMI is processed before DIE_NMI_IPI, it is
possible that APEI GHES is processed on CPU 1, while unknown NMI is
gotten on CPU 0.
In this new order of processing, performance sensitive NMI sources
such as oprofile or perf event will have better performance because
the time consuming IO port reading is done after them.
Only one NMI is eaten for each NMI handler call, even for PCI SERR and
IOCHK NMIs. Because one NMI should be raised for each of them, eating
too many NMI will cause unnecessary unknown NMI.
The die value used in NMI sources are fixed accordingly.
The NMI handler in the patch is designed by Andi Kleen.
v3:
- Make DIE_NMI and DIE_NMI_UNKNOWN work in more traditional way.
v2:
- Split process NMI reason (0x61) on non-BSP into another patch
Signed-off-by: Huang Ying <ying.huang@intel.com>
Signed-off-by: Don Zickus <dzickus@redhat.com>
---
arch/x86/kernel/cpu/perf_event.c | 1 -
arch/x86/kernel/traps.c | 63 ++++++++++++++++++++-----------------
arch/x86/oprofile/nmi_int.c | 1 -
arch/x86/oprofile/nmi_timer_int.c | 2 +-
drivers/char/ipmi/ipmi_watchdog.c | 2 +-
drivers/watchdog/hpwdt.c | 2 +-
6 files changed, 37 insertions(+), 34 deletions(-)
diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index 817d2b1..48aa91f 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -1229,7 +1229,6 @@ perf_event_nmi_handler(struct notifier_block *self,
return NOTIFY_DONE;
switch (cmd) {
- case DIE_NMI:
case DIE_NMI_IPI:
break;
case DIE_NMIUNKNOWN:
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index c7fd1ce..5184b21 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -394,41 +394,46 @@ static notrace __kprobes void default_do_nmi(struct pt_regs *regs)
unsigned char reason = 0;
int cpu;
- cpu = smp_processor_id();
+ /*
+ * CPU-specific NMI must be processed before non-CPU-specific
+ * NMI, otherwise we may lose it, because the CPU-specific
+ * NMI can not be detected/processed on other CPUs.
+ */
+
+ /*
+ * CPU-specific NMI: send to specific CPU or NMI sources must
+ * be processed on specific CPU
+ */
+ if (notify_die(DIE_NMI_IPI, "nmi_ipi", regs, 0, 2, SIGINT)
+ == NOTIFY_STOP)
+ return;
+ /* Non-CPU-specific NMI: NMI sources can be processed on any CPU */
+ cpu = smp_processor_id();
/* Only the BSP gets external NMIs from the system. */
- if (!cpu)
+ if (!cpu) {
reason = get_nmi_reason();
-
- if (!(reason & NMI_REASON_MASK)) {
- if (notify_die(DIE_NMI_IPI, "nmi_ipi", regs, reason, 2, SIGINT)
- == NOTIFY_STOP)
- return;
-
-#ifdef CONFIG_X86_LOCAL_APIC
- if (notify_die(DIE_NMI, "nmi", regs, reason, 2, SIGINT)
- == NOTIFY_STOP)
- return;
+ if (reason & NMI_REASON_MASK) {
+ if (notify_die(DIE_NMI, "nmi", regs, reason, 2, SIGINT)
+ == NOTIFY_STOP)
+ return;
+ if (reason & NMI_REASON_SERR)
+ pci_serr_error(reason, regs);
+ else if (reason & NMI_REASON_IOCHK)
+ io_check_error(reason, regs);
+#ifdef CONFIG_X86_32
+ /*
+ * Reassert NMI in case it became active
+ * meanwhile as it's edge-triggered:
+ */
+ reassert_nmi();
#endif
- unknown_nmi_error(reason, regs);
-
- return;
+ return;
+ }
}
- if (notify_die(DIE_NMI, "nmi", regs, reason, 2, SIGINT) == NOTIFY_STOP)
+ if (notify_die(DIE_NMI, "nmi", regs, 0, 2, SIGINT) == NOTIFY_STOP)
return;
-
- /* AK: following checks seem to be broken on modern chipsets. FIXME */
- if (reason & NMI_REASON_SERR)
- pci_serr_error(reason, regs);
- if (reason & NMI_REASON_IOCHK)
- io_check_error(reason, regs);
-#ifdef CONFIG_X86_32
- /*
- * Reassert NMI in case it became active meanwhile
- * as it's edge-triggered:
- */
- reassert_nmi();
-#endif
+ unknown_nmi_error(reason, regs);
}
dotraplinkage notrace __kprobes void
diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c
index 4e8baad..ee7ff0e 100644
--- a/arch/x86/oprofile/nmi_int.c
+++ b/arch/x86/oprofile/nmi_int.c
@@ -64,7 +64,6 @@ static int profile_exceptions_notify(struct notifier_block *self,
int ret = NOTIFY_DONE;
switch (val) {
- case DIE_NMI:
case DIE_NMI_IPI:
if (ctr_running)
model->check_ctrs(args->regs, &__get_cpu_var(cpu_msrs));
diff --git a/arch/x86/oprofile/nmi_timer_int.c b/arch/x86/oprofile/nmi_timer_int.c
index 0636dd9..fb40645 100644
--- a/arch/x86/oprofile/nmi_timer_int.c
+++ b/arch/x86/oprofile/nmi_timer_int.c
@@ -25,7 +25,7 @@ static int profile_timer_exceptions_notify(struct notifier_block *self,
int ret = NOTIFY_DONE;
switch (val) {
- case DIE_NMI:
+ case DIE_NMI_IPI:
oprofile_add_sample(args->regs, 0);
ret = NOTIFY_STOP;
break;
diff --git a/drivers/char/ipmi/ipmi_watchdog.c b/drivers/char/ipmi/ipmi_watchdog.c
index f4d334f..320668f 100644
--- a/drivers/char/ipmi/ipmi_watchdog.c
+++ b/drivers/char/ipmi/ipmi_watchdog.c
@@ -1081,7 +1081,7 @@ ipmi_nmi(struct notifier_block *self, unsigned long val, void *data)
{
struct die_args *args = data;
- if (val != DIE_NMI)
+ if (val != DIE_NMIUNKNOWN)
return NOTIFY_OK;
/* Hack, if it's a memory or I/O error, ignore it. */
diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index c19f4a2..717c67b 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -469,7 +469,7 @@ static int hpwdt_pretimeout(struct notifier_block *nb, unsigned long ulReason,
unsigned long rom_pl;
static int die_nmi_called;
- if (ulReason != DIE_NMI && ulReason != DIE_NMI_IPI)
+ if (ulReason != DIE_NMIUNKNOWN)
goto out;
if (!hpwdt_nmi_decoding)
--
1.7.3.2
next prev parent reply other threads:[~2010-11-30 22:28 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-11-30 22:27 [V3 PATCH 0/9] x86, NMI: give NMI handler a face-lift Don Zickus
2010-11-30 22:27 ` [PATCH 1/9] x86, NMI: Add NMI symbol constants and rename memory parity to PCI SERR Don Zickus
2010-11-30 22:27 ` [PATCH 2/9] x86, NMI: Add touch_nmi_watchdog to io_check_error delay Don Zickus
2010-11-30 22:27 ` Don Zickus [this message]
2010-11-30 22:27 ` [PATCH 4/9] x86, NMI: Remove DIE_NMI_IPI and add priorties to handlers Don Zickus
2010-12-01 18:41 ` Cyrill Gorcunov
2010-12-01 18:53 ` Peter Zijlstra
2010-12-01 19:01 ` Cyrill Gorcunov
2010-12-01 21:30 ` Andi Kleen
2010-12-01 21:35 ` Cyrill Gorcunov
2010-11-30 22:27 ` [PATCH 5/9] x86, NMI: Allow NMI reason io port (0x61) to be processed on any CPU Don Zickus
2010-11-30 22:27 ` [PATCH 6/9] x86: only call smp_processor_id in non-preempt cases Don Zickus
2010-12-01 18:07 ` Cyrill Gorcunov
2010-11-30 22:27 ` [PATCH 7/9] x86: Avoid calling arch_trigger_all_cpu_backtrace() at the same time Don Zickus
2010-11-30 22:27 ` [PATCH 8/9] panic: ratelimit panic messages Don Zickus
2010-11-30 22:27 ` [PATCH 9/9] watchdog: touch_nmi_watchdog should only touch local cpu not every one Don Zickus
2010-12-22 3:16 ` [V3 PATCH 0/9] x86, NMI: give NMI handler a face-lift Huang Ying
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=1291156050-4482-4-git-send-email-dzickus@redhat.com \
--to=dzickus@redhat.com \
--cc=andi@firstfloor.org \
--cc=gorcunov@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=peterz@infradead.org \
--cc=robert.richter@amd.com \
--cc=ying.huang@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome