Date: Mon, 28 Mar 2022 11:03:38 +0200
From: Alexander Egorenkov <egorenar@linux.ibm.com>
To: "d.hatayama@fujitsu.com" <d.hatayama@fujitsu.com>,
"crash-utility@redhat.com" <crash-utility@redhat.com>
Subject: Re: [Crash-utility] [PATCH] kernel: fix start-up time
degradation caused by strings command
Message-ID: <87ee2m4ff9.fsf@oc8242746057.ibm.com>
Content-Type: text/plain
"d.hatayama@fujitsu.com" <d.hatayama@fujitsu.com> 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@fujitsu.com>
> ---
> 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@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 ?
Maybe test for its existence first ?
Does the following patch work for you?
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;
Thanks.
Lianbo
Regards
Alex
------------------------------
Subject: Digest Footer
--
Crash-utility mailing list
Crash-utility@redhat.com
https://listman.redhat.com/mailman/listinfo/crash-utility
------------------------------
End of Crash-utility Digest, Vol 198, Issue 33
**********************************************