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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Mar 28, 2022 at 5:04 PM <crash-utility-request@xxxxxxxxxx> wrote:
Date: Mon, 28 Mar 2022 11:03:38 +0200
From: Alexander Egorenkov <egorenar@xxxxxxxxxxxxx>
To: "d.hatayama@xxxxxxxxxxx" <d.hatayama@xxxxxxxxxxx>,
        "crash-utility@xxxxxxxxxx" <crash-utility@xxxxxxxxxx>
Subject: Re: [PATCH] kernel: fix start-up time
        degradation caused by strings command
Message-ID: <87ee2m4ff9.fsf@xxxxxxxxxxxxxxxxxxxx>
Content-Type: text/plain

"d.hatayama@xxxxxxxxxxx" <d.hatayama@xxxxxxxxxxx> 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@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

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@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility


------------------------------

End of Crash-utility Digest, Vol 198, Issue 33
**********************************************

--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki

[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux