Re: [PATCH] kexec: Update vmcoreinfo after crash happened

[Xunlei Pang <xpang@redhat.com>]

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 <xlpang@redhat.com>
>> ---
>>  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
>>