On Wed, Mar 23, 2022 at 4:11 PM <[email protected]> wrote:
> Date: Wed, 23 Mar 2022 08:09:49 +0000 > From: "[email protected]" <[email protected]> > To: "[email protected]" <[email protected]> > Subject: [Crash-utility] [PATCH] kernel: fix start-up time degradation > caused by strings command > Message-ID: > < > tyapr01mb6507e11837e16b365d7743b695...@tyapr01mb6507.jpnprd01.prod.outlook.com > > > > Content-Type: text/plain; charset="iso-2022-jp" > > 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 > This looks pretty good, thank you for the improvement, Hatayama. Applied: https://github.com/crash-utility/crash/commit/cd8954023bd474521a9d45e2b09a7bce4174f52f BTW: Currently, crash performance issues may become more and more prominent on multi-core and large memory systems, hope to have more optimization work in the future. Thanks. Lianbo > Note that this commit keeps the original logic that uses strings > command for backward compatibility for in case. > > Signed-off-by: HATAYAMA Daisuke <[email protected]> > --- > 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 [email protected] https://listman.redhat.com/mailman/listinfo/crash-utility Contribution Guidelines: https://github.com/crash-utility/crash/wiki
