Re: [PATCH v2] kmem: fix the determination for slab page
by lijiang
On Thu, Sep 19, 2024 at 10:43 AM <devel-request(a)lists.crash-utility.osci.io>
wrote:
> Date: Thu, 19 Sep 2024 01:23:05 -0000
> From: qiwu.chen(a)transsion.com
> Subject: [Crash-utility] Re: [PATCH v2] kmem: fix the determination
> for slab page
> To: devel(a)lists.crash-utility.osci.io
> Message-ID: <20240919012305.29184.37668(a)lists.crash-utility.osci.io>
> Content-Type: text/plain; charset="utf-8"
>
> Hi Lianbo,
> Thanks for your review!
> >
> > Can you try to dump the values of the above two variables from
> > the dump_vm_table() and dump_offset_table()? We can display their values
> by
> > help -v and help -o options.
> >
> Sure, I will post patch v3 to display the two variables.
> >
> >
> > When initializing the page_page_type, the above "if" check seems to be
> > redundant.
> >
> Okay, it should be removed.
> >
Thanks.
>
> >
> > Can this page_type_init() be moved to the page_flags_init()?
> >
> I don't think so, because the page type is an independent field of struct
> page which has different usage compared with page flags. What do other
> reviewers think?
>
Or can it be placed after the page_flags_init()? Just like this:
diff --git a/memory.c b/memory.c
index 967a9cf58959..9340a8366b98 100644
--- a/memory.c
+++ b/memory.c
@@ -1278,6 +1278,8 @@ vm_init(void)
page_flags_init();
+ page_type_init();
+
rss_page_types_init();
vt->flags |= VM_INIT;
Thanks
Lianbo
>
> Thanks
> Qiwu
>
5 hours, 10 minutes
Re: [PATCH v1 2/3] ppc64: check sp at the start of stack back trace
by lijiang
On Wed, Sep 18, 2024 at 3:37 PM <devel-request(a)lists.crash-utility.osci.io>
wrote:
> Date: Wed, 18 Sep 2024 11:42:04 +1200
> From: Tao Liu <ltao(a)redhat.com>
> Subject: [Crash-utility] [PATCH v1 2/3] ppc64: check sp at the start
> of stack back trace
> To: devel(a)lists.crash-utility.osci.io
> Cc: adityag(a)linux.ibm.com, Tao Liu <ltao(a)redhat.com>
> Message-ID: <20240917234205.7783-3-ltao(a)redhat.com>
> Content-Type: text/plain; charset="US-ASCII"; x-default=true
>
> Previously the newsp read from stack is unchecked, it would expect
> the newsp hold a valid sp pointer of the next stack frame by default,
> which is not always true, see the following stack:
>
I guess it might be related to the Power Arch 64-Bit ELF V2 ABI changes,
please refer to this link(see the section: The Stack Frame):
[1]
https://ftp.rtems.org/pub/rtems/people/sebh/ABI64BitOpenPOWERv1.1_16July2...
[2] https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi.html#STACK
[3] https://openpowerfoundation.org/specifications/64bitelfabi/
Before that It was always correct, but It might not be correct after ABI
changes. I copied it here with a minor change, maybe it is not very
accurate.
High Address
+-> Back chain
| Floating point register save area
| General register save area
| VRSAVE save word (32-bits)
| Alignment padding (4 or 12 bytes)
| Vector register save area (quadword aligned)
| Local variable space
| Parameter save area (SP + 48)
| TOC save area (SP + 40)
| link editor doubleword (SP + 32)
| compiler doubleword (SP + 24)
| LR save area (SP + 16)
| CR save area (SP + 8)
SP ---> +-- Back chain (SP + 0)
Low Address
|
|
\/
High Address
+-> Back chain
| Floating point register save area
| General register save area
| Alignment padding (4 or 12 bytes)
| Vector register save area (quadword aligned)
| Local variable space
| Parameter save area (SP + 32)
| TOC Pointer Doubleword (SP + 24)
| LR save area (SP + 16)
| Reserved (SP + 12)
| CR save area (SP + 8)
SP ---> +-- Back chain (SP + 0)
Low Address
> R1: c00000000a327660 NIP: c0000000002b9984
> #0 [c00000000a327660] crash_setup_regs at c0000000002b9984
> c00000000a327660: c00000000015a7b0 c000000001bc8600
> c00000000a327670: 0000000000000000 c000000002e13b54
> c00000000a327680: c00000000a327850 0000000000004000
> c00000000a327690: c0000000002ba078 c0000000026f2ca8
>
> The newsp will be c00000000015a7b0, which is not a valid stack pointer
> and as a result it will fail the stack back trace afterwards. This usually
> happens at the beginning of back trace, once we get the correct sp value,
> the stack can be back traced successfully.
>
> This patch will search and check for the valid newsp at the beginning of
> stack back trace. For the above example, a valid newsp would be assigned
> as c00000000a327850.
>
> Before:
> crash> bt
> ...
> #0 [c00000000a327660] crash_setup_regs at c0000000002b9984
> After:
> crash> bt
> ...
> #0 [c00000000a327660] crash_setup_regs at c0000000002b9984
> #1 [c00000000a327850] panic at c000000000167de4
> #2 [c00000000a3278f0] sysrq_handle_crash at c000000000a4d3a8
> #3 [c00000000a327950] __handle_sysrq at c000000000a4dec4
> #4 [c00000000a3279f0] write_sysrq_trigger at c000000000a4e984
> ...
>
> Signed-off-by: Tao Liu <ltao(a)redhat.com>
> ---
> ppc64.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/ppc64.c b/ppc64.c
> index 6e5f155..8af7b8a 100644
> --- a/ppc64.c
> +++ b/ppc64.c
> @@ -2148,6 +2148,19 @@ ppc64_back_trace(struct gnu_request *req, struct
> bt_info *bt)
>
> while (INSTACK(req->sp, bt)) {
> newsp = *(ulong *)&bt->stackbuf[req->sp - bt->stackbase];
> +
> + if (!newpc) {
> + ulong *up;
> + int i;
> + for (i = 0, up = (ulong *)&bt->stackbuf[req->sp -
> bt->stackbase];
> + i < (req->sp - bt->stackbase) /
> sizeof(ulong); i++, up++) {
> + if (INSTACK(*up, bt) &&
> is_kernel_text(*(up + 2))) {
> + newsp = *up;
> + break;
> + }
> + }
> + }
> +
>
Can we decide what to do based on whether the ELF ABI V2 is enabled or not?
Because the Stack Frame Organization won't be changed in the spec v2. Just
an idea.
Thanks
Lianbo
if ((req->name = closest_symbol(req->pc)) == NULL) {
> if (CRASHDEBUG(1)) {
> error(FATAL,
> --
> 2.40.1
>
7 hours, 12 minutes
Re: [PATCH v1 1/3] ppc64: Fix bt printing error stack trace
by lijiang
Hi, Tao
Thank you for the fix.
On Wed, Sep 18, 2024 at 7:44 AM <devel-request(a)lists.crash-utility.osci.io>
wrote:
> Date: Wed, 18 Sep 2024 11:42:03 +1200
> From: Tao Liu <ltao(a)redhat.com>
> Subject: [Crash-utility] [PATCH v1 1/3] ppc64: Fix bt printing error
> stack trace
> To: devel(a)lists.crash-utility.osci.io
> Cc: adityag(a)linux.ibm.com, Tao Liu <ltao(a)redhat.com>
> Message-ID: <20240917234205.7783-2-ltao(a)redhat.com>
> Content-Type: text/plain; charset="US-ASCII"; x-default=true
>
> A error stack trace of bt cmd observed:
>
> crash> bt 1
> PID: 1 TASK: c000000003714b80 CPU: 2 COMMAND: "systemd"
> #0 [c0000000037735c0] _end at c0000000037154b0 (unreliable)
> #1 [c000000003773770] __switch_to at c00000000001fa9c
> #2 [c0000000037737d0] __schedule at c00000000112e4ec
> #3 [c0000000037738b0] schedule at c00000000112ea80
> ...
>
> The #0 stack trace is incorrect, the function address shouldn't exceed
> _end.
> The reason is for kernel>=v6.2, the offset of pt_regs to sp changed from
> STACK_FRAME_OVERHEAD, i.e 112, to STACK_SWITCH_FRAME_REGS. For
> CONFIG_PPC64_ELF_ABI_V1, it's 112, for ABI_V2, it's 48. So the nip will
> read a
> wrong value from stack when ABI_V2 enabled.
>
>
Can you help to add the related kernel commits to patch log? That will help
me a lot to review the patches.
> To determine if ABI_V2 enabled is tricky. This patch do it by check the
> following:
>
Can you try to read the value of e_flags from the elf header and to
determine what ABI version it is?
Thanks
Lianbo
>
> In arch/powerpc/include/asm/ppc_asm.h:
> #ifdef CONFIG_PPC64_ELF_ABI_V2
> #define STK_GOT 24
> #else
> #define STK_GOT 40
>
> In arch/powerpc/kernel/tm.S:
> _GLOBAL(tm_reclaim)
> mfcr r5
> mflr r0
> stw r5, 8(r1)
> std r0, 16(r1)
> std r2, STK_GOT(r1)
> ...
>
> So a disassemble on tm_reclaim, and extract the STK_GOT value from std
> instruction is used as the approach.
>
> After the patch:
> crash> bt 1
> PID: 1 TASK: c000000003714b80 CPU: 2 COMMAND: "systemd"
> #0 [c0000000037737d0] __schedule at c00000000112e4ec
> #1 [c0000000037738b0] schedule at c00000000112ea80
> ...
>
> Signed-off-by: Tao Liu <ltao(a)redhat.com>
> ---
> defs.h | 1 +
> ppc64.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 58 insertions(+), 3 deletions(-)
>
> diff --git a/defs.h b/defs.h
> index 2231cb6..d5cb8cc 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -4643,6 +4643,7 @@ struct efi_memory_desc_t {
> #define MSR_PR_LG 14 /* Problem State / Privilege Level */
> /* Used to find the user or kernel-mode
> frame*/
>
> +#define STACK_SWITCH_FRAME_REGS 48
> #define STACK_FRAME_OVERHEAD 112
> #define EXCP_FRAME_MARKER 0x7265677368657265
>
> diff --git a/ppc64.c b/ppc64.c
> index e8930a1..6e5f155 100644
> --- a/ppc64.c
> +++ b/ppc64.c
> @@ -72,6 +72,7 @@ static ulong pud_page_vaddr_l4(ulong pud);
> static ulong pmd_page_vaddr_l4(ulong pmd);
> static int is_opal_context(ulong sp, ulong nip);
> void opalmsg(void);
> +static bool is_ppc64_elf_abi_v2(void);
>
> static int is_opal_context(ulong sp, ulong nip)
> {
> @@ -2813,6 +2814,51 @@ ppc64_get_sp(ulong task)
> return sp;
> }
>
> +static bool
> +is_ppc64_elf_abi_v2(void)
> +{
> + char buf1[BUFSIZE];
> + char *pos1, *pos2;
> + int errflag = 0;
> + ulong stk_got = 0;
> + static bool ret = false;
> + static bool checked = false;
> +
> + if (checked == true || !symbol_exists("tm_reclaim"))
> + return ret;
> +
> + sprintf(buf1, "x/16i tm_reclaim");
> + open_tmpfile();
> + if (!gdb_pass_through(buf1, pc->tmpfile, GNU_RETURN_ON_ERROR))
> + goto out;
> + checked = true;
> + rewind(pc->tmpfile);
> + while (fgets(buf1, BUFSIZE, pc->tmpfile)) {
> + // "std r2, STK_GOT(r1)" is expected
> + if (strstr(buf1, "std") &&
> + strstr(buf1, "(r1)") &&
> + (pos1 = strstr(buf1, "r2,"))) {
> + pos1 += strlen("r2,");
> + for (pos2 = pos1; *pos2 != '\0' && *pos2 != '(';
> pos2++);
> + *pos2 = '\0';
> + stk_got = stol(pos1, RETURN_ON_ERROR|QUIET,
> &errflag);
> + break;
> + }
> + }
> +
> + if (!errflag) {
> + switch (stk_got) {
> + case 24:
> + ret = true;
> + case 40:
> + goto out;
> + }
> + }
> + error(WARNING, "Unstable elf_abi v1/v2 detection.\n");
> +out:
> + close_tmpfile();
> + return ret;
> +}
>
> /*
> * get the SP and PC values for idle tasks.
> @@ -2834,9 +2880,17 @@ get_ppc64_frame(struct bt_info *bt, ulong *getpc,
> ulong *getsp)
> sp = ppc64_get_sp(task);
> if (!INSTACK(sp, bt))
> goto out;
> - readmem(sp+STACK_FRAME_OVERHEAD, KVADDR, ®s,
> - sizeof(struct ppc64_pt_regs),
> - "PPC64 pt_regs", FAULT_ON_ERROR);
> +
> + if (THIS_KERNEL_VERSION >= LINUX(6,2,0) && is_ppc64_elf_abi_v2()) {
> + readmem(sp+STACK_SWITCH_FRAME_REGS, KVADDR, ®s,
> + sizeof(struct ppc64_pt_regs),
> + "PPC64 pt_regs", FAULT_ON_ERROR);
> + } else {
> + readmem(sp+STACK_FRAME_OVERHEAD, KVADDR, ®s,
> + sizeof(struct ppc64_pt_regs),
> + "PPC64 pt_regs", FAULT_ON_ERROR);
> + }
> +
> ip = regs.nip;
> closest = closest_symbol(ip);
> if (STREQ(closest, ".__switch_to") || STREQ(closest,
> "__switch_to")) {
> --
> 2.40.1
>
12 hours, 27 minutes
Re: [PATCH v1 3/3] ppc64: fix the bug eframe won't print for newer kernel
by lijiang
On Wed, Sep 18, 2024 at 3:37 PM <devel-request(a)lists.crash-utility.osci.io>
wrote:
> Date: Wed, 18 Sep 2024 11:42:05 +1200
> From: Tao Liu <ltao(a)redhat.com>
> Subject: [Crash-utility] [PATCH v1 3/3] ppc64: fix the bug eframe
> won't print for newer kernel
> To: devel(a)lists.crash-utility.osci.io
> Cc: adityag(a)linux.ibm.com, Tao Liu <ltao(a)redhat.com>
> Message-ID: <20240917234205.7783-4-ltao(a)redhat.com>
> Content-Type: text/plain; charset="US-ASCII"; x-default=true
>
> The value of EXCP_FRAME_MARKER is outdated for kernel>=6.1.0, and also
> its offset is changed since kernel>=6.2.0. This patch will fix the issue
>
Can you help to add the related kernel commits to the patch log?
Otherwise I have to find it and check why the crash tool needs to be
changed.
by updating its values.
>
> Before:
> crash> bt
> ...
> #8 [c00000000a327b80] system_call_exception at c000000000031a30
> #9 [c00000000a327e50] system_call_vectored_common at c00000000000d05c
>
> After:
> crash> bt
> ...
> #8 [c00000000a327b80] system_call_exception at c000000000031a30
> #9 [c00000000a327e50] system_call_vectored_common at c00000000000d05c
> System Call Vectored [3000] exception frame:
> R0: 0000000000000004 R1: 00007fffc3345f40 R2: 0000000000100000
> R3: 0000000000000001 R4: 000000012572c620 R5: 0000000000000002
> R6: 0000000000000001 R7: 0000000000000004 R8: 0000000000000001
> ...
>
> Signed-off-by: Tao Liu <ltao(a)redhat.com>
> ---
> defs.h | 5 ++++-
> ppc64.c | 37 +++++++++++++++++++++++++++----------
> 2 files changed, 31 insertions(+), 11 deletions(-)
>
> diff --git a/defs.h b/defs.h
> index d5cb8cc..ceeb6cc 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -4645,7 +4645,10 @@ struct efi_memory_desc_t {
>
> #define STACK_SWITCH_FRAME_REGS 48
> #define STACK_FRAME_OVERHEAD 112
> -#define EXCP_FRAME_MARKER 0x7265677368657265
> +#define EXCP_FRAME_MARKER \
> + (THIS_KERNEL_VERSION >= LINUX(6,1,0) ? \
> + (__BYTE_ORDER == __BIG_ENDIAN ? 0x52454753 : 0x53474552) : \
> + 0x7265677368657265)
>
>
Here, a code comment is needed, where does the magic number come from?
#define _SECTION_SIZE_BITS 24
> #define _MAX_PHYSMEM_BITS 44
> diff --git a/ppc64.c b/ppc64.c
> index 8af7b8a..5416281 100644
> --- a/ppc64.c
> +++ b/ppc64.c
> @@ -2217,19 +2217,34 @@ ppc64_back_trace(struct gnu_request *req, struct
> bt_info *bt)
> eframe_found = TRUE;
> else if (STREQ(req->name, ".ret_from_except"))
> eframe_found = TRUE;
> - } else if ((newsp - req->sp - STACK_FRAME_OVERHEAD) >=
> - sizeof(struct ppc64_pt_regs)) {
> - readmem(req->sp+0x60, KVADDR, &marker,
> - sizeof(ulong), "stack frame",
> FAULT_ON_ERROR);
> - if (marker == EXCP_FRAME_MARKER)
> - eframe_found = TRUE;
> + } else if (THIS_KERNEL_VERSION >= LINUX(6,2,0) &&
> is_ppc64_elf_abi_v2()) {
>
Looks like there is no better way? Can you help to describe the details?
Thanks
Lianbo
+ if ((newsp - req->sp - STACK_SWITCH_FRAME_REGS) >=
> + sizeof(struct ppc64_pt_regs)) {
> + readmem(req->sp+0x20, KVADDR, &marker,
> + sizeof(ulong), "stack frame",
> + FAULT_ON_ERROR);
> + }
> + } else {
> + if ((newsp - req->sp - STACK_FRAME_OVERHEAD) >=
> + sizeof(struct ppc64_pt_regs)) {
> + readmem(req->sp+0x60, KVADDR, &marker,
> + sizeof(ulong), "stack frame",
> + FAULT_ON_ERROR);
> + }
> }
> - if (eframe_found) {
> +
> + if (eframe_found || marker == EXCP_FRAME_MARKER) {
> char *efrm_str = NULL;
> struct ppc64_pt_regs regs;
> - readmem(req->sp+STACK_FRAME_OVERHEAD, KVADDR,
> ®s,
> - sizeof(struct ppc64_pt_regs),
> - "exception frame", FAULT_ON_ERROR);
> + if (THIS_KERNEL_VERSION >= LINUX(6,2,0) &&
> is_ppc64_elf_abi_v2()) {
> + readmem(req->sp+STACK_SWITCH_FRAME_REGS,
> KVADDR, ®s,
> + sizeof(struct ppc64_pt_regs),
> + "exception frame", FAULT_ON_ERROR);
> + } else {
> + readmem(req->sp+STACK_FRAME_OVERHEAD,
> KVADDR, ®s,
> + sizeof(struct ppc64_pt_regs),
> + "exception frame", FAULT_ON_ERROR);
> + }
>
> efrm_str = ppc64_check_eframe(®s);
> if (efrm_str) {
> @@ -2440,6 +2455,8 @@ ppc64_check_eframe(struct ppc64_pt_regs *regs)
> return("Denormalisation");
> case 0x1700:
> return("Altivec Assist");
> + case 0x3000:
> + return("System Call Vectored");
> }
>
> /* No exception frame exists */
> --
> 2.40.1
>
12 hours, 57 minutes
[PATCH v7 00/15] gdb stack unwinding support for crash utility
by Tao Liu
This patchset is a rebase/merged version of the following 3 patchsets:
1): [PATCH v10 0/5] Improve stack unwind on ppc64 [1]
2): [PATCH 0/5] x86_64 gdb stack unwinding support [2]
3): Clean up on top of one-thread-v2 [3]
A complete description of gdb stack unwinding support for crash can be
found in [1].
This patchset can be divided into the following 3 parts:
1) part1: preparations before stack unwinding support, some
bugs/regressions found when drafting this patchset.
2) part2: common part for all CPU archs, mainly dealing with
crash_target.c/gdb_interface.c files, in order to
support different archs.
3) part3: arch specific, for each ppc64/x86_64/arm64/vmware
stack unwinding support.
=== part 3
arm64: Add gdb stack unwinding support
vmware_guestdump: Various format versions support
x86_64: Add gdb stack unwinding support
ppc64: correct gdb passthroughs by implementing machdep->get_current_task_reg
=== part 2
Conditionally output gdb stack unwinding stop reasons
Stop stack unwinding at non-kernel address
Print task pid/command instead of CPU index
Rename get_cpu_reg to get_current_task_reg
Let crash change gdb context
Leave only one gdb thread for crash
Remove 'frame' from prohibited commands list
=== part 1
Fix gdb_interface: restore gdb's output streams at end of gdb_interface
x86_64: Fix invalid input "=>" for bt command
Fix cpumask_t recursive dependence issue
Fix the regression of cpumask_t for xen hyper
===
v7 -> v6:
1) Reorganise the patchset, re-divided them into 3 part against the
previous 2 parts.
2) Re-dealed with the cpumask_t part, which solved the comment No.4
pointed out by lianbo in [4].
3) Add conditional output for the failing message of gdb stack unwinding.
see [PATCH 11/15] Conditionally output gdb stack unwinding stop reasons
4) Redraft the commit messages, updated some outdated info.
5) Merged "Let crash change gdb context" and "set_context(): check if
context is already current" into one.
[4]: https://www.mail-archive.com/devel@lists.crash-utility.osci.io/msg01067.html
v6 -> v5:
1) Refactor patch 4 & 9, which changed the function signature of struct
get_cpu_reg/get_current_task_reg, and let each patch compile with no
error when added on.
2) Rebased the patchset on top of latest upstream:
("79b93ecb2e72ec Fix a "Bus error" issue caused by 'crash --osrelease' or
crash loading")
v5 -> v4:
1) Plenty of code refactoring based on Lianbo's comments on v4.
2) Removed the magic number when dealing with regs bitmap, see [6].
3) Rebased the patchset on top of latest upstream:
("1c6da3eaff8207 arm64: Fix bt command show wrong stacktrace on ramdump source")
v4 -> v3:
Fixed the author issue in [PATCH v3 06/16] Fix gdb_interface: restore gdb's
output streams at end of gdb_interface.
v3 -> v2:
1) Updated CC list as pointed out in [4]
2) Compiling issues as in [5]
v2 -> v1:
1) Added the patch: x86_64: Fix invalid input "=>" for bt command,
thanks for Kazu's testing.
2) Modify the patch: x86_64: Add gdb stack unwinding support, added the
pcp_save, spp_save and sp, for restoring the value in match of the original
code logic.
[1]: https://www.mail-archive.com/devel@lists.crash-utility.osci.io/msg00469.html
[2]: https://www.mail-archive.com/devel@lists.crash-utility.osci.io/msg00488.html
[3]: https://www.mail-archive.com/devel@lists.crash-utility.osci.io/msg00554.html
[4]: https://www.mail-archive.com/devel@lists.crash-utility.osci.io/msg00681.html
[5]: https://www.mail-archive.com/devel@lists.crash-utility.osci.io/msg00715.html
[6]: https://www.mail-archive.com/devel@lists.crash-utility.osci.io/msg00819.html
Aditya Gupta (3):
Fix gdb_interface: restore gdb's output streams at end of
gdb_interface
Remove 'frame' from prohibited commands list
ppc64: correct gdb passthroughs by implementing
machdep->get_current_task_reg
Alexey Makhalov (1):
vmware_guestdump: Various format versions support
Tao Liu (11):
Fix the regression of cpumask_t for xen hyper
Fix cpumask_t recursive dependence issue
x86_64: Fix invalid input "=>" for bt command
Leave only one gdb thread for crash
Let crash change gdb context
Rename get_cpu_reg to get_current_task_reg
Print task pid/command instead of CPU index
Stop stack unwinding at non-kernel address
Conditionally output gdb stack unwinding stop reasons
x86_64: Add gdb stack unwinding support
arm64: Add gdb stack unwinding support
arm64.c | 120 +++++++++++++++--
crash_target.c | 71 ++++++----
defs.h | 194 ++++++++++++++++++++++++++-
gdb-10.2.patch | 96 ++++++++++++++
gdb_interface.c | 39 ++----
kernel.c | 63 +++++++--
ppc64.c | 174 +++++++++++++++++++++++-
symbols.c | 15 +++
task.c | 34 +++--
tools.c | 16 ++-
unwind_x86_64.h | 4 -
vmware_guestdump.c | 321 +++++++++++++++++++++++++++++++-------------
x86_64.c | 323 ++++++++++++++++++++++++++++++++++++++++-----
13 files changed, 1247 insertions(+), 223 deletions(-)
--
2.40.1
13 hours, 13 minutes
[PATCH v2] Fix the regression of cpumask_t for xen hyper
by Tao Liu
There is a regression been found for xen hyper due to the commit:
f615f8fab7bf ("Fix "irq -a" exceeding the memory range issue").
The reason is for xen hyper, kt->cpu is not initialized due to
kernel_init() won't be called. So 0 would be assigned to cpulen and
fails the GETBUF().
Before:
crash> bt -c 2
bt: zero-size memory allocation! (called from 51f168)
After:
crash> bt -c 2
PCPU: 0 VCPU: ffff8300001b8080
#0 [ffff8300001bfe00] machine_crash_kexec at ffff83000010de72
#1 [ffff8300001bfe10] do_kexec_op at ffff83000010e3cb
#2 [ffff8300001bfe50] do_console_io at ffff83000011aff4
#3 [ffff8300001bfe90] mod_l1_entry at ffff830000129045
#4 [ffff8300001bfea0] toggle_guest_mode at ffff8300001641bf
#5 [ffff8300001bfeb0] do_iret at ffff830000164888
#6 [ffff8300001bff20] syscall_enter at ffff8300001633d2
Since xen hyper will initialize its own cpumask_t, this patch will reuse
it for XEN_HYPER_MODE.
Signed-off-by: Tao Liu <ltao(a)redhat.com>
---
Link to v1: https://www.mail-archive.com/devel@lists.crash-utility.osci.io/msg01080.html
v1 -> v2: 1) Remove "xen_hyper_defs.h" include for tools.c.
2) Modified the same code hunk in generic_get_irq_affinity().
Note this patch will cause a small merge conflict with the NO.2 patch[1] of the
v7 gdb stack unwinding support. Should be easy to fix since it has no
functional modification on [1].
[1]: https://www.mail-archive.com/devel@lists.crash-utility.osci.io/msg01081.html
---
defs.h | 3 +++
kernel.c | 10 ++++++----
tools.c | 3 +++
xen_hyper.c | 9 +++++++++
4 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/defs.h b/defs.h
index dfbd241..94f86e6 100644
--- a/defs.h
+++ b/defs.h
@@ -8031,6 +8031,9 @@ extern int have_full_symbols(void);
#if defined(X86) || defined(X86_64) || defined(IA64)
#define XEN_HYPERVISOR_ARCH
+long xen_get_cpumask_size(void);
+#else
+#define xen_get_cpumask_size() -1
#endif
/*
diff --git a/kernel.c b/kernel.c
index adb19ad..56deb3a 100644
--- a/kernel.c
+++ b/kernel.c
@@ -7382,10 +7382,12 @@ generic_get_irq_affinity(int irq)
if (!action)
return;
- len = DIV_ROUND_UP(kt->cpus, BITS_PER_LONG) * sizeof(ulong);
- len_cpumask = STRUCT_SIZE("cpumask_t");
- if (len_cpumask > 0)
- len = len_cpumask > len ? len : len_cpumask;
+ if (!XEN_HYPER_MODE() || (len = xen_get_cpumask_size()) < 0) {
+ len = DIV_ROUND_UP(kt->cpus, BITS_PER_LONG) * sizeof(ulong);
+ len_cpumask = STRUCT_SIZE("cpumask_t");
+ if (len_cpumask > 0)
+ len = len_cpumask > len ? len : len_cpumask;
+ }
affinity = (ulong *)GETBUF(len);
if (VALID_MEMBER(irq_common_data_affinity))
diff --git a/tools.c b/tools.c
index 2b78b95..c4fe9c8 100644
--- a/tools.c
+++ b/tools.c
@@ -6720,6 +6720,9 @@ get_cpumask_buf(void)
{
int cpulen, len_cpumask;
+ if (XEN_HYPER_MODE() && (cpulen = xen_get_cpumask_size()) >= 0)
+ return (ulong *)GETBUF(cpulen);
+
cpulen = DIV_ROUND_UP(kt->cpus, BITS_PER_LONG) * sizeof(ulong);
len_cpumask = STRUCT_SIZE("cpumask_t");
if (len_cpumask > 0)
diff --git a/xen_hyper.c b/xen_hyper.c
index 32e56fa..db2dd6f 100644
--- a/xen_hyper.c
+++ b/xen_hyper.c
@@ -2201,4 +2201,13 @@ xen_hyper_print_bt_header(FILE *out, ulong vcpu, int newline)
error(FATAL, "invalid vcpu\n");
fprintf(out, "PCPU: %2d VCPU: %lx\n", vcc->processor, vcpu);
}
+
+long
+xen_get_cpumask_size(void)
+{
+ if (XEN_HYPER_VALID_SIZE(cpumask_t))
+ return XEN_HYPER_SIZE(cpumask_t);
+ else
+ return -1;
+}
#endif
--
2.40.1
13 hours, 23 minutes
Re: [PATCH v2] kmem: fix the determination for slab page
by lijiang
Thank you for the update, Qiwu.
On Wed, Sep 11, 2024 at 10:27 AM <devel-request(a)lists.crash-utility.osci.io>
wrote:
> Date: Wed, 11 Sep 2024 02:25:40 -0000
> From: qiwu.chen(a)transsion.com
> Subject: [Crash-utility] [PATCH v2] kmem: fix the determination for
> slab page
> To: devel(a)lists.crash-utility.osci.io
> Message-ID: <20240911022540.15869.84683(a)lists.crash-utility.osci.io>
> Content-Type: text/plain; charset="utf-8"
>
> The determination for a slab page has changed due to changing
> PG_slab from a page flag to a page type since kernel commit
> 46df8e73a4a3.
>
> Before apply this patch:
> crash> kmem -s ffff000002aa4100
> kmem: address is not allocated in slab subsystem: ffff000002aa4100
>
> After apply this patch:
> crash> kmem -s ffff000002aa4100
> CACHE OBJSIZE ALLOCATED TOTAL SLABS SSIZE NAME
> ffff00000140f900 4096 94 126 18 32k task_struct
> SLAB MEMORY NODE TOTAL ALLOCATED FREE
> fffffdffc00aa800 ffff000002aa0000 0 7 5 2
> FREE / [ALLOCATED]
> [ffff000002aa4100]
>
> Signed-off-by: qiwu.chen <qiwu.chen(a)transsion.com>
> ---
> defs.h | 7 ++++++
> memory.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++---------
> 2 files changed, 62 insertions(+), 10 deletions(-)
>
> diff --git a/defs.h b/defs.h
> index 2231cb6..e2a9278 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -2243,6 +2243,7 @@ struct offset_table { /* stash of
> commonly-used offsets */
> long vmap_node_busy;
> long rb_list_head;
> long file_f_inode;
> + long page_page_type;
> };
>
> struct size_table { /* stash of commonly-used sizes */
> @@ -2651,6 +2652,7 @@ struct vm_table { /* kernel
> VM-related data */
> ulong max_mem_section_nr;
> ulong zero_paddr;
> ulong huge_zero_paddr;
> + uint page_type_base;
> };
>
>
Can you try to dump the values of the above two variables from
the dump_vm_table() and dump_offset_table()? We can display their values by
help -v and help -o options.
> #define NODES (0x1)
> @@ -2684,6 +2686,11 @@ struct vm_table { /* kernel
> VM-related data */
> #define SLAB_CPU_CACHE (0x10000000)
> #define SLAB_ROOT_CACHES (0x20000000)
> #define USE_VMAP_NODES (0x40000000)
> +/*
> + * The SLAB_PAGEFLAGS flag is introduced to detect the change of
> + * PG_slab's type from a page flag to a page type.
> + */
> +#define SLAB_PAGEFLAGS (0x80000000)
>
> #define IS_FLATMEM() (vt->flags & FLATMEM)
> #define IS_DISCONTIGMEM() (vt->flags & DISCONTIGMEM)
> diff --git a/memory.c b/memory.c
> index 967a9cf..48ac627 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -351,6 +351,43 @@ static ulong handle_each_vm_area(struct
> handle_each_vm_area_args *);
>
> static ulong DISPLAY_DEFAULT;
>
> +/*
> + * Before kernel commit ff202303c398e, the value is defined as a macro,
> so copy it here;
> + * After this commit, the value is defined as an enum, which can be
> evaluated at runtime.
> + */
> +#define PAGE_TYPE_BASE 0xf0000000
> +#define PageType(page_type, flag)
> \
> + ((page_type & (vt->page_type_base | flag)) == vt->page_type_base)
> +
> +static void page_type_init(void)
> +{
> + if (!enumerator_value("PAGE_TYPE_BASE", (long
> *)&vt->page_type_base))
> + vt->page_type_base = PAGE_TYPE_BASE;
> +}
> +
> +/*
> + * The PG_slab's type has changed from a page flag to a page type
> + * since kernel commit 46df8e73a4a3.
> + */
> +static bool page_slab(ulong page, ulong flags)
> +{
> + if (vt->flags & SLAB_PAGEFLAGS) {
> + if ((flags >> vt->PG_slab) & 1)
> + return TRUE;
> + }
> +
> + if (VALID_MEMBER(page_page_type)) {
> + uint page_type;
> +
> + readmem(page+OFFSET(page_page_type), KVADDR, &page_type,
> + sizeof(page_type), "page_type", FAULT_ON_ERROR);
> + if (PageType(page_type, (uint)vt->PG_slab))
> + return TRUE;
> + }
> +
> + return FALSE;
> +}
> +
> /*
> * Verify that the sizeof the primitive types are reasonable.
> */
> @@ -504,6 +541,10 @@ vm_init(void)
> ANON_MEMBER_OFFSET_INIT(page_compound_head, "page",
> "compound_head");
> MEMBER_OFFSET_INIT(page_private, "page", "private");
> MEMBER_OFFSET_INIT(page_freelist, "page", "freelist");
> + if (MEMBER_EXISTS("page", "page_type")) {
>
When initializing the page_page_type, the above "if" check seems to be
redundant.
> + MEMBER_OFFSET_INIT(page_page_type, "page", "page_type");
> + page_type_init();
>
Can this page_type_init() be moved to the page_flags_init()?
Other changes are fine to me.
Thanks
Lianbo
+ }
>
>
MEMBER_OFFSET_INIT(mm_struct_pgd, "mm_struct", "pgd");
>
> @@ -5931,7 +5972,7 @@ dump_mem_map_SPARSEMEM(struct meminfo *mi)
> if ((flags >> v22_PG_Slab) & 1)
> slabs++;
> } else if (vt->PG_slab) {
> - if ((flags >> vt->PG_slab) & 1)
> + if (page_slab(pp, flags))
> slabs++;
> } else {
> if ((flags >> v24_PG_slab) & 1)
> @@ -6381,7 +6422,7 @@ dump_mem_map(struct meminfo *mi)
> if ((flags >> v22_PG_Slab) & 1)
> slabs++;
> } else if (vt->PG_slab) {
> - if ((flags >> vt->PG_slab) & 1)
> + if (page_slab(pp, flags))
> slabs++;
> } else {
> if ((flags >> v24_PG_slab) & 1)
> @@ -6775,6 +6816,9 @@ page_flags_init_from_pageflag_names(void)
> vt->pageflags_data[i].name = nameptr;
> vt->pageflags_data[i].mask = mask;
>
> + if (!strncmp(nameptr, "slab", 4))
> + vt->flags |= SLAB_PAGEFLAGS;
> +
> if (CRASHDEBUG(1)) {
> fprintf(fp, " %08lx %s\n",
> vt->pageflags_data[i].mask,
> @@ -6836,7 +6880,8 @@ page_flags_init_from_pageflags_enum(void)
> strcpy(nameptr, arglist[0] + strlen("PG_"));
> vt->pageflags_data[p].name = nameptr;
> vt->pageflags_data[p].mask = 1 <<
> atoi(arglist[2]);
> -
> + if (!strncmp(nameptr, "slab", 4))
> + vt->flags |= SLAB_PAGEFLAGS;
> p++;
> }
> } else
> @@ -9736,14 +9781,14 @@ vaddr_to_kmem_cache(ulong vaddr, char *buf, int
> verbose)
> readmem(page+OFFSET(page_flags), KVADDR,
> &page_flags, sizeof(ulong), "page.flags",
> FAULT_ON_ERROR);
> - if (!(page_flags & (1 << vt->PG_slab))) {
> + if (!page_slab(page, page_flags)) {
> if (((vt->flags & KMALLOC_SLUB) ||
> VALID_MEMBER(page_compound_head)) ||
> ((vt->flags & KMALLOC_COMMON) &&
> VALID_MEMBER(page_slab) &&
> VALID_MEMBER(page_first_page))) {
>
> readmem(compound_head(page)+OFFSET(page_flags), KVADDR,
> &page_flags, sizeof(ulong),
> "page.flags",
> FAULT_ON_ERROR);
> - if (!(page_flags & (1 << vt->PG_slab)))
> + if (!page_slab(compound_head(page),
> page_flags))
> return NULL;
> } else
> return NULL;
> @@ -20195,7 +20240,7 @@ char *
> is_slab_page(struct meminfo *si, char *buf)
> {
> int i, cnt;
> - ulong page_slab, page_flags, name;
> + ulong pg_slab, page_flags, name;
> ulong *cache_list;
> char *retval;
>
> @@ -20210,11 +20255,11 @@ is_slab_page(struct meminfo *si, char *buf)
> RETURN_ON_ERROR|QUIET))
> return NULL;
>
> - if (!(page_flags & (1 << vt->PG_slab)))
> + if (!page_slab(si->spec_addr, page_flags))
> return NULL;
>
> - if (!readmem(si->spec_addr + OFFSET(page_slab), KVADDR,
> - &page_slab, sizeof(ulong), "page.slab",
> + if (!readmem(si->spec_addr + OFFSET(page_slab), KVADDR,
> + &pg_slab, sizeof(ulong), "page.slab",
> RETURN_ON_ERROR|QUIET))
> return NULL;
>
> @@ -20222,7 +20267,7 @@ is_slab_page(struct meminfo *si, char *buf)
> cnt = get_kmem_cache_list(&cache_list);
>
> for (i = 0; i < cnt; i++) {
> - if (page_slab == cache_list[i]) {
> + if (pg_slab == cache_list[i]) {
> if (!readmem(cache_list[i] +
> OFFSET(kmem_cache_name),
> KVADDR, &name, sizeof(char *),
> "kmem_cache.name", QUIET|RETURN_ON_ERROR))
> --
> 2.25.1
>
15 hours, 2 minutes
Re: [PATCH] x86_64: Fix the bug of getting incorrect framesize
by lijiang
On Mon, Sep 16, 2024 at 3:46 PM <devel-request(a)lists.crash-utility.osci.io>
wrote:
> Date: Mon, 16 Sep 2024 19:44:58 +1200
> From: Tao Liu <ltao(a)redhat.com>
> Subject: [Crash-utility] [PATCH] x86_64: Fix the bug of getting
> incorrect framesize
> To: devel(a)lists.crash-utility.osci.io
> Cc: mycomplexlove(a)gmail.com
> Message-ID: <20240916074458.105832-1-ltao(a)redhat.com>
> Content-Type: text/plain; charset="US-ASCII"; x-default=true
>
> Previously, "retq" is used to determine the end of a function, so the end
> of framesize calculation. However "ret" might be outputted by gdb rather
> than "retq", as a result, the framesize is returned incorrectly, and bogus
> stack trace will be outputted.
>
> Without the patch:
>
> $ crash -d 3 vmcore vmlinux
> crash> bt
> 0xffffffff92da7545 <copy_process+5>: push %rbp [framesize: 8]
> ...
> 0xffffffff92da7561 <copy_process+33>: sub $0x238,%rsp
> [framesize: 624]
> ...
> 0xffffffff92da776a <copy_process+554>: pop %r15
> [framesize: 8]
> 0xffffffff92da776c <copy_process+556>: pop %rbp
> [framesize: 0]
> 0xffffffff92da776d <copy_process+557>: ret
>
> crash> bt -D dump
> framesize_cache_entries:
> ...
> [ 3]: ffffffff92dadcbd 0 CF (copy_process+26493)
>
> crash> bt
> ...
> #9 [ffff888263157bc0] copy_process at ffffffff92dadcbd
> #10 [ffff888263157d20] __mutex_init at ffffffff92ed8dd5
> #11 [ffff888263157d38] __alloc_file at ffffffff93458397
> #12 [ffff888263157d60] alloc_empty_file at ffffffff934585d2
> #13 [ffff888263157da8] __alloc_fd at ffffffff934b5ead
> #14 [ffff888263157e38] _do_fork at ffffffff92dae7a1
> #15 [ffff888263157f28] do_syscall_64 at ffffffff92c085f4
>
> Stack #10 ~ #13 are bogus and misleading.
>
> With the patch:
> ...
> 0xffffffff92da776d <copy_process+557>: ret [framesize
> restored to: 624]
>
> crash> bt -D dump
> ...
> [ 3]: ffffffff92dadcbd 624 CF (copy_process+26493)
>
> crash> bt
> ...
> #9 [ffff888263157bc0] copy_process at ffffffff92dadcbd
> #10 [ffff888263157e38] _do_fork at ffffffff92dae7a1
> #11 [ffff888263157f28] do_syscall_64 at ffffffff92c085f4
>
> Signed-off-by: Tao Liu <ltao(a)redhat.com>
> ---
> x86_64.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/x86_64.c b/x86_64.c
> index 469d26b..7aa9430 100644
> --- a/x86_64.c
> +++ b/x86_64.c
> @@ -8781,7 +8781,8 @@ x86_64_get_framesize(struct bt_info *bt, ulong
> textaddr, ulong rsp, char *stack_
> if (CRASHDEBUG(2) || (bt->flags &
> BT_FRAMESIZE_DEBUG))
> fprintf(fp, "%s\t[framesize: %d]\n",
> strip_linefeeds(buf2), framesize);
> - } else if (STRNEQ(arglist[instr], "retq")) {
> + } else if (STRNEQ(arglist[instr], "retq") ||
> + STRNEQ(arglist[instr], "ret")) {
>
Thank you for the fix, Tao.
This looks good. Applied:
https://github.com/crash-utility/crash/commit/0d2ad774532db3c4dad6cda05d5...
Lianbo
if (!exception) {
> framesize = max;
> if (CRASHDEBUG(2) || (bt->flags &
> BT_FRAMESIZE_DEBUG))
> --
> 2.40.1
>
1 day, 6 hours
[PATCH] ppc64: Fix bt printing error stack trace
by Tao Liu
A error stack trace of bt cmd observed:
crash> bt 1
PID: 1 TASK: c000000003714b80 CPU: 2 COMMAND: "systemd"
#0 [c0000000037735c0] _end at c0000000037154b0 (unreliable)
#1 [c000000003773770] __switch_to at c00000000001fa9c
#2 [c0000000037737d0] __schedule at c00000000112e4ec
#3 [c0000000037738b0] schedule at c00000000112ea80
...
The #0 stack trace is incorrect, the function address shouldn't exceed _end.
The reason is for kernel>=v6.2, the offset of pt_regs to sp changed from
STACK_FRAME_OVERHEAD, i.e 112, to STACK_SWITCH_FRAME_REGS. For
CONFIG_PPC64_ELF_ABI_V1, it's 112, for ABI_V2, it's 48. So the nip will read a
wrong value from stack when ABI_V2 enabled.
To determine if ABI_V2 enabled is tricky. This patch do it by check the
following:
In arch/powerpc/include/asm/ppc_asm.h:
#ifdef CONFIG_PPC64_ELF_ABI_V2
#define STK_GOT 24
#else
#define STK_GOT 40
In arch/powerpc/kernel/tm.S:
_GLOBAL(tm_reclaim)
mfcr r5
mflr r0
stw r5, 8(r1)
std r0, 16(r1)
std r2, STK_GOT(r1)
...
So a disassemble on tm_reclaim, and extract the STK_GOT value from std
instruction is used as the approach.
After the patch:
crash> bt 1
PID: 1 TASK: c000000003714b80 CPU: 2 COMMAND: "systemd"
#0 [c0000000037737d0] __schedule at c00000000112e4ec
#1 [c0000000037738b0] schedule at c00000000112ea80
...
Signed-off-by: Tao Liu <ltao(a)redhat.com>
---
This patch is the follow-up of gdb stack unwinding support v7 discussion[1],
where a "gdb bt" fail is observed. After applying the patch, the "gdb bt" can
also work normal for gdb stack unwinding support v7.
[1]: https://www.mail-archive.com/devel@lists.crash-utility.osci.io/msg01120.html
---
defs.h | 1 +
ppc64.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
2 files changed, 57 insertions(+), 3 deletions(-)
diff --git a/defs.h b/defs.h
index 2231cb6..d5cb8cc 100644
--- a/defs.h
+++ b/defs.h
@@ -4643,6 +4643,7 @@ struct efi_memory_desc_t {
#define MSR_PR_LG 14 /* Problem State / Privilege Level */
/* Used to find the user or kernel-mode frame*/
+#define STACK_SWITCH_FRAME_REGS 48
#define STACK_FRAME_OVERHEAD 112
#define EXCP_FRAME_MARKER 0x7265677368657265
diff --git a/ppc64.c b/ppc64.c
index e8930a1..5e2595e 100644
--- a/ppc64.c
+++ b/ppc64.c
@@ -2813,6 +2813,51 @@ ppc64_get_sp(ulong task)
return sp;
}
+static bool
+is_ppc64_elf_abi_v2(void)
+{
+ char buf1[BUFSIZE];
+ char *pos1, *pos2;
+ int errflag = 0;
+ ulong stk_got = 0;
+ static bool ret = false;
+ static bool checked = false;
+
+ if (checked == true || !symbol_exists("tm_reclaim"))
+ return ret;
+
+ sprintf(buf1, "x/16i tm_reclaim");
+ open_tmpfile();
+ if (!gdb_pass_through(buf1, pc->tmpfile, GNU_RETURN_ON_ERROR))
+ goto out;
+ checked = true;
+ rewind(pc->tmpfile);
+ while (fgets(buf1, BUFSIZE, pc->tmpfile)) {
+ // "std r2, STK_GOT(r1)" is expected
+ if (strstr(buf1, "std") &&
+ strstr(buf1, "(r1)") &&
+ (pos1 = strstr(buf1, "r2,"))) {
+ pos1 += strlen("r2,");
+ for (pos2 = pos1; *pos2 != '\0' && *pos2 != '('; pos2++);
+ *pos2 = '\0';
+ stk_got = stol(pos1, RETURN_ON_ERROR|QUIET, &errflag);
+ break;
+ }
+ }
+
+ if (!errflag) {
+ switch (stk_got) {
+ case 24:
+ ret = true;
+ case 40:
+ goto out;
+ }
+ }
+ error(WARNING, "Unstable elf_abi v1/v2 detection.\n");
+out:
+ close_tmpfile();
+ return ret;
+}
/*
* get the SP and PC values for idle tasks.
@@ -2834,9 +2879,17 @@ get_ppc64_frame(struct bt_info *bt, ulong *getpc, ulong *getsp)
sp = ppc64_get_sp(task);
if (!INSTACK(sp, bt))
goto out;
- readmem(sp+STACK_FRAME_OVERHEAD, KVADDR, ®s,
- sizeof(struct ppc64_pt_regs),
- "PPC64 pt_regs", FAULT_ON_ERROR);
+
+ if (THIS_KERNEL_VERSION >= LINUX(6,2,0) && is_ppc64_elf_abi_v2()) {
+ readmem(sp+STACK_SWITCH_FRAME_REGS, KVADDR, ®s,
+ sizeof(struct ppc64_pt_regs),
+ "PPC64 pt_regs", FAULT_ON_ERROR);
+ } else {
+ readmem(sp+STACK_FRAME_OVERHEAD, KVADDR, ®s,
+ sizeof(struct ppc64_pt_regs),
+ "PPC64 pt_regs", FAULT_ON_ERROR);
+ }
+
ip = regs.nip;
closest = closest_symbol(ip);
if (STREQ(closest, ".__switch_to") || STREQ(closest, "__switch_to")) {
--
2.40.1
1 day, 16 hours