On 01/03/18 at 10:30am, Dave Young wrote:
> 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_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
