<div dir="ltr"><div dir="ltr">On Mon, Mar 28, 2022 at 5:04 PM <<a href="mailto:crash-utility-request@redhat.com">crash-utility-request@redhat.com</a>> wrote: </div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> Date: Mon, 28 Mar 2022 11:03:38 +0200 From: Alexander Egorenkov <<a href="mailto:egorenar@linux.ibm.com" target="_blank">egorenar@linux.ibm.com</a>> To: "<a href="mailto:d.hatayama@fujitsu.com" target="_blank">d.hatayama@fujitsu.com</a>" <<a href="mailto:d.hatayama@fujitsu.com" target="_blank">d.hatayama@fujitsu.com</a>>, "<a href="mailto:crash-utility@redhat.com" target="_blank">crash-utility@redhat.com</a>" <<a href="mailto:crash-utility@redhat.com" target="_blank">crash-utility@redhat.com</a>> Subject: Re: [Crash-utility] [PATCH] kernel: fix start-up time degradation caused by strings command Message-ID: <<a href="mailto:87ee2m4ff9.fsf@oc8242746057.ibm.com" target="_blank">87ee2m4ff9.fsf@oc8242746057.ibm.com</a>> Content-Type: text/plain "<a href="mailto:d.hatayama@fujitsu.com" target="_blank">d.hatayama@fujitsu.com</a>" <<a href="mailto:d.hatayama@fujitsu.com" target="_blank">d.hatayama@fujitsu.com</a>> 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 <<a href="mailto:d.hatayama@fujitsu.com" target="_blank">d.hatayama@fujitsu.com</a>> > --- > 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 > <a href="mailto:Crash-utility@redhat.com" target="_blank">Crash-utility@redhat.com</a> > <a href="https://listman.redhat.com/mailman/listinfo/crash-utility" rel="noreferrer" target="_blank">https://listman.redhat.com/mailman/listinfo/crash-utility</a> > Contribution Guidelines: <a href="https://github.com/crash-utility/crash/wiki" rel="noreferrer" target="_blank">https://github.com/crash-utility/crash/wiki</a> 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" </blockquote><div> </div><div>Thank you for pointing out this issue, Alex.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> Any idea why .rodata symbol is missing ? Maybe test for its existence first ? </blockquote><div> </div><div>Does the following patch work for you?</div><div> </div>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; <div> </div><div>Thanks.</div><div>Lianbo</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> Regards Alex ------------------------------ Subject: Digest Footer -- Crash-utility mailing list <a href="mailto:Crash-utility@redhat.com" target="_blank">Crash-utility@redhat.com</a> <a href="https://listman.redhat.com/mailman/listinfo/crash-utility" rel="noreferrer" target="_blank">https://listman.redhat.com/mailman/listinfo/crash-utility</a> ------------------------------ End of Crash-utility Digest, Vol 198, Issue 33 ********************************************** </blockquote></div></div>