Re: [PATCH v3 12/12] kdump: Use vmlinux_build_id to simplify

Stephen Boyd
2021-04-08T05:39:08Z

Quoting Petr Mladek (2021-04-07 10:03:28)
> On Tue 2021-03-30 20:05:20, Stephen Boyd wrote:
> > We can use the vmlinux_build_id array here now instead of open coding
> > it. This mostly consolidates code.
> >
> > Cc: Jiri Olsa <jolsa@kernel.org>
> > Cc: Alexei Starovoitov <ast@kernel.org>
> > Cc: Jessica Yu <jeyu@kernel.org>
> > Cc: Evan Green <evgreen@chromium.org>
> > Cc: Hsin-Yi Wang <hsinyi@chromium.org>
> > Cc: Dave Young <dyoung@redhat.com>
> > Cc: Baoquan He <bhe@redhat.com>
> > Cc: Vivek Goyal <vgoyal@redhat.com>
> > Cc: <kexec@lists.infradead.org>
> > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> > ---
> > include/linux/crash_core.h | 6 +-----
> > kernel/crash_core.c | 41 ++------------------------------------
> > 2 files changed, 3 insertions(+), 44 deletions(-)
> >
> > diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
> > index 206bde8308b2..fb8ab99bb2ee 100644
> > --- a/include/linux/crash_core.h
> > +++ b/include/linux/crash_core.h
> > @@ -39,7 +39,7 @@ phys_addr_t paddr_vmcoreinfo_note(void);
> > #define VMCOREINFO_OSRELEASE(value) \
> > vmcoreinfo_append_str("OSRELEASE=%s\n", value)
> > #define VMCOREINFO_BUILD_ID(value) \
> > - vmcoreinfo_append_str("BUILD-ID=%s\n", value)
> > + vmcoreinfo_append_str("BUILD-ID=%20phN\n", value)
>
> Please, add also build check that BUILD_ID_MAX == 20.
>

I added a BUILD_BUG_ON() in kernel/crash_core.c. I tried static_assert()
here but got mixed ISO errors from gcc-10, although it feels like it
should work.

In file included from ./arch/arm64/include/asm/cmpxchg.h:10,
from ./arch/arm64/include/asm/atomic.h:16,
from ./include/linux/atomic.h:7,
from ./include/linux/mm_types_task.h:13,
from ./include/linux/mm_types.h:5,
from ./include/linux/buildid.h:5,
from kernel/crash_core.c:7:
kernel/crash_core.c: In function 'crash_save_vmcoreinfo_init':
./include/linux/build_bug.h:78:41: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~~~~~~~~~~~
./include/linux/build_bug.h:77:34: note: in expansion of macro '__static_assert'
77 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
| ^~~~~~~~~~~~~~~
./include/linux/crash_core.h:42:2: note: in expansion of macro 'static_assert'
42 | static_assert(ARRAY_SIZE(value) == BUILD_ID_SIZE_MAX); \
| ^~~~~~~~~~~~~
kernel/crash_core.c:401:2: note: in expansion of macro 'VMCOREINFO_BUILD_ID'
401 | VMCOREINFO_BUILD_ID(vmlinux_build_id);

>
> The function add_build_id_vmcoreinfo() is used in
> crash_save_vmcoreinfo_init() in this context:
>
>
> VMCOREINFO_OSRELEASE(init_uts_ns.name.release);
> add_build_id_vmcoreinfo();
> VMCOREINFO_PAGESIZE(PAGE_SIZE);
>
> VMCOREINFO_SYMBOL(init_uts_ns);
> VMCOREINFO_OFFSET(uts_namespace, name);
> VMCOREINFO_SYMBOL(node_online_map);
>
> The function is not longer need. VMCOREINFO_BUILD_ID()
> can be used directly:
>
> VMCOREINFO_OSRELEASE(init_uts_ns.name.release);
> VMCOREINFO_BUILD_ID(vmlinux_build_id);
> VMCOREINFO_PAGESIZE(PAGE_SIZE);
>
> VMCOREINFO_SYMBOL(init_uts_ns);
> VMCOREINFO_OFFSET(uts_namespace, name);
> VMCOREINFO_SYMBOL(node_online_map);
>
>

Thanks. Makes sense. I've rolled that in.