From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752282AbdCPNiS (ORCPT ); Thu, 16 Mar 2017 09:38:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39642 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751001AbdCPNiR (ORCPT ); Thu, 16 Mar 2017 09:38:17 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 8E065437F75 Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=xpang@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 8E065437F75 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> <58CA86D2.70800@redhat.com> <20170316131809.GC23625@x1> To: Baoquan He , xlpang@redhat.com Cc: linux-kernel@vger.kernel.org, kexec@lists.infradead.org, akpm@linux-foundation.org, Eric Biederman , Dave Young From: Xunlei Pang Message-ID: <58CA95EA.1040807@redhat.com> Date: Thu, 16 Mar 2017 21:40:58 +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: <20170316131809.GC23625@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.29]); Thu, 16 Mar 2017 13:38:17 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 03/16/2017 at 09:18 PM, Baoquan He wrote: > On 03/16/17 at 08:36pm, Xunlei Pang wrote: >> 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? > Well, then it will make makedumpfile segfault happen too when execute > below command in 1st kernel if it existed: > makedumpfile --mem-usage /proc/kcore Yes, if the initial vmcoreinfo data was modified before "makedumpfile --mem-usage", it might happen, after all the system is going something wrong. And that's why we deploy kdump service at the very beginning when the system has a low possibility of going wrong. But we have to guarantee kdump vmcore can be generated correctly as possible as it can. > > So we still need to face that problem and need fix it. vmcoreinfo_note > is in kernel data area, how does module intrude into this area? And can > we fix the module code? > Bugs always exist in products, we can't know what will happen and fix all the errors, that's why we need kdump. I think the following update should guarantee the correct vmcoreinfo for kdump. --- kernel/kexec_core.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c index bfe62d5..0f7b328 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) +static void do_crash_save_vmcoreinfo_init(void) { VMCOREINFO_OSRELEASE(init_uts_ns.name.release); VMCOREINFO_PAGESIZE(PAGE_SIZE); @@ -1474,6 +1468,20 @@ static int __init crash_save_vmcoreinfo_init(void) #endif arch_crash_save_vmcoreinfo(); +} + +void crash_save_vmcoreinfo(void) +{ + /* Save again to protect vmcoreinfo from being modified */ + vmcoreinfo_size = 0; + do_crash_save_vmcoreinfo_init(); + vmcoreinfo_append_str("CRASHTIME=%ld\n", get_seconds()); + update_vmcoreinfo_note(); +} + +static int __init crash_save_vmcoreinfo_init(void) +{ + do_crash_save_vmcoreinfo_init(); update_vmcoreinfo_note(); return 0; -- 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 bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1coVby-0005TK-PY for kexec@lists.infradead.org; Thu, 16 Mar 2017 13:38:42 +0000 Subject: Re: [PATCH] kexec: Update vmcoreinfo after crash happened References: <1489666587-24103-1-git-send-email-xlpang@redhat.com> <20170316122730.GB23625@x1> <58CA86D2.70800@redhat.com> <20170316131809.GC23625@x1> From: Xunlei Pang Message-ID: <58CA95EA.1040807@redhat.com> Date: Thu, 16 Mar 2017 21:40:58 +0800 MIME-Version: 1.0 In-Reply-To: <20170316131809.GC23625@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 , xlpang@redhat.com Cc: Dave Young , akpm@linux-foundation.org, kexec@lists.infradead.org, linux-kernel@vger.kernel.org, Eric Biederman On 03/16/2017 at 09:18 PM, Baoquan He wrote: > On 03/16/17 at 08:36pm, Xunlei Pang wrote: >> 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? > Well, then it will make makedumpfile segfault happen too when execute > below command in 1st kernel if it existed: > makedumpfile --mem-usage /proc/kcore Yes, if the initial vmcoreinfo data was modified before "makedumpfile --mem-usage", it might happen, after all the system is going something wrong. And that's why we deploy kdump service at the very beginning when the system has a low possibility of going wrong. But we have to guarantee kdump vmcore can be generated correctly as possible as it can. > > So we still need to face that problem and need fix it. vmcoreinfo_note > is in kernel data area, how does module intrude into this area? And can > we fix the module code? > Bugs always exist in products, we can't know what will happen and fix all the errors, that's why we need kdump. I think the following update should guarantee the correct vmcoreinfo for kdump. --- kernel/kexec_core.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c index bfe62d5..0f7b328 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) +static void do_crash_save_vmcoreinfo_init(void) { VMCOREINFO_OSRELEASE(init_uts_ns.name.release); VMCOREINFO_PAGESIZE(PAGE_SIZE); @@ -1474,6 +1468,20 @@ static int __init crash_save_vmcoreinfo_init(void) #endif arch_crash_save_vmcoreinfo(); +} + +void crash_save_vmcoreinfo(void) +{ + /* Save again to protect vmcoreinfo from being modified */ + vmcoreinfo_size = 0; + do_crash_save_vmcoreinfo_init(); + vmcoreinfo_append_str("CRASHTIME=%ld\n", get_seconds()); + update_vmcoreinfo_note(); +} + +static int __init crash_save_vmcoreinfo_init(void) +{ + do_crash_save_vmcoreinfo_init(); update_vmcoreinfo_note(); return 0; -- 1.8.3.1 _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec