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

On Mon, Mar 28, 2022 at 5:04 PM <crash-utility-request(a)redhat.com&gt; wrote: > Date: Mon, 28 Mar 2022 11:03:38 +0200 > From: Alexander Egorenkov <egorenar(a)linux.ibm.com&gt; > To: &quot;d.hatayama(a)fujitsu.com&quot; <d.hatayama(a)fujitsu.com&gt;, > &quot;crash-utility(a)redhat.com&quot; <crash-utility(a)redhat.com&gt; > Subject: Re: [Crash-utility] [PATCH] kernel: fix start-up time > degradation caused by strings command > Message-ID: <87ee2m4ff9.fsf(a)oc8242746057.ibm.com&gt; > Content-Type: text/plain > > &quot;d.hatayama(a)fujitsu.com&quot; <d.hatayama(a)fujitsu.com&gt; 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(a)fujitsu.com&gt; > > --- > > 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(a)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 ?