makedumpfile saving vmcore fails with dynamically allocated mem_section (was: Re: [PATCH] handle renamed init_level4_pgt -> init_top_pgt)

[Atsushi Kumagai]

>> >> > > The root cause is this commit makes mem_section as a pointer instead of
>> >> > > the static array.
>> >> > >
>> >> > > VMCOREINFO_SYMBOL() expand it as &mem_section, this is not correct in
>> >> > > the test case any more.
>> >> > >
>> >> > > This hack code works for me:
>> >> > >
>> >> > > diff --git a/kernel/crash_core.c b/kernel/crash_core.c
>> >> > > index b3663896278e..f5fe6068ae39 100644
>> >> > > --- a/kernel/crash_core.c
>> >> > > +++ b/kernel/crash_core.c
>> >> > > @@ -376,6 +376,8 @@ phys_addr_t __weak paddr_vmcoreinfo_note(void)
>> >> > >  {
>> >> > >  	return __pa(vmcoreinfo_note);
>> >> > >  }
>> >> > > +#define VMCOREINFO_SYMBOL_HACK(name) \
>> >> > > +	vmcoreinfo_append_str("SYMBOL(%s)=%lx\n", #name, (unsigned long)name)
>> >> >
>> >> > Oh, you made a new one. We may use vmcoreinfo_append_str directly since
>> >> > there's an existing one in crash_save_vmcoreinfo():
>> >>
>> >> Yes, it should be something like this instead, this should ensure
>> >> makedumpfile maybe crash tool works without any modifications,
>> >> waiting for feedback from Atsushi, also ccing Dave for crash utility
>> >> potential issue.
>>
>> makedumpfile needs some fix anyway since some logic depend on the array length
>> of mem_section. I'm trying to fix it referring to the crash's fix.
>
>Hmm, a makedumpfile only fix is enough for this issue?

Sorry, it was thoughtless of me. The crash's fix uses dwarf info,
the kernel side fix you thought is necessary for makedumpfile to work without
vmlinux.


Thanks,
Atsushi Kumagai

>>
>> > I'll give priority to release v1.6.3 since the commit will not be included
>> > in the supported kernel(4.14).
>>
>> The kernel change was merged also into kernel 4.14.9, I'm working on this issue
>> for the next release(v1.6.3).
>>
>> Thanks,
>> Atsushi Kumagai
>>
>> >> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
>> >> index b3663896278e..122494364179 100644
>> >> --- a/kernel/crash_core.c
>> >> +++ b/kernel/crash_core.c
>> >> @@ -410,10 +410,16 @@ static int __init crash_save_vmcoreinfo_init(void)
>> >>  	VMCOREINFO_SYMBOL(contig_page_data);
>> >>  #endif
>> >>  #ifdef CONFIG_SPARSEMEM
>> >> +#ifdef CONFIG_SPARSEMEM_EXTREME
>> >> +	vmcoreinfo_append_str("SYMBOL(mem_section)=%lx\n",
>> >> +			      (unsigned long)mem_section);
>> >> +#else
>> >>  	VMCOREINFO_SYMBOL(mem_section);
>> >> +#endif
>> >
>> >	vmcoreinfo_append_str("SYMBOL(mem_section)=%lx\n",
>> >				(unsigned long)mem_section);
>> >
>> >This is enough to cover both cases. mem_section is array name in non
>> >SPARSEMEM_EXTREME case, so value of &mem_section == value of
>> >mem_section.
>> >
>> >>  	VMCOREINFO_LENGTH(mem_section, NR_SECTION_ROOTS);
>> >>  	VMCOREINFO_STRUCT_SIZE(mem_section);
>> >>  	VMCOREINFO_OFFSET(mem_section, section_mem_map);
>> >> +
>> >>  #endif
>> >>  	VMCOREINFO_STRUCT_SIZE(page);
>> >>  	VMCOREINFO_STRUCT_SIZE(pglist_data);
>> >>
>> >> >
>> >> > vmcoreinfo_append_str("CRASHTIME=%ld\n", get_seconds());
>> >> >
>> >> > The thing is that VMCOREINFO_SYMBOL will reference symbol. While
>> >> > checking all exported symbol, all of them are array names. So here
>> >> > the value of &array_name is equal to the address of the first element of
>> >> > array. Now the first exception appeared, mem_section, which could be not
>> >> > an array name, but a pointer pointing at the allocated memory.
>> >> >
>> >> > For this issue, we either change as Dave mentioned two options, or can
>> >> > we adjust VMCOREINFO_SYMBOL(name)?
>> >> >
>> >> > >
>> >> > >  static int __init crash_save_vmcoreinfo_init(void)
>> >> > >  {
>> >> > > @@ -410,10 +412,11 @@ static int __init crash_save_vmcoreinfo_init(void)
>> >> > >  	VMCOREINFO_SYMBOL(contig_page_data);
>> >> > >  #endif
>> >> > >  #ifdef CONFIG_SPARSEMEM
>> >> > > -	VMCOREINFO_SYMBOL(mem_section);
>> >> > > +	VMCOREINFO_SYMBOL_HACK(mem_section);
>> >> > >  	VMCOREINFO_LENGTH(mem_section, NR_SECTION_ROOTS);
>> >> > >  	VMCOREINFO_STRUCT_SIZE(mem_section);
>> >> > >  	VMCOREINFO_OFFSET(mem_section, section_mem_map);
>> >> > > +
>> >> > >  #endif
>> >> > >  	VMCOREINFO_STRUCT_SIZE(page);
>> >> > >  	VMCOREINFO_STRUCT_SIZE(pglist_data);
>> >> > >
>> >> > >
>> >> > > But probably we need this instead, but I can not test it because I do
>> >> > > not know how to fix makedumpfile to use a _NUMBER instead of a SYMBOL.
>> >> > > Thus bring up the issue, seeking for thoughts and discussion.
>> >> > >
>> >> > > diff --git a/kernel/crash_core.c b/kernel/crash_core.c
>> >> > > index b3663896278e..dfa2238e2c28 100644
>> >> > > --- a/kernel/crash_core.c
>> >> > > +++ b/kernel/crash_core.c
>> >> > > @@ -410,10 +410,17 @@ static int __init crash_save_vmcoreinfo_init(void)
>> >> > > 	VMCOREINFO_SYMBOL(contig_page_data);
>> >> > >  #endif
>> >> > >  #ifdef CONFIG_SPARSEMEM
>> >> > > +#ifdef CONFIG_SPARSEMEM_EXTREME
>> >> > > +	VMCOREINFO_NUMBER(mem_section);
>> >> > > +#else
>> >> > > 	VMCOREINFO_SYMBOL(mem_section);
>> >> > > +#endif
>> >> > > 	VMCOREINFO_LENGTH(mem_section, NR_SECTION_ROOTS);
>> >> > > 	VMCOREINFO_STRUCT_SIZE(mem_section);
>> >> > > 	VMCOREINFO_OFFSET(mem_section, section_mem_map);
>> >> > >
>> >> > > [snip]
>> >> > >
>> >> > > Thanks
>> >> > > Dave
>>
>>
>
>Thanks
>Dave
>
>_______________________________________________
>kexec mailing list
>kexec at lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/kexec