From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752198AbdCPMdz (ORCPT ); Thu, 16 Mar 2017 08:33:55 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50074 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751037AbdCPMdy (ORCPT ); Thu, 16 Mar 2017 08:33:54 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 29E6080480 Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=xpang@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 29E6080480 Reply-To: xlpang@redhat.com Subject: Re: [PATCH] kexec: Update vmcoreinfo after crash happened References: <1489666587-24103-1-git-send-email-xlpang@redhat.com> <20170316122730.GB23625@x1> To: Baoquan He , Xunlei Pang Cc: linux-kernel@vger.kernel.org, kexec@lists.infradead.org, akpm@linux-foundation.org, Eric Biederman , Dave Young From: Xunlei Pang Message-ID: <58CA86D2.70800@redhat.com> Date: Thu, 16 Mar 2017 20:36:34 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <20170316122730.GB23625@x1> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Thu, 16 Mar 2017 12:33:54 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 03/16/2017 at 08:27 PM, Baoquan He wrote: > Hi Xunlei, > > Did you really see this ever happened? Because the vmcore size estimate > feature, namely --mem-usage option of makedumpfile, depends on the > vmcoreinfo in 1st kernel, your change will break it. Hi Baoquan, I can reproduce it using a kernel module which modifies the vmcoreinfo, so it's a problem can actually happen. > If not, it could be not good to change that. That's a good point, then I guess we can keep the crash_save_vmcoreinfo_init(), and store again all the vmcoreinfo after crash. What do you think? Regards, Xunlei > > Baoquan > > On 03/16/17 at 08:16pm, Xunlei Pang wrote: >> Currently vmcoreinfo data is updated at boot time subsys_initcall(), >> it has the risk of being modified by some wrong code during system >> is running. >> >> As a result, vmcore dumped will contain the wrong vmcoreinfo. Later on, >> when using "crash" utility to parse this vmcore, we probably will get >> "Segmentation fault". >> >> Based on the fact that the value of each vmcoreinfo stays invariable >> once kernel boots up, we safely move all the vmcoreinfo operations into >> crash_save_vmcoreinfo() which is called after crash happened. In this >> way, vmcoreinfo data correctness is always guaranteed. >> >> Signed-off-by: Xunlei Pang >> --- >> kernel/kexec_core.c | 14 +++----------- >> 1 file changed, 3 insertions(+), 11 deletions(-) >> >> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c >> index bfe62d5..1bfdd96 100644 >> --- a/kernel/kexec_core.c >> +++ b/kernel/kexec_core.c >> @@ -1367,12 +1367,6 @@ static void update_vmcoreinfo_note(void) >> final_note(buf); >> } >> >> -void crash_save_vmcoreinfo(void) >> -{ >> - vmcoreinfo_append_str("CRASHTIME=%ld\n", get_seconds()); >> - update_vmcoreinfo_note(); >> -} >> - >> void vmcoreinfo_append_str(const char *fmt, ...) >> { >> va_list args; >> @@ -1402,7 +1396,7 @@ phys_addr_t __weak paddr_vmcoreinfo_note(void) >> return __pa_symbol((unsigned long)(char *)&vmcoreinfo_note); >> } >> >> -static int __init crash_save_vmcoreinfo_init(void) >> +void crash_save_vmcoreinfo(void) >> { >> VMCOREINFO_OSRELEASE(init_uts_ns.name.release); >> VMCOREINFO_PAGESIZE(PAGE_SIZE); >> @@ -1474,13 +1468,11 @@ static int __init crash_save_vmcoreinfo_init(void) >> #endif >> >> arch_crash_save_vmcoreinfo(); >> - update_vmcoreinfo_note(); >> + vmcoreinfo_append_str("CRASHTIME=%ld\n", get_seconds()); >> >> - return 0; >> + update_vmcoreinfo_note(); >> } >> >> -subsys_initcall(crash_save_vmcoreinfo_init); >> - >> /* >> * Move into place and start executing a preloaded standalone >> * executable. If nothing was preloaded return an error. >> -- >> 1.8.3.1 >> From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mx1.redhat.com ([209.132.183.28]) by merlin.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1coUc2-0003ii-9M for kexec@lists.infradead.org; Thu, 16 Mar 2017 12:34:39 +0000 Subject: Re: [PATCH] kexec: Update vmcoreinfo after crash happened References: <1489666587-24103-1-git-send-email-xlpang@redhat.com> <20170316122730.GB23625@x1> From: Xunlei Pang Message-ID: <58CA86D2.70800@redhat.com> Date: Thu, 16 Mar 2017 20:36:34 +0800 MIME-Version: 1.0 In-Reply-To: <20170316122730.GB23625@x1> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: xlpang@redhat.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "kexec" Errors-To: kexec-bounces+dwmw2=infradead.org@lists.infradead.org To: Baoquan He , Xunlei Pang Cc: Dave Young , akpm@linux-foundation.org, kexec@lists.infradead.org, linux-kernel@vger.kernel.org, Eric Biederman On 03/16/2017 at 08:27 PM, Baoquan He wrote: > Hi Xunlei, > > Did you really see this ever happened? Because the vmcore size estimate > feature, namely --mem-usage option of makedumpfile, depends on the > vmcoreinfo in 1st kernel, your change will break it. Hi Baoquan, I can reproduce it using a kernel module which modifies the vmcoreinfo, so it's a problem can actually happen. > If not, it could be not good to change that. That's a good point, then I guess we can keep the crash_save_vmcoreinfo_init(), and store again all the vmcoreinfo after crash. What do you think? Regards, Xunlei > > Baoquan > > On 03/16/17 at 08:16pm, Xunlei Pang wrote: >> Currently vmcoreinfo data is updated at boot time subsys_initcall(), >> it has the risk of being modified by some wrong code during system >> is running. >> >> As a result, vmcore dumped will contain the wrong vmcoreinfo. Later on, >> when using "crash" utility to parse this vmcore, we probably will get >> "Segmentation fault". >> >> Based on the fact that the value of each vmcoreinfo stays invariable >> once kernel boots up, we safely move all the vmcoreinfo operations into >> crash_save_vmcoreinfo() which is called after crash happened. In this >> way, vmcoreinfo data correctness is always guaranteed. >> >> Signed-off-by: Xunlei Pang >> --- >> kernel/kexec_core.c | 14 +++----------- >> 1 file changed, 3 insertions(+), 11 deletions(-) >> >> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c >> index bfe62d5..1bfdd96 100644 >> --- a/kernel/kexec_core.c >> +++ b/kernel/kexec_core.c >> @@ -1367,12 +1367,6 @@ static void update_vmcoreinfo_note(void) >> final_note(buf); >> } >> >> -void crash_save_vmcoreinfo(void) >> -{ >> - vmcoreinfo_append_str("CRASHTIME=%ld\n", get_seconds()); >> - update_vmcoreinfo_note(); >> -} >> - >> void vmcoreinfo_append_str(const char *fmt, ...) >> { >> va_list args; >> @@ -1402,7 +1396,7 @@ phys_addr_t __weak paddr_vmcoreinfo_note(void) >> return __pa_symbol((unsigned long)(char *)&vmcoreinfo_note); >> } >> >> -static int __init crash_save_vmcoreinfo_init(void) >> +void crash_save_vmcoreinfo(void) >> { >> VMCOREINFO_OSRELEASE(init_uts_ns.name.release); >> VMCOREINFO_PAGESIZE(PAGE_SIZE); >> @@ -1474,13 +1468,11 @@ static int __init crash_save_vmcoreinfo_init(void) >> #endif >> >> arch_crash_save_vmcoreinfo(); >> - update_vmcoreinfo_note(); >> + vmcoreinfo_append_str("CRASHTIME=%ld\n", get_seconds()); >> >> - return 0; >> + update_vmcoreinfo_note(); >> } >> >> -subsys_initcall(crash_save_vmcoreinfo_init); >> - >> /* >> * Move into place and start executing a preloaded standalone >> * executable. If nothing was preloaded return an error. >> -- >> 1.8.3.1 >> _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec