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

Hi, lijiang <lijiang(a)redhat.com&gt; writes: > 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 ? Where does the symbol '.rodata' come from ? I couldn't find it in x86's vmlinux with nm as well. On Fedora 34: $ nm /usr/lib/debug/lib/modules/5.16.16-100.fc34.x86_64/vmlinux | grep rodata ffffffff82c1d000 D __end_rodata ffffffff82e00000 D __end_rodata_aligned ffffffff82e00000 D __end_rodata_hpage_align ffffffff81199420 t frob_rodata ffffffff81c3eec7 T mark_rodata_ro ffffffff826dd200 D rodata_enabled ffffffff82e2cec0 d rodata_resource ffffffff81352690 T rodata_test ffffffff81c50238 t rodata_test.cold ffffffff8223d9d0 d rodata_test_data ffffffff836c2c36 t set_debug_rodata ffffffff838ff890 d __setup_set_debug_rodata ffffffff838b4b18 d __setup_str_set_debug_rodata ffffffff82200000 D __start_rodata Regards Alex