Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752288AbbLKAYA (ORCPT ); Thu, 10 Dec 2015 19:24:00 -0500 Received: from mail9.hitachi.co.jp ([133.145.228.44]:41941 "EHLO mail9.hitachi.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750835AbbLKAX5 (ORCPT ); Thu, 10 Dec 2015 19:23:57 -0500 From: =?utf-8?B?5rKz5ZCI6Iux5a6PIC8gS0FXQUnvvIxISURFSElSTw==?= To: "'Borislav Petkov'" CC: Jonathan Corbet , Peter Zijlstra , Ingo Molnar , "Eric W. Biederman" , "H. Peter Anvin" , Andrew Morton , Thomas Gleixner , Vivek Goyal , Baoquan He , "linux-doc@vger.kernel.org" , "x86@kernel.org" , "kexec@lists.infradead.org" , "linux-kernel@vger.kernel.org" , Steven Rostedt , Michal Hocko , Ingo Molnar , =?utf-8?B?5bmz5p2+6ZuF5bezIC8gSElSQU1BVFXvvIxNQVNBTUk=?= Subject: RE: Re: [V6 PATCH 1/6] panic/x86: Fix re-entrance problem due to panic on NMI Thread-Topic: [!]Re: [V6 PATCH 1/6] panic/x86: Fix re-entrance problem due to panic on NMI Thread-Index: AQHRM2FG2QD9mOXmnk6QBu0fxIRjnZ7E7Rvw Date: Fri, 11 Dec 2015 00:23:51 +0000 Message-ID: <04EAB7311EE43145B2D3536183D1A84454A50151@GSjpTKYDCembx31.service.hitachi.net> References: <20151210014624.25437.50028.stgit@softrs> <20151210014626.25437.13302.stgit@softrs> <20151210154136.GA4762@pd.tnic> In-Reply-To: <20151210154136.GA4762@pd.tnic> Accept-Language: ja-JP, en-US Content-Language: ja-JP X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.198.219.30] Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id tBB0O5tn005432 Content-Length: 7642 Lines: 216 > From: Borislav Petkov [mailto:bp@alien8.de] [...] > Looks better. > > Did some comments cleanup and nmi_panic() macro reformatting so that it > is more readable and ended up applying this: Thanks a lot! Hidehiro Kawai Hitachi, Ltd. Research & Development Group > --- > From: Hidehiro Kawai > 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 > Acked-by: Michal Hocko > Cc: Aaron Tomlin > Cc: Andrew Morton > Cc: Andy Lutomirski > Cc: Baoquan He > Cc: Chris Metcalf > Cc: David Hildenbrand > Cc: Don Zickus > Cc: "Eric W. Biederman" > Cc: Frederic Weisbecker > Cc: Gobinda Charan Maji > Cc: HATAYAMA Daisuke > Cc: "H. Peter Anvin" > Cc: Ingo Molnar > Cc: Javi Merino > Cc: Jonathan Corbet > Cc: kexec@lists.infradead.org > Cc: linux-doc@vger.kernel.org > Cc: lkml > Cc: Masami Hiramatsu > Cc: Michal Nazarewicz > Cc: Nicolas Iooss > Cc: Peter Zijlstra > Cc: Prarit Bhargava > Cc: Rasmus Villemoes > Cc: Rusty Russell > Cc: Seth Jennings > Cc: Steven Rostedt > Cc: Thomas Gleixner > Cc: Ulrich Obergfell > Cc: Vitaly Kuznetsov > Cc: Vivek Goyal > Link: http://lkml.kernel.org/r/20151210014626.25437.13302.stgit@softrs > [ Cleanup comments, fixup formatting. ] > Signed-off-by: Borislav Petkov > --- > 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. ????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?