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

[Dave Young]

On 01/02/18 at 05:08pm, Baoquan He wrote:
> On 01/02/18 at 04:57pm, Dave Young wrote:
> > 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. 

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_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