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