On Wed, Mar 23, 2022 at 4:11 PM <crash-utility-request@redhat.com> wrote:
Date: Wed, 23 Mar 2022 08:09:49 +0000
From: "d.hatayama@fujitsu.com" <d.hatayama@fujitsu.com>
To: "crash-utility@redhat.com" <crash-utility@redhat.com>
Subject: [Crash-utility] [PATCH] kernel: fix start-up time degradation
        caused by strings command
Message-ID:
        <TYAPR01MB6507E11837E16B365D7743B695189@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

This looks pretty good, thank you for the improvement, Hatayama.
Applied: https://github.com/crash-utility/crash/commit/cd8954023bd474521a9d45e2b09a7bce4174f52f

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@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