On 05/07/21 at 11:54am, Baoquan He wrote:
> On 04/29/21 at 01:14am, Stephen Boyd wrote:
> > Quoting Baoquan He (2021-04-28 03:13:55)
> > > On 04/20/21 at 02:50pm, 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@xxxxxxxxxx>
> > > > Cc: Alexei Starovoitov <ast@xxxxxxxxxx>
> > > > Cc: Jessica Yu <jeyu@xxxxxxxxxx>
> > > > Cc: Evan Green <evgreen@xxxxxxxxxxxx>
> > > > Cc: Hsin-Yi Wang <hsinyi@xxxxxxxxxxxx>
> > > > Cc: Dave Young <dyoung@xxxxxxxxxx>
> > > > Cc: Baoquan He <bhe@xxxxxxxxxx>
> > > > Cc: Vivek Goyal <vgoyal@xxxxxxxxxx>
> > > > Cc: <kexec@xxxxxxxxxxxxxxxxxxx>
> > > > Signed-off-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>
> > > > ---
> > > > include/linux/crash_core.h | 12 ++++-----
> > > > kernel/crash_core.c | 50 ++------------------------------------
> > > > 2 files changed, 8 insertions(+), 54 deletions(-)
> > > >
> > > > diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
> > > > index 206bde8308b2..de62a722431e 100644
> > > > --- a/include/linux/crash_core.h
> > > > +++ b/include/linux/crash_core.h
> > > > @@ -38,8 +38,12 @@ 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)
> > > > +#define VMCOREINFO_BUILD_ID() \
> > > > + ({ \
> > > > + static_assert(sizeof(vmlinux_build_id) == 20); \
> > > > + vmcoreinfo_append_str("BUILD-ID=%20phN\n", vmlinux_build_id); \
> > >
> > > Since there has been static_assert at above, can we remove the magic
> > > number '20'?
> > >
> > > And I checked format_decode(), didn't find which type corresponds to
> > > 'N', could you tell?
> >
> > It is documented in Documentation/core-api/printk-formats.rst (see "Raw
> > buffer as a hex string") where the '20' replaces the '*' to specify a
> > fixed size buffer to format. We could pass the 20 as an argument instead
> > of encoding it in the format, but then it would still be there twice. We
> > could use a #define but then it would have to be passed as an argument.
>
> Thanks for telling, I can see 'phN' is used in many places, but haven't
> figured out how it's parsed, e.g in format_decode() it's not related.
> Anyway, I may need dig further or search to get it.
>
> >
> > I don't know of a good way to enforce in printk that the buffer is the
> > size that matches the format size. Maybe it shouldn't be an option to
> > have a size in the format string if it's a problem to put the size of
> > the buffer in there.
>
> Oh, I meant that we may not need to enforce that because the size of
> vmlinux_build_id can be guaranteed, always 20, we do not need to specify
> the '20'? Afaik, we usually specify the format size when the string size
> is variable, and we want to add '0' to make the width of printing is
~'space' I meant
> fixed. Anyway, this dones't matter much.
>
> Acked-by: Baoquan He <bhe@xxxxxxxxxx>
>
> Thanks
> Baoquan
>
> >
> > >
> > > Other than these, this patch looks good to me, thanks for the effort.
> > >
> >
> > Thanks.
> >
> > _______________________________________________
> > kexec mailing list
> > kexec@xxxxxxxxxxxxxxxxxxx
> > http://lists.infradead.org/mailman/listinfo/kexec
> >
>
>
> _______________________________________________
> kexec mailing list
> kexec@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/kexec
>
_______________________________________________
kexec mailing list
kexec@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/kexec
