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

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

 



-----Original Message-----
> 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>

The patch looks good to me.  It's a huge improvement!

Acked-by: Kazuhito Hagio <k-hagio-ab@xxxxxxx>

Thanks,
Kazu


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

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