Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1288120 > unrolled thread

[V6 PATCH 0/6] Fix race issues among panic, NMI and crash_kexec

Started byHidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
First post2015-12-10 02:50 +0100
Last post2015-12-10 03:00 +0100
Articles 12 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [V6 PATCH 0/6] Fix race issues among panic, NMI and crash_kexec Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com> - 2015-12-10 02:50 +0100
    [V6 PATCH 5/6] x86/nmi: Fix to save registers for crash dump on  external NMI broadcast Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com> - 2015-12-10 02:50 +0100
      Re: [V6 PATCH 5/6] x86/nmi: Fix to save registers for crash dump on  external NMI broadcast kbuild test robot <lkp@intel.com> - 2015-12-10 05:00 +0100
        RE: Re: [V6 PATCH 5/6] x86/nmi: Fix to save registers for crash  dump on external NMI broadcast 河合英宏 / KAWAI，HIDEHIRO   <hidehiro.kawai.ez@hitachi.com> - 2015-12-10 07:40 +0100
          [V6.1 PATCH 5/6] x86/nmi: Fix to save registers for crash dump on  external NMI broadcast Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com> - 2015-12-10 08:10 +0100
            Re: [V6.1 PATCH 5/6] x86/nmi: Fix to save registers for crash dump  on external NMI broadcast Borislav Petkov <bp@alien8.de> - 2015-12-11 19:10 +0100
    [V6 PATCH 3/6] kexec: Fix race between panic() and crash_kexec()  called directly Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com> - 2015-12-10 02:50 +0100
    [V6 PATCH 1/6] panic/x86: Fix re-entrance problem due to panic on  NMI Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com> - 2015-12-10 03:00 +0100
      Re: [V6 PATCH 1/6] panic/x86: Fix re-entrance problem due to panic  on NMI Borislav Petkov <bp@alien8.de> - 2015-12-10 16:50 +0100
        RE: Re: [V6 PATCH 1/6] panic/x86: Fix re-entrance problem due to  panic on NMI 河合英宏 / KAWAI，HIDEHIRO   <hidehiro.kawai.ez@hitachi.com> - 2015-12-11 01:30 +0100
    [V6 PATCH 2/6] panic/x86: Allow CPUs to save registers even if they  are looping in NMI context Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com> - 2015-12-10 03:00 +0100
    [V6 PATCH 6/6] Documentation: Add documentation for  kernel.panic_on_io_nmi sysctl Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com> - 2015-12-10 03:00 +0100

#1288120 — [V6 PATCH 0/6] Fix race issues among panic, NMI and crash_kexec

FromHidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
Date2015-12-10 02:50 +0100
Subject[V6 PATCH 0/6] Fix race issues among panic, NMI and crash_kexec
Message-ID<qDYwN-7Pf-3@gated-at.bofh.it>
When an HA clustering software or administrator detects unresponsiveness
of a host, they issue an NMI to the host to completely stop current
works and take a crash dump.  If the kernel has already panicked
or is capturing a crash dump at that time, further NMI can cause
a crash dump failure.

Also, crash_kexec() called from oops context and panic() can
cause race conditions.

To solve these issues, this patch set does following things:

- Don't call panic() on NMI if the kernel has already panicked
- Extend exclusion control currently done by panic_lock to crash_kexec
- Introduce "apic_extnmi=none" boot option which masks external NMI
  NMI at the boot time

Additionally, "apic_extnmi=all" is provieded.  This option unmasks
external NMI for all CPUs.  This would help cause kernel panic even if
CPU 0 can't handle an external NMI due to hang-up in NMI context
or being handled by other NMI handlers.

This patch set can be applied to current -tip tree.

V6:
- Update comments and patch descriptions all over the patch series
- Add documentation for kernel.panic_on_io_nmi sysctl (PATCH 6/6)
- Separate PATCH 5/6 from PATCH 2/6 because the portion is actually
  needed for "apic_extnmi=all" case introduced by PATCH 4/6
- ...and various fixes (please see the change logs in each patch
  description for details)

V5: https://lkml.org/lkml/2015/11/20/228
- Use WRITE_ONCE() for crash_ipi_done to keep the instruction order
  (PATCH 2/4)
- Address concurrent unknown/external NMI case, too (PATCH 2/4)
- Fix build errors (PATCH 3/4)
- Rename "noextnmi" boot option to "apic_extnmi" and expand its
  feature (PATCH 4/4)

V4: https://lkml.org/lkml/2015/9/25/193
- Improve comments and descriptions (PATCH 1/4 to 3/4)
- Use new __crash_kexec(), no exclusion check version of crash_kexec(),
  instead of checking if panic_cpu is the current cpu or not
  (PATCH 3/4)

V3: https://lkml.org/lkml/2015/8/6/39
- Introduce nmi_panic() macro to reduce code duplication
- In the case of panic on NMI, don't return from NMI handlers
  if another cpu already panicked

V2: https://lkml.org/lkml/2015/7/27/31
- Use atomic_cmpxchg() instead of current spin_trylock() to exclude
  concurrent accesses to panic() and crash_kexec()
- Don't introduce no-lock version of panic() and crash_kexec()

V1: https://lkml.org/lkml/2015/7/22/81

---

Hidehiro Kawai (6):
      panic/x86: Fix re-entrance problem due to panic on NMI
      panic/x86: Allow CPUs to save registers even if they are looping in NMI context
      kexec: Fix race between panic() and crash_kexec() called directly
      x86/apic: Introduce apic_extnmi boot option
      x86/nmi: Fix to save registers for crash dump on external NMI broadcast
      Documentation: Add documentation for kernel.panic_on_io_nmi sysctl


 Documentation/kernel-parameters.txt |    9 +++++++++
 Documentation/sysctl/kernel.txt     |   15 +++++++++++++++
 arch/x86/include/asm/apic.h         |    5 +++++
 arch/x86/include/asm/reboot.h       |    1 +
 arch/x86/kernel/apic/apic.c         |   35 +++++++++++++++++++++++++++++++++--
 arch/x86/kernel/nmi.c               |   27 ++++++++++++++++++++++-----
 arch/x86/kernel/reboot.c            |   28 ++++++++++++++++++++++++++++
 include/linux/kernel.h              |   29 +++++++++++++++++++++++++++++
 include/linux/kexec.h               |    2 ++
 kernel/kexec_core.c                 |   30 +++++++++++++++++++++++++++++-
 kernel/panic.c                      |   29 ++++++++++++++++++++++++-----
 kernel/watchdog.c                   |    2 +-
 12 files changed, 198 insertions(+), 14 deletions(-)


-- 
Hidehiro Kawai
Hitachi, Ltd. Research & Development Group


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1288123 — [V6 PATCH 5/6] x86/nmi: Fix to save registers for crash dump on external NMI broadcast

FromHidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
Date2015-12-10 02:50 +0100
Subject[V6 PATCH 5/6] x86/nmi: Fix to save registers for crash dump on external NMI broadcast
Message-ID<qDYwO-7Pf-7@gated-at.bofh.it>
In reply to#1288120
Now, multiple CPUs can receive external NMI simultaneously by
specifying "apic_extnmi=all" as an boot option.  When we take a
crash dump by using external NMI with this option, we fail to save
register values into the crash dump.  This happens as follows:

  CPU 0                              CPU 1
  ================================   =============================
  receive an external NMI
  default_do_nmi()                   receive an external NMI
    spin_lock(&nmi_reason_lock)      default_do_nmi()
    io_check_error()                   spin_lock(&nmi_reason_lock)
      panic()                            busy loop
      ...
        kdump_nmi_shootdown_cpus()
          issue NMI IPI -----------> blocked until IRET
                                         busy loop...

  Here, since CPU 1 is in NMI context, additional NMI from CPU 0
  is blocked until CPU 1 executes IRET.  However, CPU 1 never
  executes IRET, so the NMI is not handled and the callback function
  to save registers is never called.

To solve this issue, we check if the IPI for crash dumping was
issued while waiting for nmi_reason_lock to be released, and if so,
call its callback function directly.  If the IPI is not issued (e.g.
kdump is disabled), the actual behavior doesn't change.

V6:
- Separated from the former patch `panic/x86: Allow cpus to save
  registers even if they are looping in NMI context'
- Fix comments
- Remove unneeded UP version of poll_crash_ipi_and_calback
- Rename poll_crash_ipi_and_callback to run_crash_ipi_callback

Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Michal Hocko <mhocko@kernel.org>
---
 arch/x86/include/asm/reboot.h |    1 +
 arch/x86/kernel/nmi.c         |   11 ++++++++++-
 arch/x86/kernel/reboot.c      |   18 +++++++++++++-----
 3 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/reboot.h b/arch/x86/include/asm/reboot.h
index a82c4f1..2cb1cc2 100644
--- a/arch/x86/include/asm/reboot.h
+++ b/arch/x86/include/asm/reboot.h
@@ -25,5 +25,6 @@ void __noreturn machine_real_restart(unsigned int type);
 
 typedef void (*nmi_shootdown_cb)(int, struct pt_regs*);
 void nmi_shootdown_cpus(nmi_shootdown_cb callback);
+void run_crash_ipi_callback(struct pt_regs *regs);
 
 #endif /* _ASM_X86_REBOOT_H */
diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
index 5e00de7..cbfa0b5 100644
--- a/arch/x86/kernel/nmi.c
+++ b/arch/x86/kernel/nmi.c
@@ -29,6 +29,7 @@
 #include <asm/mach_traps.h>
 #include <asm/nmi.h>
 #include <asm/x86_init.h>
+#include <asm/reboot.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/nmi.h>
@@ -357,7 +358,15 @@ static void default_do_nmi(struct pt_regs *regs)
 	}
 
 	/* Non-CPU-specific NMI: NMI sources can be processed on any CPU */
-	raw_spin_lock(&nmi_reason_lock);
+	/*
+	 * Another CPU may be processing panic routines while holding
+	 * nmi_reason_lock.  Check if the CPU issued the IPI for crash
+	 * dumping, and if so, call its callback directly.  If there is
+	 * no CPU preparing crash dump, we simply loop here without doing
+	 * special things.
+	 */
+	while (!raw_spin_trylock(&nmi_reason_lock))
+		run_crash_ipi_callback(regs);
 	reason = x86_platform.get_nmi_reason();
 
 	if (reason & NMI_REASON_MASK) {
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 1da1302..60a216b 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -793,17 +793,25 @@ void nmi_shootdown_cpus(nmi_shootdown_cb callback)
 	/* Leave the nmi callback set */
 }
 
+/*
+ * Wait for the crash dumping IPI to be issued, and then call its callback
+ * directly.  This function is used when we have already been in NMI handler.
+ */
+void run_crash_ipi_callback(struct pt_regs *regs)
+{
+	if (crash_ipi_issued)
+		crash_nmi_callback(0, regs); /* Don't return */
+}
+
 /* Override the weak function in kernel/panic.c */
 void nmi_panic_self_stop(struct pt_regs *regs)
 {
 	while (1) {
 		/*
-		 * Wait for the crash dumping IPI to be issued, and then
-		 * call its callback directly.
+		 * If there is no CPU preparing crash dump, we simply loop
+		 * here without doing special things.
 		 */
-		if (READ_ONCE(crash_ipi_issued))
-			crash_nmi_callback(0, regs); /* Don't return */
-
+		run_crash_ipi_callback(regs);
 		cpu_relax();
 	}
 }


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1288175 — Re: [V6 PATCH 5/6] x86/nmi: Fix to save registers for crash dump on external NMI broadcast

Fromkbuild test robot <lkp@intel.com>
Date2015-12-10 05:00 +0100
SubjectRe: [V6 PATCH 5/6] x86/nmi: Fix to save registers for crash dump on external NMI broadcast
Message-ID<qE0yC-Iu-7@gated-at.bofh.it>
In reply to#1288123

[Multipart message — attachments visible in raw view] — view raw

Hi Hidehiro,

[auto build test ERROR on v4.4-rc4]
[also build test ERROR on next-20151209]
[cannot apply to tip/x86/core]

url:    https://github.com/0day-ci/linux/commits/Hidehiro-Kawai/Fix-race-issues-among-panic-NMI-and-crash_kexec/20151210-095254
config: x86_64-randconfig-s4-12101030 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   arch/x86/built-in.o: In function `do_nmi':
>> (.text+0x7339): undefined reference to `run_crash_ipi_callback'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [next] | [standalone]

#1288227 — RE: Re: [V6 PATCH 5/6] x86/nmi: Fix to save registers for crash dump on external NMI broadcast

From河合英宏 / KAWAI，HIDEHIRO <hidehiro.kawai.ez@hitachi.com>
Date2015-12-10 07:40 +0100
SubjectRE: Re: [V6 PATCH 5/6] x86/nmi: Fix to save registers for crash dump on external NMI broadcast
Message-ID<qE33r-2pm-3@gated-at.bofh.it>
In reply to#1288175
SGkgU3RldmVuLA0KDQo+IEZyb206IFN0ZXZlbiBSb3N0ZWR0IFttYWlsdG86cm9zdGVkdEBnb29k
bWlzLm9yZ10NCj4gT24gVHVlLCBOb3YgMjQsIDIwMTUgYXQgMTE6NDg6NTNBTSArMDEwMCwgQm9y
aXNsYXYgUGV0a292IHdyb3RlOg0KPiA+ID4gKwkgKi8NCj4gPiA+ICsJd2hpbGUgKCFyYXdfc3Bp
bl90cnlsb2NrKCZubWlfcmVhc29uX2xvY2spKQ0KPiA+ID4gKwkJcG9sbF9jcmFzaF9pcGlfYW5k
X2NhbGxiYWNrKHJlZ3MpOw0KPiA+DQo+ID4gV2FhYWl0IGEgbWludXRlOiBzbyBpZiB3ZSdyZSBn
ZXR0aW5nIE5NSXMgYnJvYWRjYXN0ZWQgb24gZXZlcnkgY29yZSBidXQNCj4gPiB3ZSdyZSAqbm90
KiBjcmFzaCBkdW1waW5nLCB3ZSB3aWxsIHJ1biBpbnRvIGhlcmUgdG9vLiBUaGlzIGNhbid0IGJl
DQo+ID4gcmlnaHQuIDotXA0KPiANCj4gVGhpcyBvbmx5IGRvZXMgc29tZXRoaW5nIGlmIGNyYXNo
X2lwaV9kb25lIGlzIHNldCwgd2hpY2ggbWVhbnMgeW91IGFyZSBraWxsaW5nDQo+IHRoZSBib3gu
IEJ1dCBwZXJoYXBzIGEgY29tbWVudCB0aGF0IHN0YXRlcyB0aGF0IGhlcmUgd291bGQgYmUgdXNl
ZnVsLCBvciBtYXliZQ0KPiBqdXN0IHB1dCBpbiB0aGUgY2hlY2sgaGVyZS4gVGhlcmUncyBubyBu
ZWVkIHRvIG1ha2UgaXQgZGVwZW5kIG9uIFNNUCwgYXMNCj4gcmF3X3NwaW5fdHJ5bG9jaygpIHdp
bGwgdHVybiB0byBqdXN0ICh7MX0pIGZvciBVUCwgYW5kIHRoYXQgY29kZSB3b250IGV2ZW4gYmUN
Cj4gaGl0Lg0KDQpJdCBzZWVtcyB0aGF0IHBvbGxfY3Jhc2hfaXBpX2FuZF9jYWxsYmFjayAobm93
IHJlbmFtZWQgdG8gcnVuX2NyYXNoX2lwaV9jYWxsYmFjaykNCmlzIHJlZmVycmVkIGZvciBVUCBp
ZiBDT05GSUdfREVCVUdfU1BJTkxPQ0s9eSwgYW5kIGl0IGNhdXNlcyBhIGJ1aWxkIGVycm9yDQph
cyBiZWxvdy4gIHJ1bl9jcmFzaF9pcGlfY2FsbGJhY2sgcmVmZXJzIGNyYXNoX2lwaV9pc3N1ZWQg
YW5kIGNyYXNoX25taV9jYWxsYmFjaywNCndoaWNoIGFyZSBkZWZpbmVkIG9ubHkgaWYgQ09ORklH
X1NNUD15LiAgU28gd2UgbmVlZCB0byBkZWZpbmVkIGl0IHNlcGFyYXRlbHkNCmZvciBTTVAgYW5k
IFVQLg0KDQpJJ2xsIHJlc2VuZCB0aGlzIHBhdGNoIGxhdGVyLg0KDQo+IEhpIEhpZGVoaXJvLA0K
PiANCj4gW2F1dG8gYnVpbGQgdGVzdCBFUlJPUiBvbiB2NC40LXJjNF0NCj4gW2Fsc28gYnVpbGQg
dGVzdCBFUlJPUiBvbiBuZXh0LTIwMTUxMjA5XQ0KPiBbY2Fubm90IGFwcGx5IHRvIHRpcC94ODYv
Y29yZV0NCj4gDQo+IHVybDoNCj4gaHR0cHM6Ly9naXRodWIuY29tLzBkYXktY2kvbGludXgvY29t
bWl0cy9IaWRlaGlyby1LYXdhaS9GaXgtcmFjZS1pc3N1ZXMtYW1vbmctcGFuaWMtTk1JLWFuZC1j
cmFzaF9rZXhlYy8yMDE1MTIxMC0wOTUNCj4gMjU0DQo+IGNvbmZpZzogeDg2XzY0LXJhbmRjb25m
aWctczQtMTIxMDEwMzAgKGF0dGFjaGVkIGFzIC5jb25maWcpDQo+IHJlcHJvZHVjZToNCj4gICAg
ICAgICAjIHNhdmUgdGhlIGF0dGFjaGVkIC5jb25maWcgdG8gbGludXggYnVpbGQgdHJlZQ0KPiAg
ICAgICAgIG1ha2UgQVJDSD14ODZfNjQNCj4gDQo+IEFsbCBlcnJvcnMgKG5ldyBvbmVzIHByZWZp
eGVkIGJ5ID4+KToNCj4gDQo+ICAgIGFyY2gveDg2L2J1aWx0LWluLm86IEluIGZ1bmN0aW9uIGBk
b19ubWknOg0KPiA+PiAoLnRleHQrMHg3MzM5KTogdW5kZWZpbmVkIHJlZmVyZW5jZSB0byBgcnVu
X2NyYXNoX2lwaV9jYWxsYmFjaycNCj4gDQo+IC0tLQ0KPiAwLURBWSBrZXJuZWwgdGVzdCBpbmZy
YXN0cnVjdHVyZSAgICAgICAgICAgICAgICBPcGVuIFNvdXJjZSBUZWNobm9sb2d5IENlbnRlcg0K
PiBodHRwczovL2xpc3RzLjAxLm9yZy9waXBlcm1haWwva2J1aWxkLWFsbCAgICAgICAgICAgICAg
ICAgICBJbnRlbCBDb3Jwb3JhdGlvbg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1288241 — [V6.1 PATCH 5/6] x86/nmi: Fix to save registers for crash dump on external NMI broadcast

FromHidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
Date2015-12-10 08:10 +0100
Subject[V6.1 PATCH 5/6] x86/nmi: Fix to save registers for crash dump on external NMI broadcast
Message-ID<qE3wu-2QB-9@gated-at.bofh.it>
In reply to#1288227
Now, multiple CPUs can receive external NMI simultaneously by
specifying "apic_extnmi=all" as an boot option.  When we take a
crash dump by using external NMI with this option, we fail to save
register values into the crash dump.  This happens as follows:

  CPU 0                              CPU 1
  ================================   =============================
  receive an external NMI
  default_do_nmi()                   receive an external NMI
    spin_lock(&nmi_reason_lock)      default_do_nmi()
    io_check_error()                   spin_lock(&nmi_reason_lock)
      panic()                            busy loop
      ...
        kdump_nmi_shootdown_cpus()
          issue NMI IPI -----------> blocked until IRET
                                         busy loop...

  Here, since CPU 1 is in NMI context, additional NMI from CPU 0
  is blocked until CPU 1 executes IRET.  However, CPU 1 never
  executes IRET, so the NMI is not handled and the callback function
  to save registers is never called.

To solve this issue, we check if the IPI for crash dumping was
issued while waiting for nmi_reason_lock to be released, and if so,
call its callback function directly.  If the IPI is not issued (e.g.
kdump is disabled), the actual behavior doesn't change.

V6.1:
- Reintroduce the UP version of run_crash_ipi_callback to fix build
  error for CONFIG_SMP=n and CONFIG_DEBUG_SPINLOCK=y case

V6:
- Separated from the former patch `panic/x86: Allow cpus to save
  registers even if they are looping in NMI context'
- Fix comments
- Remove unneeded UP version of poll_crash_ipi_and_calback
- Rename poll_crash_ipi_and_callback to run_crash_ipi_callback

Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Michal Hocko <mhocko@kernel.org>
---
 arch/x86/include/asm/reboot.h |    1 +
 arch/x86/kernel/nmi.c         |   11 ++++++++++-
 arch/x86/kernel/reboot.c      |   22 +++++++++++++++++-----
 3 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/reboot.h b/arch/x86/include/asm/reboot.h
index a82c4f1..2cb1cc2 100644
--- a/arch/x86/include/asm/reboot.h
+++ b/arch/x86/include/asm/reboot.h
@@ -25,5 +25,6 @@ void __noreturn machine_real_restart(unsigned int type);
 
 typedef void (*nmi_shootdown_cb)(int, struct pt_regs*);
 void nmi_shootdown_cpus(nmi_shootdown_cb callback);
+void run_crash_ipi_callback(struct pt_regs *regs);
 
 #endif /* _ASM_X86_REBOOT_H */
diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
index 5e00de7..cbfa0b5 100644
--- a/arch/x86/kernel/nmi.c
+++ b/arch/x86/kernel/nmi.c
@@ -29,6 +29,7 @@
 #include <asm/mach_traps.h>
 #include <asm/nmi.h>
 #include <asm/x86_init.h>
+#include <asm/reboot.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/nmi.h>
@@ -357,7 +358,15 @@ static void default_do_nmi(struct pt_regs *regs)
 	}
 
 	/* Non-CPU-specific NMI: NMI sources can be processed on any CPU */
-	raw_spin_lock(&nmi_reason_lock);
+	/*
+	 * Another CPU may be processing panic routines while holding
+	 * nmi_reason_lock.  Check if the CPU issued the IPI for crash
+	 * dumping, and if so, call its callback directly.  If there is
+	 * no CPU preparing crash dump, we simply loop here without doing
+	 * special things.
+	 */
+	while (!raw_spin_trylock(&nmi_reason_lock))
+		run_crash_ipi_callback(regs);
 	reason = x86_platform.get_nmi_reason();
 
 	if (reason & NMI_REASON_MASK) {
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 1da1302..8a184e3 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -793,17 +793,25 @@ void nmi_shootdown_cpus(nmi_shootdown_cb callback)
 	/* Leave the nmi callback set */
 }
 
+/*
+ * Wait for the crash dumping IPI to be issued, and then call its callback
+ * directly.  This function is used when we have already been in NMI handler.
+ */
+void run_crash_ipi_callback(struct pt_regs *regs)
+{
+	if (crash_ipi_issued)
+		crash_nmi_callback(0, regs); /* Don't return */
+}
+
 /* Override the weak function in kernel/panic.c */
 void nmi_panic_self_stop(struct pt_regs *regs)
 {
 	while (1) {
 		/*
-		 * Wait for the crash dumping IPI to be issued, and then
-		 * call its callback directly.
+		 * If there is no CPU preparing crash dump, we simply loop
+		 * here without doing special things.
 		 */
-		if (READ_ONCE(crash_ipi_issued))
-			crash_nmi_callback(0, regs); /* Don't return */
-
+		run_crash_ipi_callback(regs);
 		cpu_relax();
 	}
 }
@@ -813,4 +821,8 @@ void nmi_shootdown_cpus(nmi_shootdown_cb callback)
 {
 	/* No other CPUs to shoot down */
 }
+
+void run_crash_ipi_callback(struct pt_regs *regs)
+{
+}
 #endif


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1289788 — Re: [V6.1 PATCH 5/6] x86/nmi: Fix to save registers for crash dump on external NMI broadcast

FromBorislav Petkov <bp@alien8.de>
Date2015-12-11 19:10 +0100
SubjectRe: [V6.1 PATCH 5/6] x86/nmi: Fix to save registers for crash dump on external NMI broadcast
Message-ID<qEAiK-7Yl-17@gated-at.bofh.it>
In reply to#1288241
On Thu, Dec 10, 2015 at 03:52:46PM +0900, Hidehiro Kawai wrote:
> Now, multiple CPUs can receive external NMI simultaneously by
> specifying "apic_extnmi=all" as an boot option.  When we take a
> crash dump by using external NMI with this option, we fail to save
> register values into the crash dump.  This happens as follows:
> 
>   CPU 0                              CPU 1
>   ================================   =============================
>   receive an external NMI
>   default_do_nmi()                   receive an external NMI
>     spin_lock(&nmi_reason_lock)      default_do_nmi()
>     io_check_error()                   spin_lock(&nmi_reason_lock)
>       panic()                            busy loop
>       ...
>         kdump_nmi_shootdown_cpus()
>           issue NMI IPI -----------> blocked until IRET
>                                          busy loop...
> 
>   Here, since CPU 1 is in NMI context, additional NMI from CPU 0
>   is blocked until CPU 1 executes IRET.  However, CPU 1 never
>   executes IRET, so the NMI is not handled and the callback function
>   to save registers is never called.
> 
> To solve this issue, we check if the IPI for crash dumping was
> issued while waiting for nmi_reason_lock to be released, and if so,
> call its callback function directly.  If the IPI is not issued (e.g.
> kdump is disabled), the actual behavior doesn't change.
> 
> V6.1:
> - Reintroduce the UP version of run_crash_ipi_callback to fix build
>   error for CONFIG_SMP=n and CONFIG_DEBUG_SPINLOCK=y case
> 
> V6:
> - Separated from the former patch `panic/x86: Allow cpus to save
>   registers even if they are looping in NMI context'
> - Fix comments
> - Remove unneeded UP version of poll_crash_ipi_and_calback
> - Rename poll_crash_ipi_and_callback to run_crash_ipi_callback
> 
> Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Michal Hocko <mhocko@kernel.org>
> ---
>  arch/x86/include/asm/reboot.h |    1 +
>  arch/x86/kernel/nmi.c         |   11 ++++++++++-
>  arch/x86/kernel/reboot.c      |   22 +++++++++++++++++-----
>  3 files changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/include/asm/reboot.h b/arch/x86/include/asm/reboot.h
> index a82c4f1..2cb1cc2 100644
> --- a/arch/x86/include/asm/reboot.h
> +++ b/arch/x86/include/asm/reboot.h
> @@ -25,5 +25,6 @@ void __noreturn machine_real_restart(unsigned int type);
>  
>  typedef void (*nmi_shootdown_cb)(int, struct pt_regs*);
>  void nmi_shootdown_cpus(nmi_shootdown_cb callback);
> +void run_crash_ipi_callback(struct pt_regs *regs);
>  
>  #endif /* _ASM_X86_REBOOT_H */
> diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
> index 5e00de7..cbfa0b5 100644
> --- a/arch/x86/kernel/nmi.c
> +++ b/arch/x86/kernel/nmi.c
> @@ -29,6 +29,7 @@
>  #include <asm/mach_traps.h>
>  #include <asm/nmi.h>
>  #include <asm/x86_init.h>
> +#include <asm/reboot.h>
>  
>  #define CREATE_TRACE_POINTS
>  #include <trace/events/nmi.h>
> @@ -357,7 +358,15 @@ static void default_do_nmi(struct pt_regs *regs)
>  	}
>  
>  	/* Non-CPU-specific NMI: NMI sources can be processed on any CPU */
> -	raw_spin_lock(&nmi_reason_lock);
> +	/*
> +	 * Another CPU may be processing panic routines while holding
> +	 * nmi_reason_lock.  Check if the CPU issued the IPI for crash
> +	 * dumping, and if so, call its callback directly.  If there is
> +	 * no CPU preparing crash dump, we simply loop here without doing
> +	 * special things.
> +	 */
> +	while (!raw_spin_trylock(&nmi_reason_lock))
> +		run_crash_ipi_callback(regs);

Added cpu_relax() here too:

+       while (!raw_spin_trylock(&nmi_reason_lock)) {
+               run_crash_ipi_callback(regs);
+               cpu_relax();
+       }

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1288124 — [V6 PATCH 3/6] kexec: Fix race between panic() and crash_kexec() called directly

FromHidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
Date2015-12-10 02:50 +0100
Subject[V6 PATCH 3/6] kexec: Fix race between panic() and crash_kexec() called directly
Message-ID<qDYwO-7Pf-9@gated-at.bofh.it>
In reply to#1288120
Currently, panic() and crash_kexec() can be called at the same time.
For example (x86 case):

CPU 0:
  oops_end()
    crash_kexec()
      mutex_trylock() // acquired
        nmi_shootdown_cpus() // stop other CPUs

CPU 1:
  panic()
    crash_kexec()
      mutex_trylock() // failed to acquire
    smp_send_stop() // stop other CPUs
    infinite loop

If CPU 1 calls smp_send_stop() before nmi_shootdown_cpus(), kdump
fails.

In another case:

CPU 0:
  oops_end()
    crash_kexec()
      mutex_trylock() // acquired
        <NMI>
        io_check_error()
          panic()
            crash_kexec()
              mutex_trylock() // failed to acquire
            infinite loop

Clearly, this is an undesirable result.

To fix this problem, this patch changes crash_kexec() to exclude
others by using atomic_t panic_cpu.

V6:
- Use PANIC_CPU_INVALID instead of hard-coded -1
- Add comments about __crash_kexec()

V5:
- Add missing dummy __crash_kexec() for !CONFIG_KEXEC_CORE case
- Replace atomic_xchg() with atomic_set() in crash_kexec() because
  it is used as a release operation and there is no need of memory
  barrier effect.  This change also removes an unused value warning

V4:
- Use new __crash_kexec(), no exclusion check version of crash_kexec(),
  instead of checking if panic_cpu is the current CPU or not

V2:
- Use atomic_cmpxchg() instead of spin_trylock() on panic_lock
  to exclude concurrent accesses
- Don't introduce no-lock version of crash_kexec()

Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
Cc: Eric Biederman <ebiederm@xmission.com>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
---
 include/linux/kexec.h |    2 ++
 kernel/kexec_core.c   |   30 +++++++++++++++++++++++++++++-
 kernel/panic.c        |    4 ++--
 3 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index d140b1e..7b68d27 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -237,6 +237,7 @@ extern int kexec_purgatory_get_set_symbol(struct kimage *image,
 					  unsigned int size, bool get_value);
 extern void *kexec_purgatory_get_symbol_addr(struct kimage *image,
 					     const char *name);
+extern void __crash_kexec(struct pt_regs *);
 extern void crash_kexec(struct pt_regs *);
 int kexec_should_crash(struct task_struct *);
 void crash_save_cpu(struct pt_regs *regs, int cpu);
@@ -332,6 +333,7 @@ int __weak arch_kexec_apply_relocations(const Elf_Ehdr *ehdr, Elf_Shdr *sechdrs,
 #else /* !CONFIG_KEXEC_CORE */
 struct pt_regs;
 struct task_struct;
+static inline void __crash_kexec(struct pt_regs *regs) { }
 static inline void crash_kexec(struct pt_regs *regs) { }
 static inline int kexec_should_crash(struct task_struct *p) { return 0; }
 #define kexec_in_progress false
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 11b64a6..c823f30 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -853,7 +853,12 @@ struct kimage *kexec_image;
 struct kimage *kexec_crash_image;
 int kexec_load_disabled;
 
-void crash_kexec(struct pt_regs *regs)
+/*
+ * No panic_cpu check version of crash_kexec().  This function is called
+ * only when panic_cpu holds the current CPU number; this is the only CPU
+ * which processes crash_kexec routines.
+ */
+void __crash_kexec(struct pt_regs *regs)
 {
 	/* Take the kexec_mutex here to prevent sys_kexec_load
 	 * running on one cpu from replacing the crash kernel
@@ -876,6 +881,29 @@ void crash_kexec(struct pt_regs *regs)
 	}
 }
 
+void crash_kexec(struct pt_regs *regs)
+{
+	int old_cpu, this_cpu;
+
+	/*
+	 * Only one CPU is allowed to execute the crash_kexec() code as with
+	 * panic().  Otherwise parallel calls of panic() and crash_kexec()
+	 * may stop each other.  To exclude them, we use panic_cpu here too.
+	 */
+	this_cpu = raw_smp_processor_id();
+	old_cpu = atomic_cmpxchg(&panic_cpu, PANIC_CPU_INVALID, this_cpu);
+	if (old_cpu == PANIC_CPU_INVALID) {
+		/* This is the 1st CPU which comes here, so go ahead. */
+		__crash_kexec(regs);
+
+		/*
+		 * Reset panic_cpu to allow another panic()/crash_kexec()
+		 * call.
+		 */
+		atomic_set(&panic_cpu, PANIC_CPU_INVALID);
+	}
+}
+
 size_t crash_get_memory_size(void)
 {
 	size_t size = 0;
diff --git a/kernel/panic.c b/kernel/panic.c
index 3d6c3f1..81a0a3d 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -138,7 +138,7 @@ void panic(const char *fmt, ...)
 	 * the "crash_kexec_post_notifiers" option to the kernel.
 	 */
 	if (!crash_kexec_post_notifiers)
-		crash_kexec(NULL);
+		__crash_kexec(NULL); /* bypass panic_cpu check */
 
 	/*
 	 * Note smp_send_stop is the usual smp shutdown function, which
@@ -163,7 +163,7 @@ void panic(const char *fmt, ...)
 	 * more unstable, it can increase risks of the kdump failure too.
 	 */
 	if (crash_kexec_post_notifiers)
-		crash_kexec(NULL);
+		__crash_kexec(NULL); /* bypass panic_cpu check */
 
 	bust_spinlocks(0);
 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1288126 — [V6 PATCH 1/6] panic/x86: Fix re-entrance problem due to panic on NMI

FromHidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
Date2015-12-10 03:00 +0100
Subject[V6 PATCH 1/6] panic/x86: Fix re-entrance problem due to panic on NMI
Message-ID<qDYGu-7Sr-7@gated-at.bofh.it>
In reply to#1288120
If panic on NMI happens just after panic() on the same CPU, panic()
is recursively called.  As the result, it stalls after failing to
acquire panic_lock.

To avoid this problem, don't call panic() in NMI context if
we've already entered panic().

V6:
- Add a comment about panic_cpu
- Replace the magic number -1 for panic_cpu with a macro

V4:
- Improve comments in io_check_error() and panic()

V3:
- Introduce nmi_panic() macro to reduce code duplication
- In the case of panic on NMI, don't return from NMI handlers
  if another CPU already panicked

V2:
- Use atomic_cmpxchg() instead of current spin_trylock() to
  exclude concurrent accesses to the panic routines
- Don't introduce no-lock version of panic()

Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Michal Hocko <mhocko@kernel.org>
---
 arch/x86/kernel/nmi.c  |   16 ++++++++++++----
 include/linux/kernel.h |   21 +++++++++++++++++++++
 kernel/panic.c         |   15 ++++++++++++---
 kernel/watchdog.c      |    2 +-
 4 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
index 697f90d..5131714 100644
--- a/arch/x86/kernel/nmi.c
+++ b/arch/x86/kernel/nmi.c
@@ -231,7 +231,7 @@ pci_serr_error(unsigned char reason, struct pt_regs *regs)
 #endif
 
 	if (panic_on_unrecovered_nmi)
-		panic("NMI: Not continuing");
+		nmi_panic("NMI: Not continuing");
 
 	pr_emerg("Dazed and confused, but trying to continue\n");
 
@@ -255,8 +255,16 @@ io_check_error(unsigned char reason, struct pt_regs *regs)
 		 reason, smp_processor_id());
 	show_regs(regs);
 
-	if (panic_on_io_nmi)
-		panic("NMI IOCK error: Not continuing");
+	if (panic_on_io_nmi) {
+		nmi_panic("NMI IOCK error: Not continuing");
+
+		/*
+		 * If we return from nmi_panic(), it means we have received
+		 * NMI while processing panic().  So, simply return without
+		 * a delay and re-enabling NMI.
+		 */
+		return;
+	}
 
 	/* Re-enable the IOCK line, wait for a few seconds */
 	reason = (reason & NMI_REASON_CLEAR_MASK) | NMI_REASON_CLEAR_IOCHK;
@@ -297,7 +305,7 @@ unknown_nmi_error(unsigned char reason, struct pt_regs *regs)
 
 	pr_emerg("Do you have a strange power saving mode enabled?\n");
 	if (unknown_nmi_panic || panic_on_unrecovered_nmi)
-		panic("NMI: Not continuing");
+		nmi_panic("NMI: Not continuing");
 
 	pr_emerg("Dazed and confused, but trying to continue\n");
 }
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 350dfb0..db66867 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -446,6 +446,27 @@ extern int sysctl_panic_on_stackoverflow;
 extern bool crash_kexec_post_notifiers;
 
 /*
+ * panic_cpu holds a panicking CPU number and is used for exclusive
+ * execution of panic and crash_kexec routines. If the value is
+ * PANIC_CPU_INVALID, it means that none of CPU has entered panic or
+ * crash_kexec.
+ */
+extern atomic_t panic_cpu;
+#define PANIC_CPU_INVALID	-1
+
+/*
+ * A variant of panic() called from NMI context.
+ * If we've already panicked on this CPU, return from here.
+ */
+#define nmi_panic(fmt, ...)						\
+	do {								\
+		int this_cpu = raw_smp_processor_id();			\
+		if (atomic_cmpxchg(&panic_cpu, PANIC_CPU_INVALID, this_cpu) \
+		    != this_cpu)					\
+			panic(fmt, ##__VA_ARGS__);			\
+	} while (0)
+
+/*
  * Only to be used by arch init code. If the user over-wrote the default
  * CONFIG_PANIC_TIMEOUT, honor it.
  */
diff --git a/kernel/panic.c b/kernel/panic.c
index 4b150bc..3261e2d 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -61,6 +61,8 @@ void __weak panic_smp_self_stop(void)
 		cpu_relax();
 }
 
+atomic_t panic_cpu = ATOMIC_INIT(PANIC_CPU_INVALID);
+
 /**
  *	panic - halt the system
  *	@fmt: The text string to print
@@ -71,17 +73,17 @@ void __weak panic_smp_self_stop(void)
  */
 void panic(const char *fmt, ...)
 {
-	static DEFINE_SPINLOCK(panic_lock);
 	static char buf[1024];
 	va_list args;
 	long i, i_next = 0;
 	int state = 0;
+	int old_cpu, this_cpu;
 
 	/*
 	 * Disable local interrupts. This will prevent panic_smp_self_stop
 	 * from deadlocking the first cpu that invokes the panic, since
 	 * there is nothing to prevent an interrupt handler (that runs
-	 * after the panic_lock is acquired) from invoking panic again.
+	 * after setting panic_cpu) from invoking panic again.
 	 */
 	local_irq_disable();
 
@@ -94,8 +96,15 @@ void panic(const char *fmt, ...)
 	 * multiple parallel invocations of panic, all other CPUs either
 	 * stop themself or will wait until they are stopped by the 1st CPU
 	 * with smp_send_stop().
+	 *
+	 * `old_cpu == PANIC_CPU_INVALID' means this is the 1st CPU which
+	 * comes here, so go ahead.
+	 * `old_cpu == this_cpu' means we came from nmi_panic() which sets
+	 * panic_cpu to this CPU.  In this case, this is also the 1st CPU.
 	 */
-	if (!spin_trylock(&panic_lock))
+	this_cpu = raw_smp_processor_id();
+	old_cpu = atomic_cmpxchg(&panic_cpu, PANIC_CPU_INVALID, this_cpu);
+	if (old_cpu != PANIC_CPU_INVALID && old_cpu != this_cpu)
 		panic_smp_self_stop();
 
 	console_verbose();
diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 18f34cf..b9be18f 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -351,7 +351,7 @@ static void watchdog_overflow_callback(struct perf_event *event,
 			trigger_allbutself_cpu_backtrace();
 
 		if (hardlockup_panic)
-			panic("Hard LOCKUP");
+			nmi_panic("Hard LOCKUP");
 
 		__this_cpu_write(hard_watchdog_warn, true);
 		return;


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1288620 — Re: [V6 PATCH 1/6] panic/x86: Fix re-entrance problem due to panic on NMI

FromBorislav Petkov <bp@alien8.de>
Date2015-12-10 16:50 +0100
SubjectRe: [V6 PATCH 1/6] panic/x86: Fix re-entrance problem due to panic on NMI
Message-ID<qEbDH-87n-1@gated-at.bofh.it>
In reply to#1288126
On Thu, Dec 10, 2015 at 10:46:26AM +0900, Hidehiro Kawai wrote:
> If panic on NMI happens just after panic() on the same CPU, panic()
> is recursively called.  As the result, it stalls after failing to
> acquire panic_lock.
> 
> To avoid this problem, don't call panic() in NMI context if
> we've already entered panic().
> 
> V6:
> - Add a comment about panic_cpu
> - Replace the magic number -1 for panic_cpu with a macro
> 
> V4:
> - Improve comments in io_check_error() and panic()
> 
> V3:
> - Introduce nmi_panic() macro to reduce code duplication
> - In the case of panic on NMI, don't return from NMI handlers
>   if another CPU already panicked
> 
> V2:
> - Use atomic_cmpxchg() instead of current spin_trylock() to
>   exclude concurrent accesses to the panic routines
> - Don't introduce no-lock version of panic()
> 
> Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Michal Hocko <mhocko@kernel.org>
> ---
>  arch/x86/kernel/nmi.c  |   16 ++++++++++++----
>  include/linux/kernel.h |   21 +++++++++++++++++++++
>  kernel/panic.c         |   15 ++++++++++++---
>  kernel/watchdog.c      |    2 +-
>  4 files changed, 46 insertions(+), 8 deletions(-)

Looks better.

Did some comments cleanup and nmi_panic() macro reformatting so that it
is more readable and ended up applying this:

---
From: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
Date: Thu, 10 Dec 2015 10:46:26 +0900
Subject: [PATCH] panic, x86: Fix re-entrance problem due to panic on NMI

If panic on NMI happens just after panic() on the same CPU, panic() is
recursively called. Kernel stalls, as a result, after failing to acquire
panic_lock.

To avoid this problem, don't call panic() in NMI context if we've
already entered panic().

For that, introduce nmi_panic() macro to reduce code duplication. In
the case of panic on NMI, don't return from NMI handlers if another CPU
already panicked.

Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Cc: Aaron Tomlin <atomlin@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Chris Metcalf <cmetcalf@ezchip.com>
Cc: David Hildenbrand <dahi@linux.vnet.ibm.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Gobinda Charan Maji <gobinda.cemk07@gmail.com>
Cc: HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Javi Merino <javi.merino@arm.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: kexec@lists.infradead.org
Cc: linux-doc@vger.kernel.org
Cc: lkml <linux-kernel@vger.kernel.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Michal Nazarewicz <mina86@mina86.com>
Cc: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Seth Jennings <sjenning@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ulrich Obergfell <uobergfe@redhat.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Vivek Goyal <vgoyal@redhat.com>
Link: http://lkml.kernel.org/r/20151210014626.25437.13302.stgit@softrs
[ Cleanup comments, fixup formatting. ]
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/nmi.c  | 16 ++++++++++++----
 include/linux/kernel.h | 20 ++++++++++++++++++++
 kernel/panic.c         | 16 +++++++++++++---
 kernel/watchdog.c      |  2 +-
 4 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
index 697f90db0e37..292a24bd0553 100644
--- a/arch/x86/kernel/nmi.c
+++ b/arch/x86/kernel/nmi.c
@@ -231,7 +231,7 @@ pci_serr_error(unsigned char reason, struct pt_regs *regs)
 #endif
 
 	if (panic_on_unrecovered_nmi)
-		panic("NMI: Not continuing");
+		nmi_panic("NMI: Not continuing");
 
 	pr_emerg("Dazed and confused, but trying to continue\n");
 
@@ -255,8 +255,16 @@ io_check_error(unsigned char reason, struct pt_regs *regs)
 		 reason, smp_processor_id());
 	show_regs(regs);
 
-	if (panic_on_io_nmi)
-		panic("NMI IOCK error: Not continuing");
+	if (panic_on_io_nmi) {
+		nmi_panic("NMI IOCK error: Not continuing");
+
+		/*
+		 * If end up here, it means we have received an NMI while
+		 * processing panic(). Simply return without delaying and
+		 * re-enabling NMI.
+		 */
+		return;
+	}
 
 	/* Re-enable the IOCK line, wait for a few seconds */
 	reason = (reason & NMI_REASON_CLEAR_MASK) | NMI_REASON_CLEAR_IOCHK;
@@ -297,7 +305,7 @@ unknown_nmi_error(unsigned char reason, struct pt_regs *regs)
 
 	pr_emerg("Do you have a strange power saving mode enabled?\n");
 	if (unknown_nmi_panic || panic_on_unrecovered_nmi)
-		panic("NMI: Not continuing");
+		nmi_panic("NMI: Not continuing");
 
 	pr_emerg("Dazed and confused, but trying to continue\n");
 }
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 350dfb08aee3..750cc5c7c999 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -446,6 +446,26 @@ extern int sysctl_panic_on_stackoverflow;
 extern bool crash_kexec_post_notifiers;
 
 /*
+ * panic_cpu is used for synchronizing panic() and crash_kexec() execution. It
+ * holds a CPU number which is executing panic() currently. A value of
+ * PANIC_CPU_INVALID means no CPU has entered panic() or crash_kexec().
+ */
+extern atomic_t panic_cpu;
+#define PANIC_CPU_INVALID	-1
+
+/*
+ * A variant of panic() called from NMI context. We return if we've already
+ * panicked on this CPU.
+ */
+#define nmi_panic(fmt, ...)						\
+do {									\
+	int cpu = raw_smp_processor_id();				\
+									\
+	if (atomic_cmpxchg(&panic_cpu, PANIC_CPU_INVALID, cpu) != cpu)	\
+		panic(fmt, ##__VA_ARGS__);				\
+} while (0)
+
+/*
  * Only to be used by arch init code. If the user over-wrote the default
  * CONFIG_PANIC_TIMEOUT, honor it.
  */
diff --git a/kernel/panic.c b/kernel/panic.c
index 4b150bc0c6c1..3344524cf6ff 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -61,6 +61,8 @@ void __weak panic_smp_self_stop(void)
 		cpu_relax();
 }
 
+atomic_t panic_cpu = ATOMIC_INIT(PANIC_CPU_INVALID);
+
 /**
  *	panic - halt the system
  *	@fmt: The text string to print
@@ -71,17 +73,17 @@ void __weak panic_smp_self_stop(void)
  */
 void panic(const char *fmt, ...)
 {
-	static DEFINE_SPINLOCK(panic_lock);
 	static char buf[1024];
 	va_list args;
 	long i, i_next = 0;
 	int state = 0;
+	int old_cpu, this_cpu;
 
 	/*
 	 * Disable local interrupts. This will prevent panic_smp_self_stop
 	 * from deadlocking the first cpu that invokes the panic, since
 	 * there is nothing to prevent an interrupt handler (that runs
-	 * after the panic_lock is acquired) from invoking panic again.
+	 * after setting panic_cpu) from invoking panic() again.
 	 */
 	local_irq_disable();
 
@@ -94,8 +96,16 @@ void panic(const char *fmt, ...)
 	 * multiple parallel invocations of panic, all other CPUs either
 	 * stop themself or will wait until they are stopped by the 1st CPU
 	 * with smp_send_stop().
+	 *
+	 * `old_cpu == PANIC_CPU_INVALID' means this is the 1st CPU which
+	 * comes here, so go ahead.
+	 * `old_cpu == this_cpu' means we came from nmi_panic() which sets
+	 * panic_cpu to this CPU.  In this case, this is also the 1st CPU.
 	 */
-	if (!spin_trylock(&panic_lock))
+	this_cpu = raw_smp_processor_id();
+	old_cpu  = atomic_cmpxchg(&panic_cpu, PANIC_CPU_INVALID, this_cpu);
+
+	if (old_cpu != PANIC_CPU_INVALID && old_cpu != this_cpu)
 		panic_smp_self_stop();
 
 	console_verbose();
diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 18f34cf75f74..b9be18fae154 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -351,7 +351,7 @@ static void watchdog_overflow_callback(struct perf_event *event,
 			trigger_allbutself_cpu_backtrace();
 
 		if (hardlockup_panic)
-			panic("Hard LOCKUP");
+			nmi_panic("Hard LOCKUP");
 
 		__this_cpu_write(hard_watchdog_warn, true);
 		return;
-- 
2.3.5

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1289031 — RE: Re: [V6 PATCH 1/6] panic/x86: Fix re-entrance problem due to panic on NMI

From河合英宏 / KAWAI，HIDEHIRO <hidehiro.kawai.ez@hitachi.com>
Date2015-12-11 01:30 +0100
SubjectRE: Re: [V6 PATCH 1/6] panic/x86: Fix re-entrance problem due to panic on NMI
Message-ID<qEjKW-57Q-1@gated-at.bofh.it>
In reply to#1288620
PiBGcm9tOiBCb3Jpc2xhdiBQZXRrb3YgW21haWx0bzpicEBhbGllbjguZGVdDQpbLi4uXQ0KPiBM
b29rcyBiZXR0ZXIuDQo+IA0KPiBEaWQgc29tZSBjb21tZW50cyBjbGVhbnVwIGFuZCBubWlfcGFu
aWMoKSBtYWNybyByZWZvcm1hdHRpbmcgc28gdGhhdCBpdA0KPiBpcyBtb3JlIHJlYWRhYmxlIGFu
ZCBlbmRlZCB1cCBhcHBseWluZyB0aGlzOg0KDQpUaGFua3MgYSBsb3QhDQoNCkhpZGVoaXJvIEth
d2FpDQpIaXRhY2hpLCBMdGQuIFJlc2VhcmNoICYgRGV2ZWxvcG1lbnQgR3JvdXANCg0KPiAtLS0N
Cj4gRnJvbTogSGlkZWhpcm8gS2F3YWkgPGhpZGVoaXJvLmthd2FpLmV6QGhpdGFjaGkuY29tPg0K
PiBEYXRlOiBUaHUsIDEwIERlYyAyMDE1IDEwOjQ2OjI2ICswOTAwDQo+IFN1YmplY3Q6IFtQQVRD
SF0gcGFuaWMsIHg4NjogRml4IHJlLWVudHJhbmNlIHByb2JsZW0gZHVlIHRvIHBhbmljIG9uIE5N
SQ0KPiANCj4gSWYgcGFuaWMgb24gTk1JIGhhcHBlbnMganVzdCBhZnRlciBwYW5pYygpIG9uIHRo
ZSBzYW1lIENQVSwgcGFuaWMoKSBpcw0KPiByZWN1cnNpdmVseSBjYWxsZWQuIEtlcm5lbCBzdGFs
bHMsIGFzIGEgcmVzdWx0LCBhZnRlciBmYWlsaW5nIHRvIGFjcXVpcmUNCj4gcGFuaWNfbG9jay4N
Cj4gDQo+IFRvIGF2b2lkIHRoaXMgcHJvYmxlbSwgZG9uJ3QgY2FsbCBwYW5pYygpIGluIE5NSSBj
b250ZXh0IGlmIHdlJ3ZlDQo+IGFscmVhZHkgZW50ZXJlZCBwYW5pYygpLg0KPiANCj4gRm9yIHRo
YXQsIGludHJvZHVjZSBubWlfcGFuaWMoKSBtYWNybyB0byByZWR1Y2UgY29kZSBkdXBsaWNhdGlv
bi4gSW4NCj4gdGhlIGNhc2Ugb2YgcGFuaWMgb24gTk1JLCBkb24ndCByZXR1cm4gZnJvbSBOTUkg
aGFuZGxlcnMgaWYgYW5vdGhlciBDUFUNCj4gYWxyZWFkeSBwYW5pY2tlZC4NCj4gDQo+IFNpZ25l
ZC1vZmYtYnk6IEhpZGVoaXJvIEthd2FpIDxoaWRlaGlyby5rYXdhaS5lekBoaXRhY2hpLmNvbT4N
Cj4gQWNrZWQtYnk6IE1pY2hhbCBIb2NrbyA8bWhvY2tvQHN1c2UuY29tPg0KPiBDYzogQWFyb24g
VG9tbGluIDxhdG9tbGluQHJlZGhhdC5jb20+DQo+IENjOiBBbmRyZXcgTW9ydG9uIDxha3BtQGxp
bnV4LWZvdW5kYXRpb24ub3JnPg0KPiBDYzogQW5keSBMdXRvbWlyc2tpIDxsdXRvQGtlcm5lbC5v
cmc+DQo+IENjOiBCYW9xdWFuIEhlIDxiaGVAcmVkaGF0LmNvbT4NCj4gQ2M6IENocmlzIE1ldGNh
bGYgPGNtZXRjYWxmQGV6Y2hpcC5jb20+DQo+IENjOiBEYXZpZCBIaWxkZW5icmFuZCA8ZGFoaUBs
aW51eC52bmV0LmlibS5jb20+DQo+IENjOiBEb24gWmlja3VzIDxkemlja3VzQHJlZGhhdC5jb20+
DQo+IENjOiAiRXJpYyBXLiBCaWVkZXJtYW4iIDxlYmllZGVybUB4bWlzc2lvbi5jb20+DQo+IENj
OiBGcmVkZXJpYyBXZWlzYmVja2VyIDxmd2Vpc2JlY0BnbWFpbC5jb20+DQo+IENjOiBHb2JpbmRh
IENoYXJhbiBNYWppIDxnb2JpbmRhLmNlbWswN0BnbWFpbC5jb20+DQo+IENjOiBIQVRBWUFNQSBE
YWlzdWtlIDxkLmhhdGF5YW1hQGpwLmZ1aml0c3UuY29tPg0KPiBDYzogIkguIFBldGVyIEFudmlu
IiA8aHBhQHp5dG9yLmNvbT4NCj4gQ2M6IEluZ28gTW9sbmFyIDxtaW5nb0BrZXJuZWwub3JnPg0K
PiBDYzogSmF2aSBNZXJpbm8gPGphdmkubWVyaW5vQGFybS5jb20+DQo+IENjOiBKb25hdGhhbiBD
b3JiZXQgPGNvcmJldEBsd24ubmV0Pg0KPiBDYzoga2V4ZWNAbGlzdHMuaW5mcmFkZWFkLm9yZw0K
PiBDYzogbGludXgtZG9jQHZnZXIua2VybmVsLm9yZw0KPiBDYzogbGttbCA8bGludXgta2VybmVs
QHZnZXIua2VybmVsLm9yZz4NCj4gQ2M6IE1hc2FtaSBIaXJhbWF0c3UgPG1hc2FtaS5oaXJhbWF0
c3UucHRAaGl0YWNoaS5jb20+DQo+IENjOiBNaWNoYWwgTmF6YXJld2ljeiA8bWluYTg2QG1pbmE4
Ni5jb20+DQo+IENjOiBOaWNvbGFzIElvb3NzIDxuaWNvbGFzLmlvb3NzX2xpbnV4QG00eC5vcmc+
DQo+IENjOiBQZXRlciBaaWpsc3RyYSA8cGV0ZXJ6QGluZnJhZGVhZC5vcmc+DQo+IENjOiBQcmFy
aXQgQmhhcmdhdmEgPHByYXJpdEByZWRoYXQuY29tPg0KPiBDYzogUmFzbXVzIFZpbGxlbW9lcyA8
bGludXhAcmFzbXVzdmlsbGVtb2VzLmRrPg0KPiBDYzogUnVzdHkgUnVzc2VsbCA8cnVzdHlAcnVz
dGNvcnAuY29tLmF1Pg0KPiBDYzogU2V0aCBKZW5uaW5ncyA8c2plbm5pbmdAcmVkaGF0LmNvbT4N
Cj4gQ2M6IFN0ZXZlbiBSb3N0ZWR0IDxyb3N0ZWR0QGdvb2RtaXMub3JnPg0KPiBDYzogVGhvbWFz
IEdsZWl4bmVyIDx0Z2x4QGxpbnV0cm9uaXguZGU+DQo+IENjOiBVbHJpY2ggT2JlcmdmZWxsIDx1
b2JlcmdmZUByZWRoYXQuY29tPg0KPiBDYzogVml0YWx5IEt1em5ldHNvdiA8dmt1em5ldHNAcmVk
aGF0LmNvbT4NCj4gQ2M6IFZpdmVrIEdveWFsIDx2Z295YWxAcmVkaGF0LmNvbT4NCj4gTGluazog
aHR0cDovL2xrbWwua2VybmVsLm9yZy9yLzIwMTUxMjEwMDE0NjI2LjI1NDM3LjEzMzAyLnN0Z2l0
QHNvZnRycw0KPiBbIENsZWFudXAgY29tbWVudHMsIGZpeHVwIGZvcm1hdHRpbmcuIF0NCj4gU2ln
bmVkLW9mZi1ieTogQm9yaXNsYXYgUGV0a292IDxicEBzdXNlLmRlPg0KPiAtLS0NCj4gIGFyY2gv
eDg2L2tlcm5lbC9ubWkuYyAgfCAxNiArKysrKysrKysrKystLS0tDQo+ICBpbmNsdWRlL2xpbnV4
L2tlcm5lbC5oIHwgMjAgKysrKysrKysrKysrKysrKysrKysNCj4gIGtlcm5lbC9wYW5pYy5jICAg
ICAgICAgfCAxNiArKysrKysrKysrKysrLS0tDQo+ICBrZXJuZWwvd2F0Y2hkb2cuYyAgICAgIHwg
IDIgKy0NCj4gIDQgZmlsZXMgY2hhbmdlZCwgNDYgaW5zZXJ0aW9ucygrKSwgOCBkZWxldGlvbnMo
LSkNCj4gDQo+IGRpZmYgLS1naXQgYS9hcmNoL3g4Ni9rZXJuZWwvbm1pLmMgYi9hcmNoL3g4Ni9r
ZXJuZWwvbm1pLmMNCj4gaW5kZXggNjk3ZjkwZGIwZTM3Li4yOTJhMjRiZDA1NTMgMTAwNjQ0DQo+
IC0tLSBhL2FyY2gveDg2L2tlcm5lbC9ubWkuYw0KPiArKysgYi9hcmNoL3g4Ni9rZXJuZWwvbm1p
LmMNCj4gQEAgLTIzMSw3ICsyMzEsNyBAQCBwY2lfc2Vycl9lcnJvcih1bnNpZ25lZCBjaGFyIHJl
YXNvbiwgc3RydWN0IHB0X3JlZ3MgKnJlZ3MpDQo+ICAjZW5kaWYNCj4gDQo+ICAJaWYgKHBhbmlj
X29uX3VucmVjb3ZlcmVkX25taSkNCj4gLQkJcGFuaWMoIk5NSTogTm90IGNvbnRpbnVpbmciKTsN
Cj4gKwkJbm1pX3BhbmljKCJOTUk6IE5vdCBjb250aW51aW5nIik7DQo+IA0KPiAgCXByX2VtZXJn
KCJEYXplZCBhbmQgY29uZnVzZWQsIGJ1dCB0cnlpbmcgdG8gY29udGludWVcbiIpOw0KPiANCj4g
QEAgLTI1NSw4ICsyNTUsMTYgQEAgaW9fY2hlY2tfZXJyb3IodW5zaWduZWQgY2hhciByZWFzb24s
IHN0cnVjdCBwdF9yZWdzICpyZWdzKQ0KPiAgCQkgcmVhc29uLCBzbXBfcHJvY2Vzc29yX2lkKCkp
Ow0KPiAgCXNob3dfcmVncyhyZWdzKTsNCj4gDQo+IC0JaWYgKHBhbmljX29uX2lvX25taSkNCj4g
LQkJcGFuaWMoIk5NSSBJT0NLIGVycm9yOiBOb3QgY29udGludWluZyIpOw0KPiArCWlmIChwYW5p
Y19vbl9pb19ubWkpIHsNCj4gKwkJbm1pX3BhbmljKCJOTUkgSU9DSyBlcnJvcjogTm90IGNvbnRp
bnVpbmciKTsNCj4gKw0KPiArCQkvKg0KPiArCQkgKiBJZiBlbmQgdXAgaGVyZSwgaXQgbWVhbnMg
d2UgaGF2ZSByZWNlaXZlZCBhbiBOTUkgd2hpbGUNCj4gKwkJICogcHJvY2Vzc2luZyBwYW5pYygp
LiBTaW1wbHkgcmV0dXJuIHdpdGhvdXQgZGVsYXlpbmcgYW5kDQo+ICsJCSAqIHJlLWVuYWJsaW5n
IE5NSS4NCj4gKwkJICovDQo+ICsJCXJldHVybjsNCj4gKwl9DQo+IA0KPiAgCS8qIFJlLWVuYWJs
ZSB0aGUgSU9DSyBsaW5lLCB3YWl0IGZvciBhIGZldyBzZWNvbmRzICovDQo+ICAJcmVhc29uID0g
KHJlYXNvbiAmIE5NSV9SRUFTT05fQ0xFQVJfTUFTSykgfCBOTUlfUkVBU09OX0NMRUFSX0lPQ0hL
Ow0KPiBAQCAtMjk3LDcgKzMwNSw3IEBAIHVua25vd25fbm1pX2Vycm9yKHVuc2lnbmVkIGNoYXIg
cmVhc29uLCBzdHJ1Y3QgcHRfcmVncyAqcmVncykNCj4gDQo+ICAJcHJfZW1lcmcoIkRvIHlvdSBo
YXZlIGEgc3RyYW5nZSBwb3dlciBzYXZpbmcgbW9kZSBlbmFibGVkP1xuIik7DQo+ICAJaWYgKHVu
a25vd25fbm1pX3BhbmljIHx8IHBhbmljX29uX3VucmVjb3ZlcmVkX25taSkNCj4gLQkJcGFuaWMo
Ik5NSTogTm90IGNvbnRpbnVpbmciKTsNCj4gKwkJbm1pX3BhbmljKCJOTUk6IE5vdCBjb250aW51
aW5nIik7DQo+IA0KPiAgCXByX2VtZXJnKCJEYXplZCBhbmQgY29uZnVzZWQsIGJ1dCB0cnlpbmcg
dG8gY29udGludWVcbiIpOw0KPiAgfQ0KPiBkaWZmIC0tZ2l0IGEvaW5jbHVkZS9saW51eC9rZXJu
ZWwuaCBiL2luY2x1ZGUvbGludXgva2VybmVsLmgNCj4gaW5kZXggMzUwZGZiMDhhZWUzLi43NTBj
YzVjN2M5OTkgMTAwNjQ0DQo+IC0tLSBhL2luY2x1ZGUvbGludXgva2VybmVsLmgNCj4gKysrIGIv
aW5jbHVkZS9saW51eC9rZXJuZWwuaA0KPiBAQCAtNDQ2LDYgKzQ0NiwyNiBAQCBleHRlcm4gaW50
IHN5c2N0bF9wYW5pY19vbl9zdGFja292ZXJmbG93Ow0KPiAgZXh0ZXJuIGJvb2wgY3Jhc2hfa2V4
ZWNfcG9zdF9ub3RpZmllcnM7DQo+IA0KPiAgLyoNCj4gKyAqIHBhbmljX2NwdSBpcyB1c2VkIGZv
ciBzeW5jaHJvbml6aW5nIHBhbmljKCkgYW5kIGNyYXNoX2tleGVjKCkgZXhlY3V0aW9uLiBJdA0K
PiArICogaG9sZHMgYSBDUFUgbnVtYmVyIHdoaWNoIGlzIGV4ZWN1dGluZyBwYW5pYygpIGN1cnJl
bnRseS4gQSB2YWx1ZSBvZg0KPiArICogUEFOSUNfQ1BVX0lOVkFMSUQgbWVhbnMgbm8gQ1BVIGhh
cyBlbnRlcmVkIHBhbmljKCkgb3IgY3Jhc2hfa2V4ZWMoKS4NCj4gKyAqLw0KPiArZXh0ZXJuIGF0
b21pY190IHBhbmljX2NwdTsNCj4gKyNkZWZpbmUgUEFOSUNfQ1BVX0lOVkFMSUQJLTENCj4gKw0K
PiArLyoNCj4gKyAqIEEgdmFyaWFudCBvZiBwYW5pYygpIGNhbGxlZCBmcm9tIE5NSSBjb250ZXh0
LiBXZSByZXR1cm4gaWYgd2UndmUgYWxyZWFkeQ0KPiArICogcGFuaWNrZWQgb24gdGhpcyBDUFUu
DQo+ICsgKi8NCj4gKyNkZWZpbmUgbm1pX3BhbmljKGZtdCwgLi4uKQkJCQkJCVwNCj4gK2RvIHsJ
CQkJCQkJCQlcDQo+ICsJaW50IGNwdSA9IHJhd19zbXBfcHJvY2Vzc29yX2lkKCk7CQkJCVwNCj4g
KwkJCQkJCQkJCVwNCj4gKwlpZiAoYXRvbWljX2NtcHhjaGcoJnBhbmljX2NwdSwgUEFOSUNfQ1BV
X0lOVkFMSUQsIGNwdSkgIT0gY3B1KQlcDQo+ICsJCXBhbmljKGZtdCwgIyNfX1ZBX0FSR1NfXyk7
CQkJCVwNCj4gK30gd2hpbGUgKDApDQo+ICsNCj4gKy8qDQo+ICAgKiBPbmx5IHRvIGJlIHVzZWQg
YnkgYXJjaCBpbml0IGNvZGUuIElmIHRoZSB1c2VyIG92ZXItd3JvdGUgdGhlIGRlZmF1bHQNCj4g
ICAqIENPTkZJR19QQU5JQ19USU1FT1VULCBob25vciBpdC4NCj4gICAqLw0KPiBkaWZmIC0tZ2l0
IGEva2VybmVsL3BhbmljLmMgYi9rZXJuZWwvcGFuaWMuYw0KPiBpbmRleCA0YjE1MGJjMGM2YzEu
LjMzNDQ1MjRjZjZmZiAxMDA2NDQNCj4gLS0tIGEva2VybmVsL3BhbmljLmMNCj4gKysrIGIva2Vy
bmVsL3BhbmljLmMNCj4gQEAgLTYxLDYgKzYxLDggQEAgdm9pZCBfX3dlYWsgcGFuaWNfc21wX3Nl
bGZfc3RvcCh2b2lkKQ0KPiAgCQljcHVfcmVsYXgoKTsNCj4gIH0NCj4gDQo+ICthdG9taWNfdCBw
YW5pY19jcHUgPSBBVE9NSUNfSU5JVChQQU5JQ19DUFVfSU5WQUxJRCk7DQo+ICsNCj4gIC8qKg0K
PiAgICoJcGFuaWMgLSBoYWx0IHRoZSBzeXN0ZW0NCj4gICAqCUBmbXQ6IFRoZSB0ZXh0IHN0cmlu
ZyB0byBwcmludA0KPiBAQCAtNzEsMTcgKzczLDE3IEBAIHZvaWQgX193ZWFrIHBhbmljX3NtcF9z
ZWxmX3N0b3Aodm9pZCkNCj4gICAqLw0KPiAgdm9pZCBwYW5pYyhjb25zdCBjaGFyICpmbXQsIC4u
LikNCj4gIHsNCj4gLQlzdGF0aWMgREVGSU5FX1NQSU5MT0NLKHBhbmljX2xvY2spOw0KPiAgCXN0
YXRpYyBjaGFyIGJ1ZlsxMDI0XTsNCj4gIAl2YV9saXN0IGFyZ3M7DQo+ICAJbG9uZyBpLCBpX25l
eHQgPSAwOw0KPiAgCWludCBzdGF0ZSA9IDA7DQo+ICsJaW50IG9sZF9jcHUsIHRoaXNfY3B1Ow0K
PiANCj4gIAkvKg0KPiAgCSAqIERpc2FibGUgbG9jYWwgaW50ZXJydXB0cy4gVGhpcyB3aWxsIHBy
ZXZlbnQgcGFuaWNfc21wX3NlbGZfc3RvcA0KPiAgCSAqIGZyb20gZGVhZGxvY2tpbmcgdGhlIGZp
cnN0IGNwdSB0aGF0IGludm9rZXMgdGhlIHBhbmljLCBzaW5jZQ0KPiAgCSAqIHRoZXJlIGlzIG5v
dGhpbmcgdG8gcHJldmVudCBhbiBpbnRlcnJ1cHQgaGFuZGxlciAodGhhdCBydW5zDQo+IC0JICog
YWZ0ZXIgdGhlIHBhbmljX2xvY2sgaXMgYWNxdWlyZWQpIGZyb20gaW52b2tpbmcgcGFuaWMgYWdh
aW4uDQo+ICsJICogYWZ0ZXIgc2V0dGluZyBwYW5pY19jcHUpIGZyb20gaW52b2tpbmcgcGFuaWMo
KSBhZ2Fpbi4NCj4gIAkgKi8NCj4gIAlsb2NhbF9pcnFfZGlzYWJsZSgpOw0KPiANCj4gQEAgLTk0
LDggKzk2LDE2IEBAIHZvaWQgcGFuaWMoY29uc3QgY2hhciAqZm10LCAuLi4pDQo+ICAJICogbXVs
dGlwbGUgcGFyYWxsZWwgaW52b2NhdGlvbnMgb2YgcGFuaWMsIGFsbCBvdGhlciBDUFVzIGVpdGhl
cg0KPiAgCSAqIHN0b3AgdGhlbXNlbGYgb3Igd2lsbCB3YWl0IHVudGlsIHRoZXkgYXJlIHN0b3Bw
ZWQgYnkgdGhlIDFzdCBDUFUNCj4gIAkgKiB3aXRoIHNtcF9zZW5kX3N0b3AoKS4NCj4gKwkgKg0K
PiArCSAqIGBvbGRfY3B1ID09IFBBTklDX0NQVV9JTlZBTElEJyBtZWFucyB0aGlzIGlzIHRoZSAx
c3QgQ1BVIHdoaWNoDQo+ICsJICogY29tZXMgaGVyZSwgc28gZ28gYWhlYWQuDQo+ICsJICogYG9s
ZF9jcHUgPT0gdGhpc19jcHUnIG1lYW5zIHdlIGNhbWUgZnJvbSBubWlfcGFuaWMoKSB3aGljaCBz
ZXRzDQo+ICsJICogcGFuaWNfY3B1IHRvIHRoaXMgQ1BVLiAgSW4gdGhpcyBjYXNlLCB0aGlzIGlz
IGFsc28gdGhlIDFzdCBDUFUuDQo+ICAJICovDQo+IC0JaWYgKCFzcGluX3RyeWxvY2soJnBhbmlj
X2xvY2spKQ0KPiArCXRoaXNfY3B1ID0gcmF3X3NtcF9wcm9jZXNzb3JfaWQoKTsNCj4gKwlvbGRf
Y3B1ICA9IGF0b21pY19jbXB4Y2hnKCZwYW5pY19jcHUsIFBBTklDX0NQVV9JTlZBTElELCB0aGlz
X2NwdSk7DQo+ICsNCj4gKwlpZiAob2xkX2NwdSAhPSBQQU5JQ19DUFVfSU5WQUxJRCAmJiBvbGRf
Y3B1ICE9IHRoaXNfY3B1KQ0KPiAgCQlwYW5pY19zbXBfc2VsZl9zdG9wKCk7DQo+IA0KPiAgCWNv
bnNvbGVfdmVyYm9zZSgpOw0KPiBkaWZmIC0tZ2l0IGEva2VybmVsL3dhdGNoZG9nLmMgYi9rZXJu
ZWwvd2F0Y2hkb2cuYw0KPiBpbmRleCAxOGYzNGNmNzVmNzQuLmI5YmUxOGZhZTE1NCAxMDA2NDQN
Cj4gLS0tIGEva2VybmVsL3dhdGNoZG9nLmMNCj4gKysrIGIva2VybmVsL3dhdGNoZG9nLmMNCj4g
QEAgLTM1MSw3ICszNTEsNyBAQCBzdGF0aWMgdm9pZCB3YXRjaGRvZ19vdmVyZmxvd19jYWxsYmFj
ayhzdHJ1Y3QgcGVyZl9ldmVudCAqZXZlbnQsDQo+ICAJCQl0cmlnZ2VyX2FsbGJ1dHNlbGZfY3B1
X2JhY2t0cmFjZSgpOw0KPiANCj4gIAkJaWYgKGhhcmRsb2NrdXBfcGFuaWMpDQo+IC0JCQlwYW5p
YygiSGFyZCBMT0NLVVAiKTsNCj4gKwkJCW5taV9wYW5pYygiSGFyZCBMT0NLVVAiKTsNCj4gDQo+
ICAJCV9fdGhpc19jcHVfd3JpdGUoaGFyZF93YXRjaGRvZ193YXJuLCB0cnVlKTsNCj4gIAkJcmV0
dXJuOw0KPiAtLQ0KPiAyLjMuNQ0KPiANCj4gLS0NCj4gUmVnYXJkcy9HcnVzcywNCj4gICAgIEJv
cmlzLg0KPiANCj4gRUNPIHRpcCAjMTAxOiBUcmltIHlvdXIgbWFpbHMgd2hlbiB5b3UgcmVwbHku
DQo=
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1288128 — [V6 PATCH 2/6] panic/x86: Allow CPUs to save registers even if they are looping in NMI context

FromHidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
Date2015-12-10 03:00 +0100
Subject[V6 PATCH 2/6] panic/x86: Allow CPUs to save registers even if they are looping in NMI context
Message-ID<qDYGu-7Sr-9@gated-at.bofh.it>
In reply to#1288120
Currently, kdump_nmi_shootdown_cpus(), a subroutine of crash_kexec(),
sends NMI IPI to non-panic CPUs to stop them and save their register
information and doing some cleanups for crash dumping.  However, if
a non-panic CPU is infinitely looping in NMI context, we fail to
save its register information into the crash dump.

For example, this can happen when unknown NMIs are broadcast to all
CPUs as follows (before applying this patch series):

  CPU 0                             CPU 1
  ===========================       ==========================
  receive an unknown NMI
  unknown_nmi_error()
    panic()                         receive an unknown NMI
      spin_trylock(&panic_lock)     unknown_nmi_error()
      crash_kexec()                   panic()
                                        spin_trylock(&panic_lock)
                                        panic_smp_self_stop()
                                          infinite loop
        kdump_nmi_shootdown_cpus()
          issue NMI IPI -----------> blocked until IRET
                                          infinite loop...

  Here, since CPU 1 is in NMI context, additional NMI from CPU 0
  is blocked until CPU 1 executes IRET.  However, CPU 1 never
  executes IRET, so the NMI is not handled and the callback function
  to save registers is never called.

  Actually, this can happen on some servers which broadcast NMIs to
  all CPUs when the dump button is pushed.

To save registers in this case, we need to:

  a) Return from NMI handler instead of looping infinitely
  or
  b) Call the callback function directly from the infinite loop

Inherently, a) is risky because NMI is also used to prevent corrupted
data from being propagated to devices.  So, we chose b).

This patch does following things:

1. Move the timing of `infinite loop in NMI context' (actually
   done by panic_smp_self_stop()) outside of panic() to enable us to
   refer pt_regs.  Please note that panic_smp_self_stop() is still
   used for normal context
2. Call a callback of kdump_nmi_shootdown_cpus() directly to save
   registers and do some cleanups after setting waiting_for_crash_ipi
   which is used for counting down the number of CPUs which handled
   the callback

V6:
- Revise the commit description and many comments
- Move changes involved with nmi_reason_lock to a later patch because
  it turned out there is no problem at this point
- Rename crash_ipi_done to crash_ipi_issued to clarify its meaning

V5:
- Use WRITE_ONCE() when setting crash_ipi_done to 1 so that the
  compiler doesn't change the instruction order
- Support the case of b in the above description
- Add poll_crash_ipi_and_callback()

V4:
- Rewrite the patch description

V3:
- Newly introduced

Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Eric Biederman <ebiederm@xmission.com>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: Michal Hocko <mhocko@kernel.org>
---
 arch/x86/kernel/nmi.c    |    6 +++---
 arch/x86/kernel/reboot.c |   20 ++++++++++++++++++++
 include/linux/kernel.h   |   14 +++++++++++---
 kernel/panic.c           |   10 ++++++++++
 kernel/watchdog.c        |    2 +-
 5 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
index 5131714..5e00de7 100644
--- a/arch/x86/kernel/nmi.c
+++ b/arch/x86/kernel/nmi.c
@@ -231,7 +231,7 @@ pci_serr_error(unsigned char reason, struct pt_regs *regs)
 #endif
 
 	if (panic_on_unrecovered_nmi)
-		nmi_panic("NMI: Not continuing");
+		nmi_panic(regs, "NMI: Not continuing");
 
 	pr_emerg("Dazed and confused, but trying to continue\n");
 
@@ -256,7 +256,7 @@ io_check_error(unsigned char reason, struct pt_regs *regs)
 	show_regs(regs);
 
 	if (panic_on_io_nmi) {
-		nmi_panic("NMI IOCK error: Not continuing");
+		nmi_panic(regs, "NMI IOCK error: Not continuing");
 
 		/*
 		 * If we return from nmi_panic(), it means we have received
@@ -305,7 +305,7 @@ unknown_nmi_error(unsigned char reason, struct pt_regs *regs)
 
 	pr_emerg("Do you have a strange power saving mode enabled?\n");
 	if (unknown_nmi_panic || panic_on_unrecovered_nmi)
-		nmi_panic("NMI: Not continuing");
+		nmi_panic(regs, "NMI: Not continuing");
 
 	pr_emerg("Dazed and confused, but trying to continue\n");
 }
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 02693dd..1da1302 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -718,6 +718,7 @@ static int crashing_cpu;
 static nmi_shootdown_cb shootdown_callback;
 
 static atomic_t waiting_for_crash_ipi;
+static int crash_ipi_issued;
 
 static int crash_nmi_callback(unsigned int val, struct pt_regs *regs)
 {
@@ -780,6 +781,9 @@ void nmi_shootdown_cpus(nmi_shootdown_cb callback)
 
 	smp_send_nmi_allbutself();
 
+	/* Kick CPUs looping in NMI context. */
+	WRITE_ONCE(crash_ipi_issued, 1);
+
 	msecs = 1000; /* Wait at most a second for the other cpus to stop */
 	while ((atomic_read(&waiting_for_crash_ipi) > 0) && msecs) {
 		mdelay(1);
@@ -788,6 +792,22 @@ void nmi_shootdown_cpus(nmi_shootdown_cb callback)
 
 	/* Leave the nmi callback set */
 }
+
+/* Override the weak function in kernel/panic.c */
+void nmi_panic_self_stop(struct pt_regs *regs)
+{
+	while (1) {
+		/*
+		 * Wait for the crash dumping IPI to be issued, and then
+		 * call its callback directly.
+		 */
+		if (READ_ONCE(crash_ipi_issued))
+			crash_nmi_callback(0, regs); /* Don't return */
+
+		cpu_relax();
+	}
+}
+
 #else /* !CONFIG_SMP */
 void nmi_shootdown_cpus(nmi_shootdown_cb callback)
 {
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index db66867..f28eebb 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -255,6 +255,7 @@ extern long (*panic_blink)(int state);
 __printf(1, 2)
 void panic(const char *fmt, ...)
 	__noreturn __cold;
+void nmi_panic_self_stop(struct pt_regs *);
 extern void oops_enter(void);
 extern void oops_exit(void);
 void print_oops_end_marker(void);
@@ -457,13 +458,20 @@ extern atomic_t panic_cpu;
 /*
  * A variant of panic() called from NMI context.
  * If we've already panicked on this CPU, return from here.
+ * If another CPU already panicked, loop in nmi_panic_self_stop() which
+ * can provide architecture dependent code such as saving register states
+ * for crash dump.
  */
-#define nmi_panic(fmt, ...)						\
+#define nmi_panic(regs, fmt, ...)					\
 	do {								\
+		int old_cpu;						\
 		int this_cpu = raw_smp_processor_id();			\
-		if (atomic_cmpxchg(&panic_cpu, PANIC_CPU_INVALID, this_cpu) \
-		    != this_cpu)					\
+		old_cpu = atomic_cmpxchg(&panic_cpu, PANIC_CPU_INVALID,	\
+					 this_cpu);			\
+		if (old_cpu == PANIC_CPU_INVALID)			\
 			panic(fmt, ##__VA_ARGS__);			\
+		else if (old_cpu != this_cpu)				\
+			nmi_panic_self_stop(regs);			\
 	} while (0)
 
 /*
diff --git a/kernel/panic.c b/kernel/panic.c
index 3261e2d..3d6c3f1 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -61,6 +61,16 @@ void __weak panic_smp_self_stop(void)
 		cpu_relax();
 }
 
+/*
+ * Stop ourselves in NMI context if another cpu has already panicked.
+ * Architecture code may override this to prepare for crash dumping
+ * (e.g. save register information).
+ */
+void __weak nmi_panic_self_stop(struct pt_regs *regs)
+{
+	panic_smp_self_stop();
+}
+
 atomic_t panic_cpu = ATOMIC_INIT(PANIC_CPU_INVALID);
 
 /**
diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index b9be18f..84b5035 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -351,7 +351,7 @@ static void watchdog_overflow_callback(struct perf_event *event,
 			trigger_allbutself_cpu_backtrace();
 
 		if (hardlockup_panic)
-			nmi_panic("Hard LOCKUP");
+			nmi_panic(regs, "Hard LOCKUP");
 
 		__this_cpu_write(hard_watchdog_warn, true);
 		return;


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1288129 — [V6 PATCH 6/6] Documentation: Add documentation for kernel.panic_on_io_nmi sysctl

FromHidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
Date2015-12-10 03:00 +0100
Subject[V6 PATCH 6/6] Documentation: Add documentation for kernel.panic_on_io_nmi sysctl
Message-ID<qDYGu-7Sr-11@gated-at.bofh.it>
In reply to#1288120
kernel.panic_on_io_nmi sysctl was introduced by commit 5211a242d0cb
("x86: Add sysctl to allow panic on IOCK NMI error"), but its
documentation is missing. So, add it.

V6:
- Newly added

Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 Documentation/sysctl/kernel.txt |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
index af70d15..235f804 100644
--- a/Documentation/sysctl/kernel.txt
+++ b/Documentation/sysctl/kernel.txt
@@ -551,6 +551,21 @@ the recommended setting is 60.
 
 ==============================================================
 
+panic_on_io_nmi:
+
+Controls the kernel's behavior when a CPU receives an NMI caused by
+an IO error.
+
+0: try to continue operation (default)
+
+1: panic immediately. The IO error triggered NMI indicates a serious
+   system condition, which could result in IO data corruption. Rather
+   than continuing, panicking might be a better choice. Some servers
+   issue this sort of NMI when the dump button is pushed, and you
+   can use this option to take a crash dump
+
+==============================================================
+
 panic_on_oops:
 
 Controls the kernel's behaviour when an oops or BUG is encountered.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web