4:50 a.m.
On Mon, Mar 28, 2022 at 5:04 PM <crash-utility-request(a)redhat.com> wrote:
Date: Mon, 28 Mar 2022 11:03:38 +0200
From: Alexander Egorenkov <egorenar(a)linux.ibm.com>
To: "d.hatayama(a)fujitsu.com" <d.hatayama(a)fujitsu.com>,
"crash-utility(a)redhat.com" <crash-utility(a)redhat.com>
Subject: Re: [Crash-utility] [PATCH] kernel: fix start-up time
degradation caused by strings command
Message-ID: <87ee2m4ff9.fsf(a)oc8242746057.ibm.com>
Content-Type: text/plain
"d.hatayama(a)fujitsu.com" <d.hatayama(a)fujitsu.com> 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(a)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
>
>
>
> --
> Crash-utility mailing list
> Crash-utility(a)redhat.com
> 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(a)redhat.com
https://listman.redhat.com/mailman/listinfo/crash-utility
------------------------------
End of Crash-utility Digest, Vol 198, Issue 33
**********************************************
4:58 a.m.
New subject: [PATCH] kernel: fix start-up time degradation caused by strings command
Hi,
lijiang <lijiang(a)redhat.com> writes:
On Mon, Mar 28, 2022 at 5:04 PM
<crash-utility-request(a)redhat.com> wrote:
> Date: Mon, 28 Mar 2022 11:03:38 +0200
> From: Alexander Egorenkov <egorenar(a)linux.ibm.com>
> To: "d.hatayama(a)fujitsu.com" <d.hatayama(a)fujitsu.com>,
> "crash-utility(a)redhat.com" <crash-utility(a)redhat.com>
> Subject: Re: [Crash-utility] [PATCH] kernel: fix start-up time
> degradation caused by strings command
> Message-ID: <87ee2m4ff9.fsf(a)oc8242746057.ibm.com>
> Content-Type: text/plain
>
> "d.hatayama(a)fujitsu.com" <d.hatayama(a)fujitsu.com> 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(a)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
> >
> >
> >
> > --
> > Crash-utility mailing list
> > Crash-utility(a)redhat.com
> > 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 ?
This symbol must be present in vmlinux ?
I'm not yet familiar with this.
I'll try to figure it out.
> 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
yes, this works, thanks!
Would be great if we had this fix on master soon :)
> Alex
>
>
>
> ------------------------------
>
> Subject: Digest Footer
>
> --
> Crash-utility mailing list
> Crash-utility(a)redhat.com
> https://listman.redhat.com/mailman/listinfo/crash-utility
>
>
> ------------------------------
>
> End of Crash-utility Digest, Vol 198, Issue 33
> **********************************************
>
>
This Message Is From an External Sender
This message came from outside your organization.
On Mon, Mar 28, 2022 at 5:04 PM <crash-utility-request(a)redhat.com> wrote:
Date: Mon, 28 Mar 2022 11:03:38 +0200
From: Alexander Egorenkov <egorenar(a)linux.ibm.com>
To: "d.hatayama(a)fujitsu.com" <d.hatayama(a)fujitsu.com>,
"crash-utility(a)redhat.com" <crash-utility(a)redhat.com>
Subject: Re: [Crash-utility] [PATCH] kernel: fix start-up time
degradation caused by strings command
Message-ID: <87ee2m4ff9.fsf(a)oc8242746057.ibm.com>
Content-Type: text/plain
"d.hatayama(a)fujitsu.com" <d.hatayama(a)fujitsu.com> 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(a)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
>
>
>
> --
> Crash-utility mailing list
> Crash-utility(a)redhat.com
> 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(a)redhat.com
https://listman.redhat.com/mailman/listinfo/crash-utility
------------------------------
End of Crash-utility Digest, Vol 198, Issue 33
**********************************************
6:13 a.m.
New subject: [PATCH] kernel: fix start-up time degradation caused by strings command
Hi,
lijiang <lijiang(a)redhat.com> writes:
On Mon, Mar 28, 2022 at 5:04 PM
<crash-utility-request(a)redhat.com> wrote:
> Date: Mon, 28 Mar 2022 11:03:38 +0200
> From: Alexander Egorenkov <egorenar(a)linux.ibm.com>
> To: "d.hatayama(a)fujitsu.com" <d.hatayama(a)fujitsu.com>,
> "crash-utility(a)redhat.com" <crash-utility(a)redhat.com>
> Subject: Re: [Crash-utility] [PATCH] kernel: fix start-up time
> degradation caused by strings command
> Message-ID: <87ee2m4ff9.fsf(a)oc8242746057.ibm.com>
> Content-Type: text/plain
>
> "d.hatayama(a)fujitsu.com" <d.hatayama(a)fujitsu.com> 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(a)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
> >
> >
> >
> > --
> > Crash-utility mailing list
> > Crash-utility(a)redhat.com
> > 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
8:57 a.m.
New subject: [PATCH] kernel: fix start-up time degradation caused by strings command
Alexander Egorenkov <egorenar(a)linux.ibm.com> writes:
Hi,
lijiang <lijiang(a)redhat.com> writes:
> On Mon, Mar 28, 2022 at 5:04 PM <crash-utility-request(a)redhat.com> wrote:
>
>> Date: Mon, 28 Mar 2022 11:03:38 +0200
>> From: Alexander Egorenkov <egorenar(a)linux.ibm.com>
>> To: "d.hatayama(a)fujitsu.com" <d.hatayama(a)fujitsu.com>,
>> "crash-utility(a)redhat.com" <crash-utility(a)redhat.com>
>> Subject: Re: [Crash-utility] [PATCH] kernel: fix start-up time
>> degradation caused by strings command
>> Message-ID: <87ee2m4ff9.fsf(a)oc8242746057.ibm.com>
>> Content-Type: text/plain
>>
>> "d.hatayama(a)fujitsu.com" <d.hatayama(a)fujitsu.com> 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(a)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
>> >
>> >
>> >
>> > --
>> > Crash-utility mailing list
>> > Crash-utility(a)redhat.com
>> > 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
midnight
New subject: [PATCH] kernel: fix start-up time degradation caused by strings command
On Mon, Mar 28, 2022 at 9:57 PM Alexander Egorenkov <egorenar(a)linux.ibm.com>
wrote:
> Where does the symbol '.rodata' come from ?
> I couldn't find it in x86's vmlinux with nm as well.
>
The "nm -a" option may be a good choice.
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 ?
I guess that it is to filter out some duplicated or unneeded symbols in
crash-utility, especially for the old kernel. No more changelogs.
Thanks.
Lianbo
Thanks
Regards
Alex
12:28 a.m.
New subject: [PATCH] kernel: fix start-up time degradation caused by strings command
Hi,
lijiang <lijiang(a)redhat.com> writes:
On Mon, Mar 28, 2022 at 9:57 PM Alexander Egorenkov
<egorenar(a)linux.ibm.com>
wrote:
>
> > Where does the symbol '.rodata' come from ?
> > I couldn't find it in x86's vmlinux with nm as well.
> >
>
The "nm -a" option may be a good choice.
> 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 ?
>
I guess that it is to filter out some duplicated or unneeded symbols in
crash-utility, especially for the old kernel. No more changelogs.
Thanks.
Lianbo
> Thanks
> Regards
> Alex
>
>
This Message Is From an External Sender
This message came from outside your organization.
On Mon, Mar 28, 2022 at 9:57 PM Alexander Egorenkov <egorenar(a)linux.ibm.com>
wrote:
> Where does the symbol '.rodata' come from ?
> I couldn't find it in x86's vmlinux with nm as well.
>
The "nm -a" option may be a good choice.
Thanks!
> 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 ?
I guess that it is to filter out some duplicated or unneeded symbols in crash-utility,
especially for the old kernel. No more changelogs.
Do you think it makes sense to add an exception for '.rodata' symbol
to s390x_verify_symbol() ? Something like:
if (STREQ(name), ".rodata"))
return TRUE;
I did a quick test and it works on s390x as well.
Thanks
Regards
Alex
1:04 a.m.
New subject: [PATCH] kernel: fix start-up time degradation caused by strings command
On Tue, Mar 29, 2022 at 1:29 PM Alexander Egorenkov <egorenar(a)linux.ibm.com>
wrote:
Do you think it makes sense to add an exception for '.rodata'
symbol
to s390x_verify_symbol() ? Something like:
if (STREQ(name), ".rodata"))
return TRUE;
Maybe it makes sense, but, need to find out where to filter out this
symbol, it
will have more changes. For example: s390_verify_symbol()...
However, for safety, it could be good to still check the symbols.
Thanks.
Lianbo
I did a quick test and it works on s390x as well.
Thanks
Regards
Alex
825
days inactive
826
days old
devel@lists.crash-utility.osci.io
6 comments
2 participants
participants (2)
-
Alexander Egorenkov -
lijiang