All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
To: Jonathan Corbet <corbet@lwn.net>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Vivek Goyal <vgoyal@redhat.com>
Cc: linux-doc@vger.kernel.org, x86@kernel.org,
	kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
	Michal Hocko <mhocko@kernel.org>, Ingo Molnar <mingo@redhat.com>,
	Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Subject: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI
Date: Mon, 27 Jul 2015 10:58:50 +0900	[thread overview]
Message-ID: <20150727015850.4928.50289.stgit@softrs> (raw)
In-Reply-To: <20150727015850.4928.87717.stgit@softrs>

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

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>
---
 arch/x86/kernel/nmi.c  |   15 +++++++++++----
 include/linux/kernel.h |    1 +
 kernel/panic.c         |   13 ++++++++++---
 3 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
index d05bd2e..5b32d81 100644
--- a/arch/x86/kernel/nmi.c
+++ b/arch/x86/kernel/nmi.c
@@ -230,7 +230,8 @@ void unregister_nmi_handler(unsigned int type, const char *name)
 	}
 #endif
 
-	if (panic_on_unrecovered_nmi)
+	if (panic_on_unrecovered_nmi &&
+	    atomic_cmpxchg(&panicking_cpu, -1, raw_smp_processor_id()) == -1)
 		panic("NMI: Not continuing");
 
 	pr_emerg("Dazed and confused, but trying to continue\n");
@@ -255,8 +256,13 @@ void unregister_nmi_handler(unsigned int type, const char *name)
 		 reason, smp_processor_id());
 	show_regs(regs);
 
-	if (panic_on_io_nmi)
-		panic("NMI IOCK error: Not continuing");
+	if (panic_on_io_nmi) {
+		if (atomic_cmpxchg(&panicking_cpu, -1, raw_smp_processor_id())
+		    == -1)
+			panic("NMI IOCK error: Not continuing");
+		else
+			return; /* We don't want to wait and re-enable NMI */
+	}
 
 	/* Re-enable the IOCK line, wait for a few seconds */
 	reason = (reason & NMI_REASON_CLEAR_MASK) | NMI_REASON_CLEAR_IOCHK;
@@ -296,7 +302,8 @@ void unregister_nmi_handler(unsigned int type, const char *name)
 		 reason, smp_processor_id());
 
 	pr_emerg("Do you have a strange power saving mode enabled?\n");
-	if (unknown_nmi_panic || panic_on_unrecovered_nmi)
+	if ((unknown_nmi_panic || panic_on_unrecovered_nmi) &&
+	    atomic_cmpxchg(&panicking_cpu, -1, raw_smp_processor_id()) == -1)
 		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 5582410..8ca199b 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -442,6 +442,7 @@ extern __scanf(2, 0)
 extern int sysctl_panic_on_stackoverflow;
 
 extern bool crash_kexec_post_notifiers;
+extern atomic_t panicking_cpu;
 
 /*
  * Only to be used by arch init code. If the user over-wrote the default
diff --git a/kernel/panic.c b/kernel/panic.c
index 04e91ff..7e6b568 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -60,6 +60,8 @@ void __weak panic_smp_self_stop(void)
 		cpu_relax();
 }
 
+atomic_t panicking_cpu = ATOMIC_INIT(-1);
+
 /**
  *	panic - halt the system
  *	@fmt: The text string to print
@@ -70,17 +72,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 panicking_cpu) from invoking panic again.
 	 */
 	local_irq_disable();
 
@@ -93,8 +95,13 @@ 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 == -1' means we are the first comer.
+	 * `old_cpu == this_cpu' means we came here due to panic on NMI.
 	 */
-	if (!spin_trylock(&panic_lock))
+	this_cpu = raw_smp_processor_id();
+	old_cpu = atomic_cmpxchg(&panicking_cpu, -1, this_cpu);
+	if (old_cpu != -1 && old_cpu != this_cpu)
 		panic_smp_self_stop();
 
 	console_verbose();



_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

WARNING: multiple messages have this Message-ID (diff)
From: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
To: Jonathan Corbet <corbet@lwn.net>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Vivek Goyal <vgoyal@redhat.com>
Cc: linux-doc@vger.kernel.org, x86@kernel.org,
	kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
	Michal Hocko <mhocko@kernel.org>, Ingo Molnar <mingo@redhat.com>,
	Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Subject: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI
Date: Mon, 27 Jul 2015 10:58:50 +0900	[thread overview]
Message-ID: <20150727015850.4928.50289.stgit@softrs> (raw)
In-Reply-To: <20150727015850.4928.87717.stgit@softrs>

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

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>
---
 arch/x86/kernel/nmi.c  |   15 +++++++++++----
 include/linux/kernel.h |    1 +
 kernel/panic.c         |   13 ++++++++++---
 3 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
index d05bd2e..5b32d81 100644
--- a/arch/x86/kernel/nmi.c
+++ b/arch/x86/kernel/nmi.c
@@ -230,7 +230,8 @@ void unregister_nmi_handler(unsigned int type, const char *name)
 	}
 #endif
 
-	if (panic_on_unrecovered_nmi)
+	if (panic_on_unrecovered_nmi &&
+	    atomic_cmpxchg(&panicking_cpu, -1, raw_smp_processor_id()) == -1)
 		panic("NMI: Not continuing");
 
 	pr_emerg("Dazed and confused, but trying to continue\n");
@@ -255,8 +256,13 @@ void unregister_nmi_handler(unsigned int type, const char *name)
 		 reason, smp_processor_id());
 	show_regs(regs);
 
-	if (panic_on_io_nmi)
-		panic("NMI IOCK error: Not continuing");
+	if (panic_on_io_nmi) {
+		if (atomic_cmpxchg(&panicking_cpu, -1, raw_smp_processor_id())
+		    == -1)
+			panic("NMI IOCK error: Not continuing");
+		else
+			return; /* We don't want to wait and re-enable NMI */
+	}
 
 	/* Re-enable the IOCK line, wait for a few seconds */
 	reason = (reason & NMI_REASON_CLEAR_MASK) | NMI_REASON_CLEAR_IOCHK;
@@ -296,7 +302,8 @@ void unregister_nmi_handler(unsigned int type, const char *name)
 		 reason, smp_processor_id());
 
 	pr_emerg("Do you have a strange power saving mode enabled?\n");
-	if (unknown_nmi_panic || panic_on_unrecovered_nmi)
+	if ((unknown_nmi_panic || panic_on_unrecovered_nmi) &&
+	    atomic_cmpxchg(&panicking_cpu, -1, raw_smp_processor_id()) == -1)
 		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 5582410..8ca199b 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -442,6 +442,7 @@ extern __scanf(2, 0)
 extern int sysctl_panic_on_stackoverflow;
 
 extern bool crash_kexec_post_notifiers;
+extern atomic_t panicking_cpu;
 
 /*
  * Only to be used by arch init code. If the user over-wrote the default
diff --git a/kernel/panic.c b/kernel/panic.c
index 04e91ff..7e6b568 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -60,6 +60,8 @@ void __weak panic_smp_self_stop(void)
 		cpu_relax();
 }
 
+atomic_t panicking_cpu = ATOMIC_INIT(-1);
+
 /**
  *	panic - halt the system
  *	@fmt: The text string to print
@@ -70,17 +72,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 panicking_cpu) from invoking panic again.
 	 */
 	local_irq_disable();
 
@@ -93,8 +95,13 @@ 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 == -1' means we are the first comer.
+	 * `old_cpu == this_cpu' means we came here due to panic on NMI.
 	 */
-	if (!spin_trylock(&panic_lock))
+	this_cpu = raw_smp_processor_id();
+	old_cpu = atomic_cmpxchg(&panicking_cpu, -1, this_cpu);
+	if (old_cpu != -1 && old_cpu != this_cpu)
 		panic_smp_self_stop();
 
 	console_verbose();



  reply	other threads:[~2015-07-27  5:11 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-27  1:58 [V2 PATCH 0/3] x86: Fix panic vs. NMI issues Hidehiro Kawai
2015-07-27  1:58 ` Hidehiro Kawai
2015-07-27  1:58 ` Hidehiro Kawai [this message]
2015-07-27  1:58   ` [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI Hidehiro Kawai
2015-07-27 14:34   ` Michal Hocko
2015-07-27 14:34     ` Michal Hocko
2015-07-28  2:02     ` Hidehiro Kawai
2015-07-28  2:02       ` Hidehiro Kawai
2015-07-28  8:01       ` Michal Hocko
2015-07-28  8:01         ` Michal Hocko
2015-07-29  5:48       ` 河合英宏 / KAWAI,HIDEHIRO
2015-07-29  5:48         ` 河合英宏 / KAWAI,HIDEHIRO
2015-07-29  8:23         ` Michal Hocko
2015-07-29  8:23           ` Michal Hocko
2015-07-29  9:09           ` 河合英宏 / KAWAI,HIDEHIRO
2015-07-29  9:09             ` 河合英宏 / KAWAI,HIDEHIRO
2015-07-29  9:21             ` Michal Hocko
2015-07-29  9:21               ` Michal Hocko
2015-07-30  1:45               ` 河合英宏 / KAWAI,HIDEHIRO
2015-07-30  1:45                 ` 河合英宏 / KAWAI,HIDEHIRO
2015-07-30  7:33                 ` 河合英宏 / KAWAI,HIDEHIRO
2015-07-30  7:33                   ` 河合英宏 / KAWAI,HIDEHIRO
2015-07-30  7:55                   ` Michal Hocko
2015-07-30  7:55                     ` Michal Hocko
2015-07-30  8:06                     ` 河合英宏 / KAWAI,HIDEHIRO
2015-07-30  8:06                       ` 河合英宏 / KAWAI,HIDEHIRO
2015-07-30  7:48                 ` Michal Hocko
2015-07-30  7:48                   ` Michal Hocko
2015-07-30 11:55                   ` 河合英宏 / KAWAI,HIDEHIRO
2015-07-30 11:55                     ` 河合英宏 / KAWAI,HIDEHIRO
2015-07-30 12:27                     ` Michal Hocko
2015-07-30 12:27                       ` Michal Hocko
2015-07-31 11:23                       ` 河合英宏 / KAWAI,HIDEHIRO
2015-07-31 11:23                         ` 河合英宏 / KAWAI,HIDEHIRO
2015-08-04  8:56                         ` Michal Hocko
2015-08-04  8:56                           ` Michal Hocko
2015-08-04 11:53                           ` 河合英宏 / KAWAI,HIDEHIRO
2015-08-04 11:53                             ` 河合英宏 / KAWAI,HIDEHIRO
2015-07-27  1:58 ` [V2 PATCH 2/3] kexec: Fix race between panic() and crash_kexec() called directly Hidehiro Kawai
2015-07-27  1:58   ` Hidehiro Kawai
2015-07-27 14:55   ` Michal Hocko
2015-07-27 14:55     ` Michal Hocko
2015-07-28  2:15     ` Hidehiro Kawai
2015-07-28  2:15       ` Hidehiro Kawai
2015-07-27  1:58 ` [V2 PATCH 3/3] x86/apic: Introduce noextnmi boot option Hidehiro Kawai
2015-07-27  1:58   ` Hidehiro Kawai

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=20150727015850.4928.50289.stgit@softrs \
    --to=hidehiro.kawai.ez@hitachi.com \
    --cc=akpm@linux-foundation.org \
    --cc=corbet@lwn.net \
    --cc=ebiederm@xmission.com \
    --cc=hpa@zytor.com \
    --cc=kexec@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masami.hiramatsu.pt@hitachi.com \
    --cc=mhocko@kernel.org \
    --cc=mingo@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=vgoyal@redhat.com \
    --cc=x86@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.