From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932537AbbELInN (ORCPT ); Tue, 12 May 2015 04:43:13 -0400 Received: from mail4.hitachi.co.jp ([133.145.228.5]:50002 "EHLO mail4.hitachi.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932227AbbELInJ (ORCPT ); Tue, 12 May 2015 04:43:09 -0400 X-AuditID: 85900ec0-9f5ccb9000001a57-94-5551bd0dc984 Message-ID: <5551BD17.6040606@hitachi.com> Date: Tue, 12 May 2015 17:43:03 +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: Vivek Goyal CC: Ingo Molnar , Masami Hiramatsu , Baoquan He , =?UTF-8?B?IkhhdGF5YW1hLCBEYWlzdWtlL+eVkeWxsQ==?= =?UTF-8?B?IOWkp+i8lCI=?= , ebiederm@xmission.com, linux-kernel@vger.kernel.org, kexec@lists.infradead.org, akpm@linux-foundation.org, mingo@redhat.com, bp@suse.de Subject: Re: Re: Re: [PATCH v2] kernel/panic/kexec: fix "crash_kexec_post_notifiers" option issue in oops path References: <54F9D645.2050008@jp.fujitsu.com> <20150323034752.GD2068@dhcp-16-105.nay.redhat.com> <20150323071943.GA22765@gmail.com> <5510DA42.6040708@hitachi.com> <20150324071129.GA28619@gmail.com> <20150324144608.GB2970@redhat.com> <20150324161814.GB8661@gmail.com> <20150324170417.GA5030@redhat.com> In-Reply-To: <20150324170417.GA5030@redhat.com> Content-Type: text/plain; charset=UTF-8 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 Hi all, What is the current status of this bug fix patch? I think it's OK if resending Hatayama-san's patch with Ingo's. Thanks, (2015/03/25 2:04), Vivek Goyal wrote: > On Tue, Mar 24, 2015 at 05:18:14PM +0100, Ingo Molnar wrote: >> >> * Vivek Goyal wrote: >> >>>> Yet the actual bug is in that commit, 'crash_kexec_post_notifiers' >>>> was clearly not a no-op in the default case, against expectations. >>> >>> Hi Ingo, >>> >>> I did a quick test and in default case crash_kexec() runs before >>> panic notifiers. So it does look like crash_kexec_post_notifiers is >>> a no-op in default case. >>> >>> What am I missing. >> >> Well, look at f06e5153f4ae: >> >> diff --git a/kernel/panic.c b/kernel/panic.c >> index d02fa9fef46a..62e16cef9cc2 100644 >> --- a/kernel/panic.c >> +++ b/kernel/panic.c >> @@ -32,6 +32,7 @@ static unsigned long tainted_mask; >> static int pause_on_oops; >> static int pause_on_oops_flag; >> static DEFINE_SPINLOCK(pause_on_oops_lock); >> +static bool crash_kexec_post_notifiers; >> >> int panic_timeout = CONFIG_PANIC_TIMEOUT; >> EXPORT_SYMBOL_GPL(panic_timeout); >> @@ -112,9 +113,11 @@ void panic(const char *fmt, ...) >> /* >> * If we have crashed and we have a crash kernel loaded let it handle >> * everything else. >> - * Do we want to call this before we try to display a message? >> + * If we want to run this after calling panic_notifiers, pass >> + * the "crash_kexec_post_notifiers" option to the kernel. >> */ >> - crash_kexec(NULL); >> + if (!crash_kexec_post_notifiers) >> + crash_kexec(NULL); >> >> /* >> * Note smp_send_stop is the usual smp shutdown function, which >> @@ -131,6 +134,15 @@ void panic(const char *fmt, ...) >> >> kmsg_dump(KMSG_DUMP_PANIC); >> >> + /* >> + * If you doubt kdump always works fine in any situation, >> + * "crash_kexec_post_notifiers" offers you a chance to run >> + * panic_notifiers and dumping kmsg before kdump. >> + * Note: since some panic_notifiers can make crashed kernel >> + * more unstable, it can increase risks of the kdump failure too. >> + */ >> + crash_kexec(NULL); >> + >> bust_spinlocks(0); >> >> if (!panic_blink) >> >> >> Without knowing what crash_kexec() does, the patch looks buggy: it >> should preserve the old behavior by default, yet it will now execute a >> second crash_kexec() after the kmsg_dump() line. >> >> So the invariant change would have been to do: >> >> - crash_kexec(NULL); >> + if (!crash_kexec_post_notifiers) >> + crash_kexec(NULL); >> >> ... >> >> + if (crash_kexec_post_notifiers) >> + crash_kexec(NULL); >> >> Which in the !crash_kexec_post_notifiers flag case reduces to: >> >> crash_kexec(); >> >> ... >> >> /* NOP */ >> >> I.e. to exactly what the kernel was doing without the patch >> originally. >> >> Which is what my patch does. Nothing more, nothing less. > > Ok, I got it what you mean. > > crash_kexec() does not return if a kdump kernel is loaded. If kdump > kernel is not loaded, then crash_kexec() returns without doing anything. > > I think that explains why not making second call to crash_kexec() under > if, did not create problems. In first case it will never be called and > in second case, it will do nothing and simply return back. > > But anyway, we need your patch as that's right thing to do. > > Thanks > Vivek > -- > 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/ > > -- Hidehiro Kawai Hitachi, Ltd. Research & Development Group