Hi, Aditya
Thank you for the patch.
On Wed, Jun 18, 2025 at 1:33 PM Aditya Gupta <adit...@linux.ibm.com> wrote:
> Few vmcores don't have vmcoreinfo elf note, such as those created using
> virsh-dump.
>
> On architectures such as PowerPC64, vmcoreinfo is mandatory to fetch the
> first_vmalloc_address, for vmcores of upstream linux, since crash-utility
> commit:
>
> commit 5b24e363a898 ("get vmalloc start address from vmcoreinfo")
>
> Try reading from the 'vmcoreinfo_data' symbol instead, if the vmcoreinfo
> crash tries to read in case of diskdump/netdump is empty/missing.
>
> The approach to read 'vmcoreinfo_data' was used for a live kernel, which
> can be
> reused in the case of missing vmcoreinfo note also, as the
> 'vmcoreinfo_data' symbol is available with vmcore too
>
> Note though, till GDB interface is not initialised, reading from
> vmcoreinfo_data symbol is not done, so behaviour is same as previously
> with no vmcoreinfo (only till GDB interface is not initialised)
>
> Hence rename 'vmcoreinfo_read_string' in kernel.c to
> 'vmcoreinfo_read_from_memory', and use it in netdump.c and diskdump.c
> too.
>
> Reported-by: Anushree Mathur <anushree.mat...@linux.ibm.com>
> Reported-by: Kowshik Jois <kowsj...@linux.ibm.com>
> Tested-by: Anushree Mathur <anushree.mat...@linux.ibm.com>
> Tested-by: Kowshik Jois <kowsj...@linux.ibm.com>
> Signed-off-by: Aditya Gupta <adit...@linux.ibm.com>
> ---
> defs.h | 1 +
> diskdump.c | 18 ++++++++++++++++++
> kernel.c | 17 ++++++++++++-----
> netdump.c | 19 +++++++++++++++++++
> 4 files changed, 50 insertions(+), 5 deletions(-)
>
> diff --git a/defs.h b/defs.h
> index 2fdb4db56a05..fbd09e19103f 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -6213,6 +6213,7 @@ void dump_kernel_table(int);
> void dump_bt_info(struct bt_info *, char *where);
> void dump_log(int);
> void parse_kernel_version(char *);
> +char *vmcoreinfo_read_from_memory(const char *);
>
> #define LOG_LEVEL(v) ((v) & 0x07)
> #define SHOW_LOG_LEVEL (0x1)
> diff --git a/diskdump.c b/diskdump.c
> index ce3cbb7b12dd..3be56248c7a9 100644
> --- a/diskdump.c
> +++ b/diskdump.c
> @@ -1041,6 +1041,13 @@ pfn_to_pos(ulong pfn)
> return desc_pos;
> }
>
> +/**
> + * Check if vmcoreinfo in vmcore is missing/empty
> + */
> +static bool is_vmcoreinfo_empty(void)
> +{
> + return (dd->sub_header_kdump->size_vmcoreinfo == 0);
> +}
>
>
I have seen two versions of the implementation, another one is in
netdump.c. Can we do it like this?
bool is_vmcoreinfo_empty(void)
{
if (dd && dd->sub_header_kdump)
return (dd->sub_header_kdump->size_vmcoreinfo == 0);
if (nd)
return (nd->size_vmcoreinfo == 0);
return true;
}
And implement it in a common file, E.g: kernel.c, put its definition to
defs.h. So that we can call this one in netdump.c and diskdump.c.
What do you think?
Other changes are fine to me.
Thanks
Lianbo
/*
> * Determine whether a file is a diskdump creation, and if TRUE,
> @@ -1088,6 +1095,17 @@ is_diskdump(char *file)
>
> pc->read_vmcoreinfo = vmcoreinfo_read_string;
>
> + /*
> + * vmcoreinfo can be empty in case of dump collected via virsh-dump
> + *
> + * check if vmcoreinfo is not available in vmcore, and try to read
> + * the vmcoreinfo from memory, using "vmcoreinfo_data" symbol
> + */
> + if (is_vmcoreinfo_empty()) {
> + error(WARNING, "vmcoreinfo is empty, will read from
> symbols\n");
> + pc->read_vmcoreinfo = vmcoreinfo_read_from_memory;
> + }
> +
> if ((pc->flags2 & GET_LOG) && KDUMP_CMPRS_VALID()) {
> pc->dfd = dd->dfd;
> pc->readmem = read_diskdump;
> diff --git a/kernel.c b/kernel.c
> index b8d3b7999974..b296487ea036 100644
> --- a/kernel.c
> +++ b/kernel.c
> @@ -99,7 +99,6 @@ static ulong dump_audit_skb_queue(ulong);
> static ulong __dump_audit(char *);
> static void dump_audit(void);
> 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);
> @@ -11892,8 +11891,8 @@ dump_printk_safe_seq_buf(int msg_flags)
> * Returns a string (that has to be freed by the caller) that contains the
> * value for key or NULL if the key has not been found.
> */
> -static char *
> -vmcoreinfo_read_string(const char *key)
> +char *
> +vmcoreinfo_read_from_memory(const char *key)
> {
> char *buf, *value_string, *p1, *p2;
> size_t value_length;
> @@ -11903,6 +11902,14 @@ vmcoreinfo_read_string(const char *key)
>
> buf = value_string = NULL;
>
> + if (!(pc->flags & GDB_INIT)) {
> + /*
> + * GDB interface hasn't been initialised yet, so can't
> + * access vmcoreinfo_data
> + */
> + return NULL;
> + }
> +
> switch (get_symbol_type("vmcoreinfo_data", NULL, NULL))
> {
> case TYPE_CODE_PTR:
> @@ -11958,10 +11965,10 @@ check_vmcoreinfo(void)
> switch (get_symbol_type("vmcoreinfo_data", NULL, NULL))
> {
> case TYPE_CODE_PTR:
> - pc->read_vmcoreinfo = vmcoreinfo_read_string;
> + pc->read_vmcoreinfo = vmcoreinfo_read_from_memory;
> break;
> case TYPE_CODE_ARRAY:
> - pc->read_vmcoreinfo = vmcoreinfo_read_string;
> + pc->read_vmcoreinfo = vmcoreinfo_read_from_memory;
> break;
> }
> }
> diff --git a/netdump.c b/netdump.c
> index c7ff009e7f90..c9f0e4eaa580 100644
> --- a/netdump.c
> +++ b/netdump.c
> @@ -111,6 +111,14 @@ map_cpus_to_prstatus(void)
> FREEBUF(nt_ptr);
> }
>
> +/**
> + * Check if vmcoreinfo in vmcore is missing/empty
> + */
> +static bool is_vmcoreinfo_empty(void)
> +{
> + return (nd->size_vmcoreinfo == 0);
> +}
> +
> /*
> * Determine whether a file is a netdump/diskdump/kdump creation,
> * and if TRUE, initialize the vmcore_data structure.
> @@ -464,6 +472,17 @@ is_netdump(char *file, ulong source_query)
>
> pc->read_vmcoreinfo = vmcoreinfo_read_string;
>
> + /*
> + * vmcoreinfo can be empty in case of dump collected via virsh-dump
> + *
> + * check if vmcoreinfo is not available in vmcore, and try to read
> + * the vmcoreinfo from memory, using "vmcoreinfo_data" symbol
> + */
> + if (is_vmcoreinfo_empty()) {
> + error(WARNING, "vmcoreinfo is empty, will read from
> symbols\n");
> + pc->read_vmcoreinfo = vmcoreinfo_read_from_memory;
> + }
> +
> if ((source_query == KDUMP_LOCAL) &&
> (pc->flags2 & GET_OSRELEASE))
> kdump_get_osrelease();
> --
> 2.49.0
>
>
--
Crash-utility mailing list -- devel@lists.crash-utility.osci.io
To unsubscribe send an email to devel-le...@lists.crash-utility.osci.io
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
