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

Date: Wed, 23 Mar 2022 08:09:49 +0000 From: &quot;d.hatayama(a)fujitsu.com&quot; <d.hatayama(a)fujitsu.com&gt; To: &quot;crash-utility(a)redhat.com&quot; <crash-utility(a)redhat.com&gt; Subject: [Crash-utility] [PATCH] kernel: fix start-up time degradation caused by strings command Message-ID: < TYAPR01MB6507E11837E16B365D7743B695189(a)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

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