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

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

 



Alexander Egorenkov <egorenar@xxxxxxxxxxxxx> writes:

> Hi,
>
> lijiang <lijiang@xxxxxxxxxx> writes:
>
>> 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 ?
>
> Where does the symbol '.rodata' come from ?
> I couldn't find it in x86's vmlinux with nm as well.
>
> On Fedora 34:
>
> $ nm /usr/lib/debug/lib/modules/5.16.16-100.fc34.x86_64/vmlinux | grep rodata
> ffffffff82c1d000 D __end_rodata
> ffffffff82e00000 D __end_rodata_aligned
> ffffffff82e00000 D __end_rodata_hpage_align
> ffffffff81199420 t frob_rodata
> ffffffff81c3eec7 T mark_rodata_ro
> ffffffff826dd200 D rodata_enabled
> ffffffff82e2cec0 d rodata_resource
> ffffffff81352690 T rodata_test
> ffffffff81c50238 t rodata_test.cold
> ffffffff8223d9d0 d rodata_test_data
> ffffffff836c2c36 t set_debug_rodata
> ffffffff838ff890 d __setup_set_debug_rodata
> ffffffff838b4b18 d __setup_str_set_debug_rodata
> ffffffff82200000 D __start_rodata
>
> Regards
> Alex

Found the reason why all symbols starting with '.' are dropped for s390x

/*
 *  Accept or reject a symbol from the kernel namelist.
 */
static int
s390x_verify_symbol(const char *name, ulong value, char type)
{
	int i;

...

	/* throw away all symbols containing a '.' */
	for(i = 0; i < strlen(name);i++){
		if(name[i] == '.')                    <-------------- !!!
			return FALSE;
	}

	return TRUE;
}


But i have no idea why. Anybody familiar with this ?

Thanks
Regards
Alex

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