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 <xlp...@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
>>
