Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754750AbbG1CCS (ORCPT ); Mon, 27 Jul 2015 22:02:18 -0400 Received: from mail7.hitachi.co.jp ([133.145.228.42]:47444 "EHLO mail7.hitachi.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754512AbbG1CCR (ORCPT ); Mon, 27 Jul 2015 22:02:17 -0400 X-AuditID: 85900ec0-9cdc8b9000001a57-88-55b6e26da60c Message-ID: <55B6E2A3.8070004@hitachi.com> Date: Tue, 28 Jul 2015 11:02:11 +0900 From: Hidehiro Kawai User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120604 Thunderbird/13.0 MIME-Version: 1.0 To: Michal Hocko CC: Jonathan Corbet , Peter Zijlstra , Ingo Molnar , "Eric W. Biederman" , "H. Peter Anvin" , Andrew Morton , Thomas Gleixner , Vivek Goyal , linux-doc@vger.kernel.org, x86@kernel.org, kexec@lists.infradead.org, linux-kernel@vger.kernel.org, Ingo Molnar , Masami Hiramatsu Subject: Re: [V2 PATCH 1/3] x86/panic: Fix re-entrance problem due to panic on NMI References: <20150727015850.4928.87717.stgit@softrs> <20150727015850.4928.50289.stgit@softrs> <20150727143405.GF11317@dhcp22.suse.cz> In-Reply-To: <20150727143405.GF11317@dhcp22.suse.cz> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-Brightmail-Tracker: AAAAAA== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1877 Lines: 74 Hi, Thanks for the review. (2015/07/27 23:34), Michal Hocko wrote: > On Mon 27-07-15 10:58:50, Hidehiro Kawai wrote: > [...] >> 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"); > > Spreading the check to all NMI callers is quite ugly. Wouldn't it be > better to introduce nmi_panic() which wouldn't be __noreturn unlike the > regular panic. Sure. I'll fix it. > The check could be also relaxed a bit and nmi_panic would > return only if the ongoing panic is the current cpu when we really have > to return and allow the preempted panic to finish. It's reasonable. I'll do that in the next version. > Something like [...] > +void nmi_panic(const char *fmt, ...) Since we can't directly pass variable arguments to a subroutine, we have to use a macro or do like this: void nmi_panic(const char *msg) { ... panic("%s", msg); } If there is no objection, I'm going to use a macro. > +{ > + /* > + * We have to back off if the NMI has preempted an ongoing panic and > + * allow it to finish > + */ > + if (atomic_read(&panic_cpu) == raw_smp_processor_id()) > + return; > + > + panic(); > +} > +EXPORT_SYMBOL(nmi_panic); > > struct tnt { > u8 bit; > -- 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/