[Crash-utility] [PATCH] kernel: fix start-up time degradation caused by strings command

lijiang lijiang at redhat.com
Mon Mar 28 09:50:02 UTC 2022


On Mon, Mar 28, 2022 at 5:04 PM <crash-utility-request at redhat.com> wrote:

> Date: Mon, 28 Mar 2022 11:03:38 +0200
> From: Alexander Egorenkov <egorenar at linux.ibm.com>
> To: "d.hatayama at fujitsu.com" <d.hatayama at fujitsu.com>,
>         "crash-utility at redhat.com" <crash-utility at redhat.com>
> Subject: Re: [Crash-utility] [PATCH] kernel: fix start-up time
>         degradation caused by strings command
> Message-ID: <87ee2m4ff9.fsf at oc8242746057.ibm.com>
> Content-Type: text/plain
>
> "d.hatayama at fujitsu.com" <d.hatayama at fujitsu.com> writes:
>
> > verify_namelist() uses strings command and scans full part of vmlinux
> > file to find linux_banner string. However, vmlinux file is quite large
> > these days, reaching over 500MB. As a result, this degradates start-up
> > time of crash command 10 or more seconds. (Of course, this depends on
> > machines you use for investigation, but I guess typically we cannot
> > use such powerful machines to investigate crash dump...)
> >
> > To resolve this issue, let's use bfd library and read linux_banner
> > string in vmlinux file directly.
> >
> > A simple benchmark shows the following result:
> >
> > Without the fix:
> >
> >     # cat ./commands.txt
> >     quit
> >     # time ./crash -i ./commands.txt \
> >         /usr/lib/debug/lib/modules/5.16.15-201.fc35.x86_64/vmlinux \
> >         /var/crash/*/vmcore >/dev/null 2>&1
> >
> >     real        0m20.251s
> >     user        0m19.022s
> >     sys 0m1.054s
> >
> > With the fix:
> >
> >     # time ./crash -i ./commands.txt \
> >         /usr/lib/debug/lib/modules/5.16.15-201.fc35.x86_64/vmlinux \
> >         /var/crash/*/vmcore >/dev/null 2>&1
> >
> >     real        0m6.528s
> >     user        0m6.143s
> >     sys 0m0.431s
> >
> > Note that this commit keeps the original logic that uses strings
> > command for backward compatibility for in case.
> >
> > Signed-off-by: HATAYAMA Daisuke <d.hatayama at fujitsu.com>
> > ---
> >  Makefile |  2 +-
> >  kernel.c | 41 +++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 42 insertions(+), 1 deletion(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 007d030..e520b12 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -364,7 +364,7 @@ task.o: ${GENERIC_HFILES} task.c
> >       ${CC} -c ${CRASH_CFLAGS} task.c ${WARNING_OPTIONS} ${WARNING_ERROR}
> >
> >  kernel.o: ${GENERIC_HFILES} kernel.c
> > -     ${CC} -c ${CRASH_CFLAGS} kernel.c ${WARNING_OPTIONS}
> ${WARNING_ERROR}
> > +     ${CC} -c ${CRASH_CFLAGS} kernel.c -I${BFD_DIRECTORY}
> -I${GDB_INCLUDE_DIRECTORY} ${WARNING_OPTIONS} ${WARNING_ERROR}
> >
> >  printk.o: ${GENERIC_HFILES} printk.c
> >       ${CC} -c ${CRASH_CFLAGS} printk.c ${WARNING_OPTIONS}
> ${WARNING_ERROR}
> > diff --git a/kernel.c b/kernel.c
> > index 1c63447..92434a3 100644
> > --- a/kernel.c
> > +++ b/kernel.c
> > @@ -23,6 +23,11 @@
> >  #include <ctype.h>
> >  #include <stdbool.h>
> >  #include "xendump.h"
> > +#if defined(GDB_7_6) || defined(GDB_10_2)
> > +#define __CONFIG_H__ 1
> > +#include "config.h"
> > +#endif
> > +#include "bfd.h"
> >
> >  static void do_module_cmd(ulong, char *, ulong, char *, char *);
> >  static void show_module_taint(void);
> > @@ -97,6 +102,7 @@ static void dump_printk_safe_seq_buf(int);
> >  static char *vmcoreinfo_read_string(const char *);
> >  static void check_vmcoreinfo(void);
> >  static int is_pvops_xen(void);
> > +static int get_linux_banner_from_vmlinux(char *, size_t);
> >
> >
> >  /*
> > @@ -1324,6 +1330,12 @@ verify_namelist()
> >       target_smp = strstr(kt->utsname.version, " SMP ") ? TRUE : FALSE;
> >       namelist_smp = FALSE;
> >
> > +     if (get_linux_banner_from_vmlinux(buffer, sizeof(buffer)) &&
> > +         strstr(buffer, kt->proc_version)) {
> > +             found = TRUE;
> > +             goto found;
> > +     }
> > +
> >          sprintf(command, "/usr/bin/strings %s", namelist);
> >          if ((pipe = popen(command, "r")) == NULL) {
> >                  error(INFO, "%s: %s\n", namelist, strerror(errno));
> > @@ -1384,6 +1396,7 @@ verify_namelist()
> >               }
> >       }
> >
> > +found:
> >       if (found) {
> >                  if (CRASHDEBUG(1)) {
> >                       fprintf(fp, "verify_namelist:\n");
> > @@ -11770,3 +11783,31 @@ check_vmcoreinfo(void)
> >               }
> >       }
> >  }
> > +
> > +static
> > +int get_linux_banner_from_vmlinux(char *buf, size_t size)
> > +{
> > +     struct bfd_section *sect;
> > +     long offset;
> > +
> > +     sect = bfd_get_section_by_name(st->bfd, ".rodata");
> > +     if (!sect)
> > +             return FALSE;
> > +
> > +     /*
> > +      * Although symbol_value() returns dynamic symbol value that
> > +      * is affected by kaslr, which is different from static symbol
> > +      * value in vmlinux file, but relative offset to linux_banner
> > +      * object in .rodata section is idential.
> > +      */
> > +     offset = symbol_value("linux_banner") - symbol_value(".rodata");
> > +
> > +     if (!bfd_get_section_contents(st->bfd,
> > +                                   sect,
> > +                                   buf,
> > +                                   offset,
> > +                                   size))
> > +             return FALSE;
> > +
> > +     return TRUE;
> > +}
> > --
> > 2.31.1
> >
> >
> >
> > --
> > Crash-utility mailing list
> > Crash-utility at redhat.com
> > https://listman.redhat.com/mailman/listinfo/crash-utility
> > Contribution Guidelines: https://github.com/crash-utility/crash/wiki
>
> Hi,
>
> this patch broke crash-utility on s390.
> When i try to open a dump file, then i get this error message:
>
> crash: cannot resolve ".rodata"
>
>
Thank you for pointing out this issue,  Alex.

Any idea why .rodata symbol is missing ?
> Maybe test for its existence first ?
>

Does the following patch work for you?

diff --git a/kernel.c b/kernel.c
index 92434a3..b504564 100644
--- a/kernel.c
+++ b/kernel.c
@@ -11790,6 +11790,9 @@ int get_linux_banner_from_vmlinux(char *buf, size_t
size)
        struct bfd_section *sect;
        long offset;

+       if (!symbol_exists(".rodata"))
+               return FALSE;
+
        sect = bfd_get_section_by_name(st->bfd, ".rodata");
        if (!sect)
                return FALSE;

Thanks.
Lianbo

Regards
> Alex
>
>
>
> ------------------------------
>
> Subject: Digest Footer
>
> --
> Crash-utility mailing list
> Crash-utility at redhat.com
> https://listman.redhat.com/mailman/listinfo/crash-utility
>
>
> ------------------------------
>
> End of Crash-utility Digest, Vol 198, Issue 33
> **********************************************
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/crash-utility/attachments/20220328/36307f55/attachment.htm>


More information about the Crash-utility mailing list