On Wed, Mar 23, 2022 at 4:11 PM <crash-utility-request@xxxxxxxxxx> wrote:
Date: Wed, 23 Mar 2022 08:09:49 +0000
From: "d.hatayama@xxxxxxxxxxx" <d.hatayama@xxxxxxxxxxx>
To: "crash-utility@xxxxxxxxxx" <crash-utility@xxxxxxxxxx>
Subject: [PATCH] kernel: fix start-up time degradation
caused by strings command
Message-ID:
<TYAPR01MB6507E11837E16B365D7743B695189@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
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
This looks pretty good, thank you for the improvement, Hatayama.
BTW: Currently, crash performance issues may become more and more prominent on multi-core and large memory systems, hope to have more optimization work in the future.
Thanks.
Lianbo
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@xxxxxxxxxxx>
---
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@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/crash-utility Contribution Guidelines: https://github.com/crash-utility/crash/wiki
