From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail-wi0-x232.google.com ([2a00:1450:400c:c05::232]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1YZweh-0000I4-6G for kexec@lists.infradead.org; Mon, 23 Mar 2015 07:20:12 +0000 Received: by wibgn9 with SMTP id gn9so53062901wib.1 for ; Mon, 23 Mar 2015 00:19:48 -0700 (PDT) Date: Mon, 23 Mar 2015 08:19:43 +0100 From: Ingo Molnar Subject: Re: [PATCH v2] kernel/panic/kexec: fix "crash_kexec_post_notifiers" option issue in oops path Message-ID: <20150323071943.GA22765@gmail.com> References: <54F9D645.2050008@jp.fujitsu.com> <20150323034752.GD2068@dhcp-16-105.nay.redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20150323034752.GD2068@dhcp-16-105.nay.redhat.com> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: "kexec" Errors-To: kexec-bounces+dwmw2=infradead.org@lists.infradead.org To: Baoquan He Cc: kexec@lists.infradead.org, linux-kernel@vger.kernel.org, hidehiro.kawai.ez@hitachi.com, =?utf-8?B?IkhhdGF5YW1hLCBEYWlzdWtlL+eVkeWxsSDlpKfovJQi?= , mingo@redhat.com, ebiederm@xmission.com, masami.hiramatsu.pt@hitachi.com, akpm@linux-foundation.org, bp@suse.de, Vivek Goyal CiogQmFvcXVhbiBIZSA8YmhlQHJlZGhhdC5jb20+IHdyb3RlOgoKPiBDQyBtb3JlIHBlb3BsZSAu Li4KPiAKPiBPbiAwMy8wNy8xNSBhdCAwMTozMWFtLCAiSGF0YXlhbWEsIERhaXN1a2Uv55WR5bGx IOWkp+i8lCIgd3JvdGU6Cj4gPiBUaGUgY29tbWl0IGYwNmU1MTUzZjRhZTJlMmYzYjAzMDBmMGUy NjBlNDBjYjdmZWZkNDUgaW50cm9kdWNlZAo+ID4gImNyYXNoX2tleGVjX3Bvc3Rfbm90aWZpZXJz IiBrZXJuZWwgYm9vdCBvcHRpb24sIHdoaWNoIHRvZ2dsZXMKPiA+IHdoZWF0aGVyIHBhbmljKCkg Y2FsbHMgY3Jhc2hfa2V4ZWMoKSBiZWZvcmUgcGFuaWNfbm90aWZpZXJzIGFuZCBkdW1wCj4gPiBr bXNnIG9yIGFmdGVyLgo+ID4gCj4gPiBUaGUgcHJvYmxlbSBpcyB0aGF0IHRoZSBjb21taXQgb3Zl cmxvb2tzIHBhbmljX29uX29vcHMga2VybmVsIGJvb3QKPiA+IG9wdGlvbi4gSWYgaXQgaXMgZW5h YmxlZCwgY3Jhc2hfa2V4ZWMoKSBpcyBjYWxsZWQgZGlyZWN0bHkgd2l0aG91dAo+ID4gZ29pbmcg dGhyb3VnaCBwYW5pYygpIGluIG9vcHMgcGF0aC4KPiA+IAo+ID4gVG8gZml4IHRoaXMgaXNzdWUs IHRoaXMgcGF0Y2ggYWRkcyBhIGNoZWNrIHRvCj4gPiAiY3Jhc2hfa2V4ZWNfcG9zdF9ub3RpZmll cnMiIGluIHRoZSBjb25kaXRpb24gb2Yga2V4ZWNfc2hvdWxkX2NyYXNoKCkuCj4gPiAKPiA+IEFs c28sIHB1dCBhIGNvbW1lbnQgaW4ga2V4ZWNfc2hvdWxkX2NyYXNoKCkgdG8gZXhwbGFpbiBub3Qg b2J2aW91cwo+ID4gdGhpbmdzIG9uIHRoaXMgcGF0Y2guCj4gPiAKPiA+IFNpZ25lZC1vZmYtYnk6 IEhBVEFZQU1BIERhaXN1a2UgPGQuaGF0YXlhbWFAanAuZnVqaXRzdS5jb20+Cj4gPiBBY2tlZC1i eTogQmFvcXVhbiBIZSA8YmhlQHJlZGhhdC5jb20+Cj4gPiBUZXN0ZWQtYnk6IEhpZGVoaXJvIEth d2FpIDxoaWRlaGlyby5rYXdhaS5lekBoaXRhY2hpLmNvbT4KPiA+IFJldmlld2VkLWJ5OiBNYXNh bWkgSGlyYW1hdHN1IDxtYXNhbWkuaGlyYW1hdHN1LnB0QGhpdGFjaGkuY29tPgo+ID4gLS0tCj4g PiAgaW5jbHVkZS9saW51eC9rZXJuZWwuaCB8ICAzICsrKwo+ID4gIGtlcm5lbC9rZXhlYy5jICAg ICAgICAgfCAxMSArKysrKysrKysrKwo+ID4gIGtlcm5lbC9wYW5pYy5jICAgICAgICAgfCAgMiAr LQo+ID4gIDMgZmlsZXMgY2hhbmdlZCwgMTUgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbigtKQoK VGhpcyBpcyBoYWNrIHVwb24gaGFjaywgYnV0IHdoeSB3YXMgdGhpcyBjcmFwIG1lcmdlZCBpbiB0 aGUgZmlyc3QgCnBsYWNlPwoKSSBzZWUgdHdvIHByb2JsZW1zIGp1c3QgYnkgY3Vyc29yeSByZXZp ZXc6CgoxKQoKRmlyc3RseSwgdGhlIHJlYWwgYnVnIGluOgoKICBmMDZlNTE1M2Y0YWUgKCJrZXJu ZWwvcGFuaWMuYzogYWRkICJjcmFzaF9rZXhlY19wb3N0X25vdGlmaWVycyIgb3B0aW9uIGZvciBr ZHVtcCBhZnRlciBwYW5pY19ub3RpZmVycyIpCgpXYXMgdGhhdCBjcmFzaF9rZXhlYygpIHdhcyBj YWxsZWQgdW5jb25kaXRpb25hbGx5IGFmdGVyIG5vdGlmaWVycyB3ZXJlIApjYWxsZWQsIHdoaWNo IHNob3VsZCBiZSBmaXhlZCB2aWEgdGhlIHNpbXBsZSBwYXRjaCBiZWxvdyAodW50ZXN0ZWQpLiAK TG9va3MgbXVjaCBzaW1wbGVyIHRoYW4geW91ciBmaXguCgoyKQoKU2Vjb25kbHksIGFuZCBtb3Jl IGltcG9ydGFudGx5LCB0aGUgd2hvbGUgcHJlbWlzZSBvZiBjb21taXQgCmYwNmU1MTUzZjRhZSBp cyBicm9rZW4gSU1ITzoKCiAiVGhpcyBjYW4gaGVscCByYXJlIHNpdHVhdGlvbnMgd2hlcmUga2R1 bXAgZmFpbHMgYmVjYXVzZSBvZiB1bnN0YWJsZQogIGNyYXNoZWQga2VybmVsIG9yIGhhcmR3YXJl IGZhaWx1cmUgKG1lbW9yeSBjb3JydXB0aW9uIG9uIGNyaXRpY2FsCiAgZGF0YS9jb2RlKSIKCnd0 Zj8KCklmIHRoZSBrZXJuZWwgY3Jhc2hlZCBkdWUgdG8gYSBrZXJuZWwgY3Jhc2gsIHRoZW4gdGhl IGtlcm5lbCBib290aW5nIAp1cCBpbiB3aGF0ZXZlciBoYXJkd2FyZSBzdGF0ZSBzaG91bGQgYmUg YWJsZSB0byBkbyBhIGNsZWFuIGJvb3R1cC4gVGhlIApmaXggZm9yIHRob3NlICdyYXJlIHNpdHVh dGlvbnMnIHNob3VsZCBiZSB0byBmaXggdGhlIHJlYWwgYnVnIChmb3IgCmV4YW1wbGUgYnkgbWFr aW5nIGhhcmR3YXJlIGRyaXZlciBpbml0IChvciBkZWluaXQpIHNlcXVlbmNlcyBtb3JlIApyb2J1 c3QpLCBub3QgdG8gcGFwZXIgaXQgb3ZlciBieSBvcmRlcmluZyBhcm91bmQgY3Jhc2gtdGltZSBz ZXF1ZW5jZXMgCi4uLgoKSWYgaXQgY3Jhc2hlZCBkdWUgdG8gc29tZSBoYXJkd2FyZSBmYWlsdXJl LCB0aGVyZSdzIGxpdGVyYWxseSBhbiAKaW5maW5pdGUgYW1vdW50IG9mIGZhaWx1cmUgbW9kZXMg dGhhdCBtYXkgb3IgbWF5IG5vdCBiZSBpbXBhY3RlZCBieSAKa2V4ZWMgY3Jhc2gtdGltZSBoYW5k bGluZyBvcmRlcmluZy4gV2UgZG9uJ3Qgd2FudCB0byBwdXQgYSB6aWxsaW9uIApzdWNoIGZsYWdz IGludG8gdGhlIGtlcm5lbCBwcm9wZXIganVzdCB0byBhbGxvdyB0aGUgcGVydHVyYmF0aW9uIG9m IAp0aGUga2VybmVsLgoKVGhhbmtzLAoKCUluZ28KCmRpZmYgLS1naXQgYS9rZXJuZWwvcGFuaWMu YyBiL2tlcm5lbC9wYW5pYy5jCmluZGV4IDgxMzZhZDc2ZTVmZC4uNzc0NjE0ZjcyY2JkIDEwMDY0 NAotLS0gYS9rZXJuZWwvcGFuaWMuYworKysgYi9rZXJuZWwvcGFuaWMuYwpAQCAtMTQyLDcgKzE0 Miw4IEBAIHZvaWQgcGFuaWMoY29uc3QgY2hhciAqZm10LCAuLi4pCiAJICogTm90ZTogc2luY2Ug c29tZSBwYW5pY19ub3RpZmllcnMgY2FuIG1ha2UgY3Jhc2hlZCBrZXJuZWwKIAkgKiBtb3JlIHVu c3RhYmxlLCBpdCBjYW4gaW5jcmVhc2Ugcmlza3Mgb2YgdGhlIGtkdW1wIGZhaWx1cmUgdG9vLgog CSAqLwotCWNyYXNoX2tleGVjKE5VTEwpOworCWlmIChjcmFzaF9rZXhlY19wb3N0X25vdGlmaWVy cykKKwkJY3Jhc2hfa2V4ZWMoTlVMTCk7CiAKIAlidXN0X3NwaW5sb2NrcygwKTsKIAoKX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18Ka2V4ZWMgbWFpbGluZyBs aXN0CmtleGVjQGxpc3RzLmluZnJhZGVhZC5vcmcKaHR0cDovL2xpc3RzLmluZnJhZGVhZC5vcmcv bWFpbG1hbi9saXN0aW5mby9rZXhlYwo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752106AbbCWHTu (ORCPT ); Mon, 23 Mar 2015 03:19:50 -0400 Received: from mail-we0-f171.google.com ([74.125.82.171]:35090 "EHLO mail-we0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751935AbbCWHTt (ORCPT ); Mon, 23 Mar 2015 03:19:49 -0400 Date: Mon, 23 Mar 2015 08:19:43 +0100 From: Ingo Molnar To: Baoquan He Cc: =?utf-8?B?IkhhdGF5YW1hLCBEYWlzdWtlL+eVkeWxsSDlpKfovJQi?= , ebiederm@xmission.com, Vivek Goyal , masami.hiramatsu.pt@hitachi.com, hidehiro.kawai.ez@hitachi.com, linux-kernel@vger.kernel.org, kexec@lists.infradead.org, akpm@linux-foundation.org, mingo@redhat.com, bp@suse.de Subject: Re: [PATCH v2] kernel/panic/kexec: fix "crash_kexec_post_notifiers" option issue in oops path Message-ID: <20150323071943.GA22765@gmail.com> References: <54F9D645.2050008@jp.fujitsu.com> <20150323034752.GD2068@dhcp-16-105.nay.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20150323034752.GD2068@dhcp-16-105.nay.redhat.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Baoquan He wrote: > CC more people ... > > On 03/07/15 at 01:31am, "Hatayama, Daisuke/畑山 大輔" wrote: > > The commit f06e5153f4ae2e2f3b0300f0e260e40cb7fefd45 introduced > > "crash_kexec_post_notifiers" kernel boot option, which toggles > > wheather panic() calls crash_kexec() before panic_notifiers and dump > > kmsg or after. > > > > The problem is that the commit overlooks panic_on_oops kernel boot > > option. If it is enabled, crash_kexec() is called directly without > > going through panic() in oops path. > > > > To fix this issue, this patch adds a check to > > "crash_kexec_post_notifiers" in the condition of kexec_should_crash(). > > > > Also, put a comment in kexec_should_crash() to explain not obvious > > things on this patch. > > > > Signed-off-by: HATAYAMA Daisuke > > Acked-by: Baoquan He > > Tested-by: Hidehiro Kawai > > Reviewed-by: Masami Hiramatsu > > --- > > include/linux/kernel.h | 3 +++ > > kernel/kexec.c | 11 +++++++++++ > > kernel/panic.c | 2 +- > > 3 files changed, 15 insertions(+), 1 deletion(-) This is hack upon hack, but why was this crap merged in the first place? I see two problems just by cursory review: 1) Firstly, the real bug in: f06e5153f4ae ("kernel/panic.c: add "crash_kexec_post_notifiers" option for kdump after panic_notifers") Was that crash_kexec() was called unconditionally after notifiers were called, which should be fixed via the simple patch below (untested). Looks much simpler than your fix. 2) Secondly, and more importantly, the whole premise of commit f06e5153f4ae is broken IMHO: "This can help rare situations where kdump fails because of unstable crashed kernel or hardware failure (memory corruption on critical data/code)" wtf? If the kernel crashed due to a kernel crash, then the kernel booting up in whatever hardware state should be able to do a clean bootup. The fix for those 'rare situations' should be to fix the real bug (for example by making hardware driver init (or deinit) sequences more robust), not to paper it over by ordering around crash-time sequences ... If it crashed due to some hardware failure, there's literally an infinite amount of failure modes that may or may not be impacted by kexec crash-time handling ordering. We don't want to put a zillion such flags into the kernel proper just to allow the perturbation of the kernel. Thanks, Ingo diff --git a/kernel/panic.c b/kernel/panic.c index 8136ad76e5fd..774614f72cbd 100644 --- a/kernel/panic.c +++ b/kernel/panic.c @@ -142,7 +142,8 @@ void panic(const char *fmt, ...) * Note: since some panic_notifiers can make crashed kernel * more unstable, it can increase risks of the kdump failure too. */ - crash_kexec(NULL); + if (crash_kexec_post_notifiers) + crash_kexec(NULL); bust_spinlocks(0);