[PATCH v4 0/5] gdb multi-stack unwinding support
by Tao Liu
This patchset is based on Alexy's work [1], and is the follow-up of the
previous "gdb stack unwinding support for crash utility" patchset.
Currently gdb target analyzes only one task at a time and it backtraces
only straight stack until end of the stack. If stacks were concatenated
during exceptions or interrupts, gdb bt will show only the topmost one.
This patchset will introduce multiple stacks support for gdb stack unwinding,
which can be observed as a different threads from gdb perspective. A
short usage is as follows:
'set <PID>' - to switch to a specific task
'gdb info threads' - to see list of in-kernel stacks of this task.
'gdb thread <ID>' - to switch to the stack.
'gdb bt' - to unwind it.
E.g, with the patchset:
crash> bt
PID: 17636 TASK: ffff88032e0742c0 CPU: 11 COMMAND: "kworker/11:4"
#0 [ffff88037fca6b58] machine_kexec at ffffffff8103cef2
#1 [ffff88037fca6ba8] crash_kexec at ffffffff810c9aa3
#2 [ffff88037fca6c70] panic at ffffffff815f0444
...
#9 [ffff88037fca6ec8] do_nmi at ffffffff815fd980
#10 [ffff88037fca6ef0] end_repeat_nmi at ffffffff815fcec1
[exception RIP: memcpy+13]
RIP: ffffffff812f5b1d RSP: ffff88034f2a9728 RFLAGS: 00010046
RAX: ffffc900139fe000 RBX: ffff880374b7a1b0 RCX: 0000000000000030
RBP: ffff88034f2a9778 R8: 000000007fffffff R9: 00000000ffffffff
...
ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018
--- <NMI exception stack> ---
#11 [ffff88034f2a9728] memcpy at ffffffff812f5b1d
#12 [ffff88034f2a9728] mga_dirty_update at ffffffffa024ad2b [mgag200]
#13 [ffff88034f2a9780] mga_imageblit at ffffffffa024ae3f [mgag200]
#14 [ffff88034f2a97a0] bit_putcs at ffffffff813424ef
...
crash> info threads
Id Target Id Frame
* 1 17636 kworker/11:4 (stack 0) crash_setup_regs (oldregs=0x0, newregs=0xffff88037fca6bb0)
2 17636 kworker/11:4 (stack 1) 0xffffffff812f5b1d in memcpy ()
crash> thread 2
crash> gdb bt
#0 0xffffffff812f5b1d in memcpy () at arch/x86/lib/memcpy_64.S:69
...
There are 2 stacks of the current task, and we can list/switch-to/unwind
each stack.
[1]: https://www.mail-archive.com/devel@lists.crash-utility.osci.io/msg01204.html
v2 -> v1: 1) Rebase this patchset onto gdb-16.2 [2].
2) Improved the silent_call_bt() to catch the error FATAL.
[2]: https://www.mail-archive.com/devel@lists.crash-utility.osci.io/msg01354.html
v3 -> v2: 1) Rebase this patchset to crash v9.0.0.
2) Fix v2's segfault in cmd "bt -E".
3) Elimit repeat stacks by adding constraints before
gdb_add_substack().
v4 -> v3: 1) Fix compiling warning of silent_call_bt()
2) Add known issue link found in ppc arch.
Tao Liu (5):
Add multi-threads support in crash target
Call cmd_bt silently after "set pid"
x86_64: Add gdb multi-stack unwind support
arm64: Add gdb multi-stack unwind support
ppc64: Add gdb multi-stack unwind support
arm64.c | 102 +++++++++++++++++++++++++++++++++--
crash_target.c | 49 +++++++++++++++--
defs.h | 3 +-
gdb_interface.c | 6 +--
kernel.c | 44 +++++++++++++++
ppc64.c | 78 ++++++++++++++++++++++++---
task.c | 4 +-
x86_64.c | 138 +++++++++++++++++++++++++++++++++++++++++++++---
8 files changed, 394 insertions(+), 30 deletions(-)
--
2.47.0
2 months, 1 week
"make target=ARM" fails on Ubuntu24 LTS
by Naveen Kumar Chaudhary
I am trying to build crash-9.0.0 on Ubuntu 24 LTS, but it always fails with
below error :
configure: error: Building GDB requires GMP 4.2+, and MPFR 3.1.0+.
Try the --with-gmp and/or --with-mpfr options to specify
$ pkg-config --modversion gmp
6.3.0
$ pkg-config --modversion mpfr
4.2.1
$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description: Ubuntu 24.04.1 LTS
Release: 24.04
Codename: noble
The packages are properly installed and the header files are also present :
$ ls /usr/include/x86_64-linux-gnu/gmp.h
/usr/include/x86_64-linux-gnu/gmp.h
$ ls /usr/include/mpfr.h
/usr/include/mpfr.h
I even tried this :
$ CPPFLAGS="-I/usr/include/x86_64-linux-gnu -I/usr/include"
LDFLAGS="-L/usr/lib/x86_64-linux-gnu" make target=ARM
but still same error. Can someone please give any pointers.
Regards,
Naveen
2 months, 1 week
Re: [PATCH v3 5/5] ppc64: Add gdb multi-stack unwind support
by lijiang
For ppc64 machine, I noticed that the gdb bt may not work as expected, for
example:
crash> set 2
PID: 2
COMMAND: "kthreadd"
TASK: c000000004797f80 [THREAD_INFO: c000000004797f80]
CPU: 0
STATE: TASK_INTERRUPTIBLE
crash> bt
PID: 2 TASK: c000000004797f80 CPU: 0 COMMAND: "kthreadd"
#0 [c00000000484fbc0] _end at c00000000484fd70 (unreliable)
#1 [c00000000484fd70] __switch_to at c00000000001fabc
#2 [c00000000484fdd0] __schedule at c0000000011ca9dc
#3 [c00000000484feb0] schedule at c0000000011caeb0
#4 [c00000000484ff20] kthreadd at c0000000001af6c4
#5 [c00000000484ffe0] start_kernel_thread at c00000000000ded8
crash> gdb bt
#0 0xc00000000484fd70 in ?? ()
gdb: gdb request failed: bt
crash>
crash> sys|grep RELEASE
RELEASE: 6.12.0
Is that expected behavior?(I do not remember if I mentioned the similar
issue in the previous patch review.)
I have no more comments for other changes, just three issues(see another
two comments).
Thanks
Lianbo
On Tue, Jun 3, 2025 at 1:18 PM <devel-request(a)lists.crash-utility.osci.io>
wrote:
> Date: Tue, 3 Jun 2025 17:11:38 +1200
> From: Tao Liu <ltao(a)redhat.com>
> Subject: [Crash-utility] [PATCH v3 5/5] ppc64: Add gdb multi-stack
> unwind support
> To: devel(a)lists.crash-utility.osci.io
> Cc: Alexey Makhalov <alexey.makhalov(a)broadcom.com>
> Message-ID: <20250603051138.59896-6-ltao(a)redhat.com>
> Content-Type: text/plain; charset="US-ASCII"; x-default=true
>
> Co-developed-by: Alexey Makhalov <alexey.makhalov(a)broadcom.com>
> Co-developed-by: Tao Liu <ltao(a)redhat.com>
> Signed-off-by: Tao Liu <ltao(a)redhat.com>
> ---
> ppc64.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 64 insertions(+), 6 deletions(-)
>
> diff --git a/ppc64.c b/ppc64.c
> index 532eb3f..d1a5067 100644
> --- a/ppc64.c
> +++ b/ppc64.c
> @@ -2053,6 +2053,7 @@ ppc64_back_trace_cmd(struct bt_info *bt)
> char buf[BUFSIZE];
> struct gnu_request *req;
> extern void print_stack_text_syms(struct bt_info *, ulong, ulong);
> + extra_stacks_idx = 0;
>
> bt->flags |= BT_EXCEPTION_FRAME;
>
> @@ -2071,6 +2072,29 @@ ppc64_back_trace_cmd(struct bt_info *bt)
> req->pc = bt->instptr;
> req->sp = bt->stkptr;
>
> + if (is_task_active(bt->task)) {
> + if (!extra_stacks_regs[extra_stacks_idx]) {
> + extra_stacks_regs[extra_stacks_idx] =
> + (struct user_regs_bitmap_struct *)
> + malloc(sizeof(struct
> user_regs_bitmap_struct));
> + }
> + memset(extra_stacks_regs[extra_stacks_idx], 0,
> + sizeof(struct user_regs_bitmap_struct));
> + extra_stacks_regs[extra_stacks_idx]->ur.nip = req->pc;
> + extra_stacks_regs[extra_stacks_idx]->ur.gpr[1] = req->sp;
> + SET_BIT(extra_stacks_regs[extra_stacks_idx]->bitmap,
> + REG_SEQ(ppc64_pt_regs, nip));
> + SET_BIT(extra_stacks_regs[extra_stacks_idx]->bitmap,
> + REG_SEQ(ppc64_pt_regs, gpr[0]) + 1);
> + if (!bt->machdep ||
> + (extra_stacks_regs[extra_stacks_idx]->ur.gpr[1] !=
> + ((struct user_regs_bitmap_struct
> *)(bt->machdep))->ur.gpr[1] &&
> + extra_stacks_regs[extra_stacks_idx]->ur.nip !=
> + ((struct user_regs_bitmap_struct
> *)(bt->machdep))->ur.nip)) {
> + gdb_add_substack (extra_stacks_idx++);
> + }
> + }
> +
> if (bt->flags &
> (BT_TEXT_SYMBOLS|BT_TEXT_SYMBOLS_PRINT|BT_TEXT_SYMBOLS_NOPRINT)) {
> if (!INSTACK(req->sp, bt))
> @@ -2512,6 +2536,28 @@ ppc64_print_eframe(char *efrm_str, struct
> ppc64_pt_regs *regs,
> fprintf(fp, " %s [%lx] exception frame:\n", efrm_str, regs->trap);
> ppc64_print_regs(regs);
> ppc64_print_nip_lr(regs, 1);
> +
> + if (!((regs->msr >> MSR_PR_LG) & 0x1) &&
> + !(bt->flags & BT_EFRAME_SEARCH)) {
> + if (!extra_stacks_regs[extra_stacks_idx]) {
> + extra_stacks_regs[extra_stacks_idx] =
> + (struct user_regs_bitmap_struct *)
> + malloc(sizeof(struct
> user_regs_bitmap_struct));
> + }
> + memset(extra_stacks_regs[extra_stacks_idx], 0,
> + sizeof(struct user_regs_bitmap_struct));
> + memcpy(&extra_stacks_regs[extra_stacks_idx]->ur, regs,
> + sizeof(struct ppc64_pt_regs));
> + for (int i = 0; i < sizeof(struct
> ppc64_pt_regs)/sizeof(ulong); i++)
> +
> SET_BIT(extra_stacks_regs[extra_stacks_idx]->bitmap, i);
> + if (!bt->machdep ||
> + (extra_stacks_regs[extra_stacks_idx]->ur.gpr[1] !=
> + ((struct user_regs_bitmap_struct
> *)(bt->machdep))->ur.gpr[1] &&
> + extra_stacks_regs[extra_stacks_idx]->ur.nip !=
> + ((struct user_regs_bitmap_struct
> *)(bt->machdep))->ur.nip)) {
> + gdb_add_substack (extra_stacks_idx++);
> + }
> + }
> }
>
> static int
> @@ -2552,6 +2598,12 @@ ppc64_get_current_task_reg(int regno, const char
> *name, int size,
> tc = CURRENT_CONTEXT();
> if (!tc)
> return FALSE;
> +
> + if (sid && sid <= extra_stacks_idx) {
> + ur_bitmap = extra_stacks_regs[sid - 1];
> + goto get_sub;
> + }
> +
> BZERO(&bt_setup, sizeof(struct bt_info));
> clone_bt_info(&bt_setup, &bt_info, tc);
> fill_stackbuf(&bt_info);
> @@ -2570,39 +2622,45 @@ ppc64_get_current_task_reg(int regno, const char
> *name, int size,
> goto get_all;
> }
>
> +get_sub:
> switch (regno) {
> case PPC64_R0_REGNUM ... PPC64_R31_REGNUM:
> if (!NUM_IN_BITMAP(ur_bitmap->bitmap,
> REG_SEQ(ppc64_pt_regs, gpr[0]) + regno -
> PPC64_R0_REGNUM)) {
> - FREEBUF(ur_bitmap);
> + if (!sid)
> + FREEBUF(ur_bitmap);
> return FALSE;
> }
> break;
> case PPC64_PC_REGNUM:
> if (!NUM_IN_BITMAP(ur_bitmap->bitmap,
> REG_SEQ(ppc64_pt_regs, nip))) {
> - FREEBUF(ur_bitmap);
> + if (!sid)
> + FREEBUF(ur_bitmap);
> return FALSE;
> }
> break;
> case PPC64_MSR_REGNUM:
> if (!NUM_IN_BITMAP(ur_bitmap->bitmap,
> REG_SEQ(ppc64_pt_regs, msr))) {
> - FREEBUF(ur_bitmap);
> + if (!sid)
> + FREEBUF(ur_bitmap);
> return FALSE;
> }
> break;
> case PPC64_LR_REGNUM:
> if (!NUM_IN_BITMAP(ur_bitmap->bitmap,
> REG_SEQ(ppc64_pt_regs, link))) {
> - FREEBUF(ur_bitmap);
> + if (!sid)
> + FREEBUF(ur_bitmap);
> return FALSE;
> }
> break;
> case PPC64_CTR_REGNUM:
> if (!NUM_IN_BITMAP(ur_bitmap->bitmap,
> REG_SEQ(ppc64_pt_regs, ctr))) {
> - FREEBUF(ur_bitmap);
> + if (!sid)
> + FREEBUF(ur_bitmap);
> return FALSE;
> }
> break;
> @@ -2645,7 +2703,7 @@ get_all:
> ret = TRUE;
> break;
> }
> - if (bt_info.need_free) {
> + if (!sid && bt_info.need_free) {
> FREEBUF(ur_bitmap);
> bt_info.need_free = FALSE;
> }
> --
> 2.47.0
>
2 months, 2 weeks
Re: [PATCH v3 2/5] Call cmd_bt silently after "set pid"
by lijiang
Thank you for working on this.
On Tue, Jun 3, 2025 at 1:15 PM <devel-request(a)lists.crash-utility.osci.io>
wrote:
> Date: Tue, 3 Jun 2025 17:11:35 +1200
> From: Tao Liu <ltao(a)redhat.com>
> Subject: [Crash-utility] [PATCH v3 2/5] Call cmd_bt silently after
> "set pid"
> To: devel(a)lists.crash-utility.osci.io
> Cc: Alexey Makhalov <alexey.makhalov(a)broadcom.com>
> Message-ID: <20250603051138.59896-3-ltao(a)redhat.com>
> Content-Type: text/plain; charset="US-ASCII"; x-default=true
>
> Cmd bt will list multi-stacks of one task. After we "set <pid>" switch
> context to one task, we first need a bt call to detect the multi-stacks,
> however we don't want any console output from it, so a nullfp is used for
> output receive. The silent bt call is only triggered once as part of task
> context switch by cmd set.
>
> A array of user_regs pointers is reserved for each supported arch. If one
> extra stack found, a user_regs structure will be allocated for storing regs
> value of the stack.
>
> Co-developed-by: Alexey Makhalov <alexey.makhalov(a)broadcom.com>
> Co-developed-by: Tao Liu <ltao(a)redhat.com>
> Signed-off-by: Tao Liu <ltao(a)redhat.com>
> ---
> arm64.c | 4 ++++
> crash_target.c | 7 +++++++
> kernel.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> ppc64.c | 4 ++++
> task.c | 4 ++--
> x86_64.c | 3 +++
> 6 files changed, 63 insertions(+), 2 deletions(-)
>
> diff --git a/arm64.c b/arm64.c
> index 1cdde5f..8291301 100644
> --- a/arm64.c
> +++ b/arm64.c
> @@ -126,6 +126,10 @@ struct user_regs_bitmap_struct {
> ulong bitmap[32];
> };
>
> +#define MAX_EXCEPTION_STACKS 7
> +ulong extra_stacks_idx = 0;
> +struct user_regs_bitmap_struct *extra_stacks_regs[MAX_EXCEPTION_STACKS] =
> {0};
> +
> static inline bool is_mte_kvaddr(ulong addr)
> {
> /* check for ARM64_MTE enabled */
> diff --git a/crash_target.c b/crash_target.c
> index 71998ef..ad1480c 100644
> --- a/crash_target.c
> +++ b/crash_target.c
> @@ -31,6 +31,9 @@ extern "C" int crash_get_current_task_reg (int regno,
> const char *regname,
> extern "C" int gdb_change_thread_context (void);
> extern "C" int gdb_add_substack (int);
> extern "C" void crash_get_current_task_info(unsigned long *pid, char
> **comm);
> +#if defined (X86_64) || defined (ARM64) || defined (PPC64)
> +extern "C" void silent_call_bt(void);
> +#endif
>
> /* The crash target. */
>
> @@ -164,6 +167,10 @@ gdb_change_thread_context (void)
> /* 3rd, refresh regcache for tid 0 */
> target_fetch_registers(get_thread_regcache(inferior_thread()), -1);
> reinit_frame_cache();
> +#if defined (X86_64) || defined (ARM64) || defined (PPC64)
> + /* 4th, invoke bt silently to refresh the additional stacks */
> + silent_call_bt();
> +#endif
> return TRUE;
> }
>
> diff --git a/kernel.c b/kernel.c
> index b8d3b79..b15c32c 100644
> --- a/kernel.c
> +++ b/kernel.c
> @@ -12002,3 +12002,46 @@ int get_linux_banner_from_vmlinux(char *buf,
> size_t size)
>
> return TRUE;
> }
> +
> +#if defined(X86_64) || defined(ARM64) || defined(PPC64)
> +extern ulong extra_stacks_idx;
> +extern void *extra_stacks_regs[];
> +void silent_call_bt(void)
> +{
> + jmp_buf main_loop_env_save;
> + unsigned long long flags_save = pc->flags;
> + FILE *fp_save = fp;
> + FILE *error_fp_save = pc->error_fp;
> + /* Redirect all cmd_bt() outputs into null */
> + fp = pc->nullfp;
> + pc->error_fp = pc->nullfp;
> +
> + for (int i = 0; i < extra_stacks_idx; i++) {
> + /* Note: GETBUF/FREEBUF is not applicable for
> extra_stacks_regs,
> + because we are reserving extra_stacks_regs by cmd_bt()
> + for later use. But GETBUF/FREEBUF is designed for use
> only
> + within one cmd. See process_command_line() ->
> restore_sanity()
> + -> free_all_bufs(). So we use malloc/free instead. */
> + free(extra_stacks_regs[i]);
> + extra_stacks_regs[i] = NULL;
> + }
> + /* Prepare args used by cmd_bt() */
> + sprintf(pc->command_line, "bt\n");
> + argcnt = parse_line(pc->command_line, args);
> + optind = 1;
> + pc->flags |= RUNTIME;
>
Is this unnecessary? The flag 'RUNTIME' is set only once when
initializing(see the main_loop()), and will never be cleared.
Again: a warning observed
gcc -c -g -DX86_64 -DLZO -DGDB_16_2 kernel.c -I./gdb-16.2/bfd
-I./gdb-16.2/include -Wall -O2 -Wstrict-prototypes -Wmissing-prototypes
-fstack-protector -Wformat-security
kernel.c:12009:6: warning: no previous prototype for ‘silent_call_bt’
[-Wmissing-prototypes]
12009 | void silent_call_bt(void)
| ^~~~~~~~~~~~~~
Thanks
Lianbo
+
> + /* Catch error FATAL generated by cmd_bt() if any */
> + memcpy(&main_loop_env_save, &pc->main_loop_env, sizeof(jmp_buf));
> + if (setjmp(pc->main_loop_env)) {
> + goto out;
> + }
> + cmd_bt();
> +out:
> + /* Restore all */
> + memcpy(&pc->main_loop_env, &main_loop_env_save, sizeof(jmp_buf));
> + pc->flags = flags_save;
> + fp = fp_save;
> + pc->error_fp = error_fp_save;
> +}
> +#endif
> diff --git a/ppc64.c b/ppc64.c
> index 7ac12fe..532eb3f 100644
> --- a/ppc64.c
> +++ b/ppc64.c
> @@ -80,6 +80,10 @@ struct user_regs_bitmap_struct {
> ulong bitmap[32];
> };
>
> +#define MAX_EXCEPTION_STACKS 7
> +ulong extra_stacks_idx = 0;
> +struct user_regs_bitmap_struct *extra_stacks_regs[MAX_EXCEPTION_STACKS] =
> {0};
> +
> static int is_opal_context(ulong sp, ulong nip)
> {
> uint64_t opal_start, opal_end;
> diff --git a/task.c b/task.c
> index e07b479..ec04b55 100644
> --- a/task.c
> +++ b/task.c
> @@ -3062,7 +3062,7 @@ sort_context_array(void)
> curtask = CURRENT_TASK();
> qsort((void *)tt->context_array, (size_t)tt->running_tasks,
> sizeof(struct task_context), sort_by_pid);
> - set_context(curtask, NO_PID, TRUE);
> + set_context(curtask, NO_PID, FALSE);
>
> sort_context_by_task();
> }
> @@ -3109,7 +3109,7 @@ sort_context_array_by_last_run(void)
> curtask = CURRENT_TASK();
> qsort((void *)tt->context_array, (size_t)tt->running_tasks,
> sizeof(struct task_context), sort_by_last_run);
> - set_context(curtask, NO_PID, TRUE);
> + set_context(curtask, NO_PID, FALSE);
>
> sort_context_by_task();
> }
> diff --git a/x86_64.c b/x86_64.c
> index a46fb9d..ee23d8b 100644
> --- a/x86_64.c
> +++ b/x86_64.c
> @@ -160,6 +160,9 @@ struct user_regs_bitmap_struct {
> ulong bitmap[32];
> };
>
> +ulong extra_stacks_idx = 0;
> +struct user_regs_bitmap_struct *extra_stacks_regs[MAX_EXCEPTION_STACKS] =
> {0};
> +
> /*
> * Do all necessary machine-specific setup here. This is called several
> * times during initialization.
> --
> 2.47.0
>
2 months, 2 weeks
Re: [PATCH] Fix "kmem -p" option on Linux 6.16-rc1 and later kernels
by lijiang
Thank you for the fix, Kazu.
On Wed, Jun 18, 2025 at 2:05 PM <devel-request(a)lists.crash-utility.osci.io>
wrote:
> Date: Tue, 17 Jun 2025 06:08:52 +0000
> From: HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com>
> Subject: [Crash-utility] [PATCH] Fix "kmem -p" option on Linux
> 6.16-rc1 and later kernels
> To: "devel(a)lists.crash-utility.osci.io"
> <devel(a)lists.crash-utility.osci.io>
> Message-ID: <1750140529-10427-1-git-send-email-k-hagio-ab(a)nec.com>
> Content-Type: text/plain; charset="iso-2022-jp"
>
> Kernel commit acc53a0b4c156 ("mm: rename page->index to
> page->__folio_index"), which is contained in Linux 6.16-rc1 and later
> kernels, renamed the member. Without the patch, the "kmem -p" option
> fails with the following error:
>
> kmem: invalid structure member offset: page_index
> FILE: memory.c LINE: 6016 FUNCTION: dump_mem_map_SPARSEMEM()
>
> Signed-off-by: Kazuhito Hagio <k-hagio-ab(a)nec.com>
> ---
> memory.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/memory.c b/memory.c
> index 0d8d89862383..5cb8b58e2181 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -531,6 +531,8 @@ vm_init(void)
> ASSIGN_OFFSET(page_mapping) = MEMBER_OFFSET("page",
> "_mapcount") +
> STRUCT_SIZE("atomic_t") + sizeof(ulong);
> MEMBER_OFFSET_INIT(page_index, "page", "index");
> + if (INVALID_MEMBER(page_index)) /* 6.16 and later */
> + MEMBER_OFFSET_INIT(page_index, "page", "__folio_index");
>
This looks good to me, so: Ack.
Thanks
Lianbo
> if (INVALID_MEMBER(page_index))
> ANON_MEMBER_OFFSET_INIT(page_index, "page", "index");
> MEMBER_OFFSET_INIT(page_buffers, "page", "buffers");
> --
> 2.31.1
>
2 months, 2 weeks
Re: [PATCH] Fix the "ps -m" command shows wrong duration of RU task
by lijiang
On Mon, May 26, 2025 at 5:04 PM <devel-request(a)lists.crash-utility.osci.io>
wrote:
> Date: Fri, 23 May 2025 05:23:40 +0000
> From: HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com>
> Subject: [Crash-utility] Re: [PATCH] Fix the "ps -m" command shows
> wrong duration of RU task
> To: Tao Liu <ltao(a)redhat.com>, Ke Yin <kyin(a)redhat.com>
> Cc: "devel(a)lists.crash-utility.osci.io"
> <devel(a)lists.crash-utility.osci.io>
> Message-ID: <23c74e07-d439-422a-bbea-8b2bf49b38f1(a)nec.com>
> Content-Type: text/plain; charset="utf-8"
>
> Hi,
>
> On 2025/05/15 8:43, Tao Liu wrote:
> > Hi Ke Yin,
> >
> > On Wed, May 14, 2025 at 8:58 PM Ke Yin <kyin(a)redhat.com> wrote:
> >>
> >> Hi Tao Liu & Kazu,
> >>
> >> Thanks for replying and sharing your thoughts.
> >>
> >> After a quick review of crash tool code, I found:
> >>
> >> runq -m will call dump_on_rq_milliseconds() to print the amount
> >> of time that the active task on each cpu has been running,
> >> but only for the current running task.
> >>
> >> runq -d will call dump_on_rq_tasks() to print all tasks in the run queue
> >> and the task running on cpu without calling translate_nanoseconds().
> >>
> >> My preliminary idea is to combine these two functions and add a new
> >> parameter, for example -q, to print the tasks on each cpu that has
> >> been waiting in the run queue only. And as well as update doc of runq.
> >>
> >> In short:
> >> runq -q will call new_function which is the modified function based on
> dump_on_rq_tasks() (skip current + translate_nanoseconds).
> >>
> >> What do you think?
>
> I didn't know the "runq -d" option because it's a kind of debugging
> option and has no description in the help page. Also it searches all
> tasks for ones that have on_rq = 1 and doesn't look very efficient
> (nr_tasks * nr_cpus). so ideally, maybe a new function should be based
> on dump_runq() than based on dump_on_rq_tasks(), if possible..
>
>
Looks like getting a solution: adding a new option(E.g: runq -q) to achieve
this purpose? Or need more discussion? Any update?
Thanks
Lianbo
> Thanks,
> Kazu
>
>
> >
> > I'm OK with your idea in general. Please check if I understood
> > correctly, your implementation is like:
> > cmd_runq() {
> > ...
> > if (-d option) {
> > dump_on_rq_tasks(old path);
> > } else if (-q option) {
> > dump_on_rq_tasks(new path);
> > }
> > }
> >
> > dump_on_rq_tasks(option)
> > {
> > ...
> > for (i = 0; i < RUNNING_TASKS(); i++, tc++) {
> > if (old path) // Old path stay unchanged
> > dump_task_runq_entry(tc, 0);
> > else // New path will output your time duration
> > your_new_function_with_translate_nanoseconds();
> > }
> > }
> >
> > Thanks,
> > Tao Liu
> >
> >>
> >> Thanks
> >> Kenneth Yin
> >>
> >>
> >>
> >>
> >> On Mon, May 12, 2025 at 1:36 PM Tao Liu <ltao(a)redhat.com> wrote:
> >>>
> >>> Hi Kazu & Kenneth,
> >>>
> >>> Sorry for the late reply, and thanks for your fix and comments!
> >>>
> >>> On Thu, May 8, 2025 at 12:20 PM HAGIO KAZUHITO(萩尾 一仁)
> >>> <k-hagio-ab(a)nec.com> wrote:
> >>>>
> >>>> On 2025/05/07 16:16, HAGIO KAZUHITO(萩尾 一仁) wrote:
> >>>>> Hi,
> >>>>>
> >>>>> On 2025/04/28 19:38, Kenneth Yin wrote:
> >>>>>> The RU/TASK_RUNNING stat means the task is runnable.
> >>>>>> It is either currently running or on a run queue waiting to run.
> >>>>>>
> >>>>>> Currently, the crash tool uses the "rq_clock -
> sched_info->last_arrival" formula to
> >>>>>> calculate the duration of task in RU state. This is for the
> scenario of a task running on a CPU.
> >>>>>
> >>>>> The "ps -l" and "ps -m" options display what their help text
> describes,
> >>>>> not the duration of task in RU state. Please see "help ps".
> >>>>>
> >>>>> Also, tasks are sorted by the value, using different values for it
> could
> >>>>> make another confusion.
> >>>>>
> >>>>> The options have been used for a long time with the current code, if
> we
> >>>>> change the semantics of the options, it would be better to be
> careful.
> >>>>> The change might lose a kind of information instead of getting
> another
> >>>>> kind of information.
> >>>>>
> >>>>> On the other hand, I think that the duration of waiting in queue
> might
> >>>>> also be useful information. I'm not sure how we should display them,
> >>>>> but for example, how about adding a new option or adding a column for
> >>>>> last_queued?
> >>>>
> >>>> I thought of that the "runq" command might be suitable to display the
> >>>> waiting duration, because only tasks in the run queues have it. For
> >>>> example, extending the "runq -m" option or adding a new option. just
> my
> >>>> thought.
> >>>>
> >>>> Thanks,
> >>>> Kazu
> >>>>
> >>>>>
> >>>>> What do you think, folks?
> >>>>>
> >>>>> Thanks,
> >>>>> Kazu
> >>>>>
> >>>>>>
> >>>>>> But for the scenario of a task waiting in the CPU run queue (due
> to some reason
> >>>>>> for example cfs/rt queue throttled), this formula could cause
> misunderstanding.
> >>>>>>
> >>>>>> For example:
> >>>>>> [ 220 10:36:38.026] [RU] PID: 12345 TASK: ffff8d674ab6b180 CPU:
> 1 COMMAND: "task"
> >>>>>>
> >>>>>> Looking closer:
> >>>>>>
> >>>>>> crash> rq.clock ffff8de438a5acc0
> >>>>>> clock = 87029229985307234,
> >>>>>>
> >>>>>> crash> task -R sched_info,se.exec_start
> >>>>>> PID: 12345 TASK: ffff8d674ab6b180 CPU: 1 COMMAND: "task"
> >>>>>> sched_info = {
> >>>>>> pcount = 33,
> >>>>>> run_delay = 0,
> >>>>>> last_arrival = 67983031958439673,
> >>>>>> last_queued = 87029224561119369
> >>>>>> },
> >>>>>> se.exec_start = 67983031958476937,
> >>>>>>
> >>>>>> 67983031 67983031 87029224
> 87029229
> >>>>>> |<- running on CPU ->| <- IN ->|<- waiting in queue
> ->|
> >>>>>>
> >>>>>> For this scenario, the "task" was waiting in the run queue of the
> CPU only for 5 seconds,
> >>>>>> we should use the "rq_clock - sched_info->last_queued" formula.
> >>>
> >>> Please check if my understanding is correct:
> >>>
> >>> The result you saw is "rq_clock - sched_info->last_arrival == 87029229
> >>> - 67983031 == 19046198"
> >>> The expected result you want is: "rq_clock - sched_info->last_queued
> >>> == 87029229 - 87029224 == 5"
> >>>
> >>> You think the 19046198 value is misleading and should be 5 which only
> >>> contains the waiting in queue duration, am I correct?
> >>>
> >>> I agree with Kazu's idea, that we shouldn't change the existing ps
> >>> cmd's behaviour, and runq is a better alternative for the
> >>> waiting-in-queue duration display.
> >>>
> >>> What do you think? Could you please improve your code as well as an
> >>> updated "help runq" doc for runq?
> >>>
> >>> Thanks,
> >>> Tao Liu
> >>>
> >>>>>>
> >>>>>> We can trust sched_info->last_queued as it is only set when the
> task enters the CPU run queue.
> >>>>>> Furthermore, when the task hits/runs on a CPU or dequeues the CPU
> run queue, it will be reset to 0.
> >>>>>>
> >>>>>> Therefore, my idea is simple:
> >>>>>>
> >>>>>> If a task in RU stat and sched_info->last_queued has value (!= 0),
> >>>>>> it means this task is waiting in the run queue, use "rq_clock -
> sched_info->last_queued".
> >>>>>>
> >>>>>> Otherwise, if a task in RU stat and sched_info->last_queued = 0
> >>>>>> and sched_info->last_arrival has value (it must be), it means this
> task is running on the CPU,
> >>>>>> use "rq_clock - sched_info->last_arrival".
> >>>>>>
> >>>>>> Signed-off-by: Kenneth Yin <kyin(a)redhat.com>
> >>>>>> ---
> >>>>>> defs.h | 1 +
> >>>>>> symbols.c | 2 ++
> >>>>>> task.c | 21 +++++++++++++++------
> >>>>>> 3 files changed, 18 insertions(+), 6 deletions(-)
> >>>>>>
> >>>>>> diff --git a/defs.h b/defs.h
> >>>>>> index 4cf169c..66f5ce4 100644
> >>>>>> --- a/defs.h
> >>>>>> +++ b/defs.h
> >>>>>> @@ -1787,6 +1787,7 @@ struct offset_table { /*
> stash of commonly-used offsets */
> >>>>>> long vcpu_struct_rq;
> >>>>>> long task_struct_sched_info;
> >>>>>> long sched_info_last_arrival;
> >>>>>> + long sched_info_last_queued;
> >>>>>> long page_objects;
> >>>>>> long kmem_cache_oo;
> >>>>>> long char_device_struct_cdev;
> >>>>>> diff --git a/symbols.c b/symbols.c
> >>>>>> index e30fafe..fb5035f 100644
> >>>>>> --- a/symbols.c
> >>>>>> +++ b/symbols.c
> >>>>>> @@ -9930,6 +9930,8 @@ dump_offset_table(char *spec, ulong
> makestruct)
> >>>>>> OFFSET(sched_rt_entity_run_list));
> >>>>>> fprintf(fp, " sched_info_last_arrival: %ld\n",
> >>>>>> OFFSET(sched_info_last_arrival));
> >>>>>> + fprintf(fp, " sched_info_last_queued: %ld\n",
> >>>>>> + OFFSET(sched_info_last_queued));
> >>>>>> fprintf(fp, " task_struct_thread_info: %ld\n",
> >>>>>> OFFSET(task_struct_thread_info));
> >>>>>> fprintf(fp, " task_struct_stack: %ld\n",
> >>>>>> diff --git a/task.c b/task.c
> >>>>>> index 3bafe79..f5386ac 100644
> >>>>>> --- a/task.c
> >>>>>> +++ b/task.c
> >>>>>> @@ -332,9 +332,12 @@ task_init(void)
> >>>>>> MEMBER_OFFSET_INIT(task_struct_last_run, "task_struct",
> "last_run");
> >>>>>> MEMBER_OFFSET_INIT(task_struct_timestamp,
> "task_struct", "timestamp");
> >>>>>> MEMBER_OFFSET_INIT(task_struct_sched_info,
> "task_struct", "sched_info");
> >>>>>> - if (VALID_MEMBER(task_struct_sched_info))
> >>>>>> + if (VALID_MEMBER(task_struct_sched_info)) {
> >>>>>> MEMBER_OFFSET_INIT(sched_info_last_arrival,
> >>>>>> "sched_info", "last_arrival");
> >>>>>> + MEMBER_OFFSET_INIT(sched_info_last_queued,
> >>>>>> + "sched_info", "last_queued");
> >>>>>> + }
> >>>>>> if (VALID_MEMBER(task_struct_last_run) ||
> >>>>>> VALID_MEMBER(task_struct_timestamp) ||
> >>>>>> VALID_MEMBER(sched_info_last_arrival)) {
> >>>>>> @@ -6035,7 +6038,7 @@ ulonglong
> >>>>>> task_last_run(ulong task)
> >>>>>> {
> >>>>>> ulong last_run;
> >>>>>> - ulonglong timestamp;
> >>>>>> + ulonglong timestamp,last_queued;
> >>>>>>
> >>>>>> timestamp = 0;
> >>>>>> fill_task_struct(task);
> >>>>>> @@ -6047,10 +6050,16 @@ task_last_run(ulong task)
> >>>>>> } else if (VALID_MEMBER(task_struct_timestamp))
> >>>>>> timestamp = tt->last_task_read ?
> ULONGLONG(tt->task_struct +
> >>>>>> OFFSET(task_struct_timestamp)) : 0;
> >>>>>> - else if (VALID_MEMBER(sched_info_last_arrival))
> >>>>>> - timestamp = tt->last_task_read ?
> ULONGLONG(tt->task_struct +
> >>>>>> - OFFSET(task_struct_sched_info) +
> >>>>>> - OFFSET(sched_info_last_arrival)) : 0;
> >>>>>> + else if (VALID_MEMBER(sched_info_last_queued))
> >>>>>> + last_queued = ULONGLONG(tt->task_struct +
> >>>>>> + OFFSET(task_struct_sched_info) +
> >>>>>> + OFFSET(sched_info_last_queued));
> >>>>>> + if (last_queued != 0) {
> >>>>>> + timestamp = tt->last_task_read ? last_queued :
> 0;
> >>>>>> + } else if (VALID_MEMBER(sched_info_last_arrival))
> >>>>>> + timestamp = tt->last_task_read ?
> ULONGLONG(tt->task_struct +
> >>>>>> + OFFSET(task_struct_sched_info) +
> >>>>>> + OFFSET(sched_info_last_arrival)) : 0;
> >>>>>>
> >>>>>> return timestamp;
> >>>>>> }
> >>>>> --
> >>>>> Crash-utility mailing list -- devel(a)lists.crash-utility.osci.io
> >>>>> To unsubscribe send an email to
> devel-leave(a)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
> >>>> --
> >>>> Crash-utility mailing list -- devel(a)lists.crash-utility.osci.io
> >>>> To unsubscribe send an email to
> devel-leave(a)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
> >>>
> >>
> >>
> >> --
> >> Kenneth Yin
> >> Senior Software Maintenance Engineer
> >> Customer Experience and Engagement
> >> Phone: +86-10-6533-9459
> >> Red Hat China
>
2 months, 3 weeks
Re: [PATCH v2 0/3] Update to gdb-16.2
by lijiang
On Thu, Feb 6, 2025 at 3:05 PM <devel-request(a)lists.crash-utility.osci.io>
wrote:
> Date: Thu, 6 Feb 2025 20:04:11 +1300
> From: Tao Liu <ltao(a)redhat.com>
> Subject: [Crash-utility] [PATCH v2 0/3] Update to gdb-16.2
> To: devel(a)lists.crash-utility.osci.io
> Cc: Tao Liu <ltao(a)redhat.com>
> Message-ID: <20250206070418.1038668-1-ltao(a)redhat.com>
> Content-Type: text/plain; charset="US-ASCII"; x-default=true
>
> v2 -> v1: Splite the "Fix several build failures" patchset, which is
> already merged btw, from gdb v16.2 upgrading.
>
Thank you for the update, Tao.
I will have a look next week.
Thanks
Lianbo
>
> Tao Liu (3):
> LoongArch64: Revert all previous LoongArch64 related commits
> Revert: Fix C99 compatibility issues in embedded copy of GDB
> Update to gdb-16.2
>
> Makefile | 10 +-
> README | 6 +-
> configure.c | 64 +-
> crash.8 | 2 +-
> crash_target.c | 6 +-
> defs.h | 174 +-
> diskdump.c | 24 +-
> gdb-10.2.patch | 16323 ------------------------------------------
> gdb-16.2.patch | 2254 ++++++
> gdb_interface.c | 14 +-
> help.c | 15 +-
> kernel.c | 6 +-
> lkcd_vmdump_v1.h | 2 +-
> lkcd_vmdump_v2_v3.h | 5 +-
> loongarch64.c | 1368 ----
> main.c | 3 +-
> netdump.c | 27 +-
> ramdump.c | 2 -
> symbols.c | 37 +-
> 19 files changed, 2322 insertions(+), 18020 deletions(-)
> delete mode 100644 gdb-10.2.patch
> create mode 100644 gdb-16.2.patch
> delete mode 100644 loongarch64.c
>
> --
> 2.47.0
>
2 months, 3 weeks
Re: [PATCH v2] Use CC env var to get compiler version
by lijiang
Hi, Kéléfa Sané
Thank you for the patch.
On Mon, May 26, 2025 at 5:04 PM <devel-request(a)lists.crash-utility.osci.io>
wrote:
> Date: Mon, 26 May 2025 11:03:24 +0200
> From: kelefa.sane(a)smile.fr
> Subject: [Crash-utility] [meta-oe][PATCH v2] Use CC env var to get
> compiler version
> To: devel(a)lists.crash-utility.osci.io
> Cc: Kéléfa Sané <kelefa.sane(a)smile.fr>
> Message-ID: <20250526090324.3113589-1-kelefa.sane(a)smile.fr>
> Content-Type: text/plain; charset=UTF-8
>
> From: Kéléfa Sané <kelefa.sane(a)smile.fr>
>
> The source file build_data.c generated at compilation time define a
> variable compiler_version which is obtained by calling "gcc --version"
> cmd. This call retrieve the native gcc compiler install on host build
> machine but not necessarily the compiler use to build the project (ex:
> cross compilation).
>
Good findings.
>
> The CC env variable commonly used in Makefile project define the
> compiler to use at build, so this is the appropriate way to retrieve the
> compiler version, when the CC env var is define.
>
If the CC env variable is not set, this is still a problem. We should not
expect that the CC env variable is always defined(or set).
I would suggest parsing the GDB_CONF_FLAGS to get the target gcc version,
which is visible in the configure.c
What do you think?
Thanks
Lianbo
> Signed-off-by: Kéléfa Sané <kelefa.sane(a)smile.fr>
> ---
> configure.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/configure.c b/configure.c
> index 4668c9a..4b65bd7 100644
> --- a/configure.c
> +++ b/configure.c
> @@ -1362,7 +1362,17 @@ make_build_data(char *target)
>
> fp1 = popen("date", "r");
> fp2 = popen("id", "r");
> - fp3 = popen("gcc --version", "r");
> +
> + const char *cc_env = getenv("CC");
> + if(NULL == cc_env) {
> + fp3 = popen("gcc --version", "r");
> + }
> + else {
> + char compiler_version_cmd[512];
> +
> + snprintf(compiler_version_cmd,
> sizeof(compiler_version_cmd), "%s --version", cc_env);
> + fp3 = popen(compiler_version_cmd, "r");
> + }
>
> if ((fp4 = fopen("build_data.c", "w")) == NULL) {
> perror("build_data.c");
>
>
>
>
2 months, 4 weeks
Re: [PATCH] Fix incorrect task state during exit
by lijiang
On Tue, May 13, 2025 at 1:26 AM <devel-request(a)lists.crash-utility.osci.io>
wrote:
> Date: Mon, 12 May 2025 10:22:43 +1200
> From: Tao Liu <ltao(a)redhat.com>
> Subject: [Crash-utility] Re: [PATCH] Fix incorrect task state during
> exit
> To: Stephen Brennan <stephen.s.brennan(a)oracle.com>
> Cc: devel(a)lists.crash-utility.osci.io
> Message-ID:
> <CAO7dBbWHa5x+BdiVF3y_QL-JgQhDLgEqKzaQS+YfBrL=
> jY6N_w(a)mail.gmail.com>
> Content-Type: text/plain; charset="UTF-8"
>
> Hi Stephen,
>
> Thanks for your fix and detailed explanation!
>
> On Sat, May 3, 2025 at 8:19 AM Stephen Brennan
> <stephen.s.brennan(a)oracle.com> wrote:
> >
> > task_state() assumes that exit_state is a unsigned long, when in
> > reality, it has been declared as an int since 97dc32cdb1b53 ("reduce
> > size of task_struct on 64-bit machines"), in Linux 2.6.22. So on 64-bit
> > machines, task_state() reads 8 bytes rather than 4, and gets the wrong
> > exit_state value by including the next field.
> >
> > This has gone unnoticed because directly after exit_state comes
> > exit_code, which is generally zero while the task is alive. When the
> > exit_code is set, exit_state is usually set not long after. Since
> > task_state_string() only checks whether exit_state bits are set, it
> > never notices the presence of the exit code inside of the state.
> >
> > But this leaves open a window during the process exit, when the
> > exit_code has been set (in do_exit()), but the exit_state has not (in
> > exit_notify()). In this case, crash reports a state of "??", but in
> > reality, the task is still running -- it's just running the exit()
> > system call. This race window can be long enough to be observed in core
> > dumps, for example if the mmput() takes a long time.
> >
> > This should be considered a bug. A task state of "??" or "(unknown)" is
> > frequently of concern when debugging, as it could indicate that the
> > state fields had some sort of corruption, and draw the attention of the
> > debugger. To handle it properly, record the size of exit_state, and read
> > it conditionally as a UINT or ULONG, just like the state. This ensures
> > we retain compatibility with kernel before v2.6.22. Whether that is
> > actually desirable is anybody's guess.
> >
> > Reported-by: Jeffery Yoder <jeffery.yoder(a)oracle.com>
> > Signed-off-by: Stephen Brennan <stephen.s.brennan(a)oracle.com>
> > ---
> > defs.h | 1 +
> > task.c | 11 +++++++++--
> > 2 files changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/defs.h b/defs.h
> > index 4cf169c..58362d0 100644
> > --- a/defs.h
> > +++ b/defs.h
> > @@ -2435,6 +2435,7 @@ struct size_table { /* stash of
> commonly-used sizes */
> > long prb_desc;
> > long wait_queue_entry;
> > long task_struct_state;
> > + long task_struct_exit_state;
> > long printk_safe_seq_buf_buffer;
> > long sbitmap_word;
> > long sbitmap;
>
> The patch looks good to me, except for the above code. Any newly added
>
In addition, also need to dump it, please see the "help -o".
Otherwise, for the patch: Ack.
Thanks
Lianbo
> members for size/offset_table should go to the end of the struct,
> rather than the middle of it. See the 2nd item of
> https://github.com/crash-utility/crash/wiki#writing-patches. But this
> is a slight error, I can get it corrected when merging. Other than
> that, Ack for the patch.
>
> Thanks,
> Tao Liu
>
> > diff --git a/task.c b/task.c
> > index 3bafe79..e07b479 100644
> > --- a/task.c
> > +++ b/task.c
> > @@ -306,6 +306,7 @@ task_init(void)
> > MEMBER_SIZE_INIT(task_struct_state, "task_struct",
> "__state");
> > }
> > MEMBER_OFFSET_INIT(task_struct_exit_state, "task_struct",
> "exit_state");
> > + MEMBER_SIZE_INIT(task_struct_exit_state, "task_struct",
> "exit_state");
> > MEMBER_OFFSET_INIT(task_struct_pid, "task_struct", "pid");
> > MEMBER_OFFSET_INIT(task_struct_comm, "task_struct", "comm");
> > MEMBER_OFFSET_INIT(task_struct_next_task, "task_struct",
> "next_task");
> > @@ -5965,8 +5966,14 @@ task_state(ulong task)
> > state = ULONG(tt->task_struct +
> OFFSET(task_struct_state));
> > else
> > state = UINT(tt->task_struct +
> OFFSET(task_struct_state));
> > - exit_state = VALID_MEMBER(task_struct_exit_state) ?
> > - ULONG(tt->task_struct + OFFSET(task_struct_exit_state))
> : 0;
> > +
> > + if (VALID_MEMBER(task_struct_exit_state)
> > + && SIZE(task_struct_exit_state) == sizeof(ulong))
> > + exit_state = ULONG(tt->task_struct +
> OFFSET(task_struct_exit_state));
> > + else if (VALID_MEMBER(task_struct_exit_state))
> > + exit_state = UINT(tt->task_struct +
> OFFSET(task_struct_exit_state));
> > + else
> > + exit_state = 0;
> >
> > return (state | exit_state);
> > }
> > --
>
3 months
[PATCH] vmware_guestdump: Version 7 support
by Alexey Makhalov
ESXi 9.0 updated debug.guest format. CPU architecture type was
introduced and several fields of the header not used by the crash
were moved around. It is version 7 now.
Make corresponding changes in debug.guest parser and keep it
backward compatible with older versions.
Fix comment and log messages typos as well.
Signed-off-by: Alexey Makhalov <alexey.makhalov(a)broadcom.com>
---
vmware_guestdump.c | 48 ++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 44 insertions(+), 4 deletions(-)
diff --git a/vmware_guestdump.c b/vmware_guestdump.c
index 78f37fb..1a6ef9b 100644
--- a/vmware_guestdump.c
+++ b/vmware_guestdump.c
@@ -30,6 +30,7 @@
* 2. Number of Virtual CPUs (4 bytes) } - struct guestdumpheader
* 3. Reserved gap
* 4. Main Memory information - struct mainmeminfo{,_old}
+ * 5. Reserved gap #2. Only in v7+
* (use get_vcpus_offset() to get total size of guestdumpheader)
* vcpus_offset: ---------\
* 1. struct vcpu_state1 \
@@ -111,6 +112,22 @@ struct vcpu_state2 {
uint8_t reserved3[65];
} __attribute__((packed));
+typedef enum {
+ CPU_ARCH_AARCH64,
+ CPU_ARCH_X86,
+} cpu_arch;
+
+/*
+ * Returns the size of reserved gap #2 in the header right after the Main Mem.
+ */
+static inline long
+get_gap2_size(uint32_t version)
+{
+ if (version == 7)
+ return 11;
+ return 0;
+}
+
/*
* Returns the size of the guest dump header.
*/
@@ -128,6 +145,9 @@ get_vcpus_offset(uint32_t version, int mem_holes)
return sizeof(struct guestdumpheader) + 14 + sizeof(struct mainmeminfo);
case 6: /* ESXi 8.0u2 */
return sizeof(struct guestdumpheader) + 15 + sizeof(struct mainmeminfo);
+ case 7: /* ESXi 9.0 */
+ return sizeof(struct guestdumpheader) + 8 + sizeof(struct mainmeminfo) +
+ get_gap2_size(version);
}
return 0;
@@ -155,10 +175,10 @@ get_vcpu_gapsize(uint32_t version)
*
* guestdump (debug.guest) is a simplified version of the *.vmss which does
* not contain a full VM state, but minimal guest state, such as a memory
- * layout and CPUs state, needed for debugger. is_vmware_guestdump()
+ * layout and CPUs state, needed for the debugger. is_vmware_guestdump()
* and vmware_guestdump_init() functions parse guestdump header and
* populate vmss data structure (from vmware_vmss.c). In result, all
- * handlers (except mempry_dump) from vmware_vmss.c can be reused.
+ * handlers (except memory_dump) from vmware_vmss.c can be reused.
*
* debug.guest does not have a dedicated header magic or file format signature
* To probe debug.guest we need to perform series of validations. In addition,
@@ -225,7 +245,8 @@ is_vmware_guestdump(char *filename)
/* vcpu_offset adjustment for mem_holes is required only for version 1. */
vcpus_offset = get_vcpus_offset(hdr.version, mmi.mem_holes);
} else {
- if (fseek(fp, vcpus_offset - sizeof(struct mainmeminfo), SEEK_SET) == -1) {
+ if (fseek(fp, vcpus_offset - sizeof(struct mainmeminfo) - get_gap2_size(hdr.version),
+ SEEK_SET) == -1) {
if (CRASHDEBUG(1))
error(INFO, LOGPRX"Failed to fseek '%s': [Error %d] %s\n",
filename, errno, strerror(errno));
@@ -240,6 +261,25 @@ is_vmware_guestdump(char *filename)
fclose(fp);
return FALSE;
}
+
+ /* Check CPU architecture field. Next 4 bytes after the Main Mem */
+ if (hdr.version >= 7) {
+ cpu_arch arch;
+ if (fread(&arch, sizeof(cpu_arch), 1, fp) != 1) {
+ if (CRASHDEBUG(1))
+ error(INFO, LOGPRX"Failed to read '%s' from file '%s': [Error %d] %s\n",
+ "CPU arch", filename, errno, strerror(errno));
+ fclose(fp);
+ return FALSE;
+ }
+ if (arch != CPU_ARCH_X86) {
+ if (CRASHDEBUG(1))
+ error(INFO,
+ LOGPRX"Invalid or unsupported CPU architecture: %d\n", arch);
+ fclose(fp);
+ return FALSE;
+ }
+ }
}
if (fseek(fp, 0L, SEEK_END) == -1) {
if (CRASHDEBUG(1))
@@ -300,7 +340,7 @@ vmware_guestdump_init(char *filename, FILE *ofp)
if (!machine_type("X86") && !machine_type("X86_64")) {
error(INFO,
- LOGPRX"Invalid or unsupported host architecture for .vmss file: %s\n",
+ LOGPRX"Invalid or unsupported host architecture for .guest file: %s\n",
MACHINE_TYPE);
result = FALSE;
goto exit;
--
2.43.5
3 months
