Dave, Zhao,
-----Original Message-----
From: Dave Anderson <anderson(a)redhat.com>
Sent: Tuesday, March 31, 2020 10:52 PM
To: Discussion list for crash utility usage, maintenance and development
<crash-utility(a)redhat.com>
Cc: d hatayama <d.hatayama(a)gmail.com>; Hatayama, Daisuke/畑山 大輔
<d.hatayama(a)fujitsu.com>
Subject: Re: [Crash-utility] zram decompress support for gcore/crash-utility
----- Original Message -----
>
>
> Hello list,
>
> When i try to use gcore to parse coredump from fulldump,I got below issue
> that make the coredump unavailable.
>
> 1. Zram is very common feature,in current android system supports zram
> swap,but gcore/crash-utility does not support,even zram swap can decoded
> from ram,many page fault due to this reason.
>
> I added zram decompress feature to gcore,and i ’ m also considering wheather
> support zram in crash-utility,but for this feature,i have to add miniLZO to
> codebase, I'm not sure if it's acceptable,plase help give some advice.
>
> miniLZO :miniLZO is a very lightweight subset of the LZO library,Distributed
> under the terms of the GNU General Public License (GPL v2+).
If you build the upstream crash utility package with "make lzo", it will bring
in those libraries, which are required for dumpfiles compressed with LZO. You
can enter the make command after the package has been built, as it simply
re-compiles diskdump.c and includes the libraries in the build.
As far as the gcore extension module, that is maintained by Daisuke Hatayama,
and he make all decisions w/respect to that codebase. I've cc'd this response
to him.
Thanks Zhao for your patch set.
Thanks for ccing me, Dave.
I agree that ZRAM support is useful as your explanation. On the other
hand, it is not only for crash gcore command, but also for crash utility. I think
it more natural than the current implementation of your patch set that you
implement a ZRAM support in crash utility and then do it in crash gcore command.
If the ZRAM support were transparent to readmem() interface, there would be no need
to implement crash gcore command at all. If not, there would be need to add a new code
for the ZRAM support just corresponding to the following stanza in
0001-gcore-add-support-zram-swap.patch:
@@ -225,6 +417,18 @@ void gcore_coredump(void)
strerror(errno));
} else {
pagefaultf("page fault at %lx\n", addr);
+ if (paddr != 0) {
+ pte_val = paddr;
+ if(try_zram_decompress(pte_val, (unsigned char
*)buffer) == PAGE_SIZE)
+ {
+ error(WARNING, "zram decompress
successed\n");
+ if (fwrite(buffer, PAGE_SIZE, 1,
gcore->fp) != 1)
+ error(FATAL, "%s: write:
%s\n",
+ gcore->corename,
+ strerror(errno));
+ continue;
+ }
+ }
Dave
.
>
>
http://www.oberhumer.com/opensource/lzo/
>
>
>
> This change is a bit big,I attached it to the mail,if attachment is not
> available,you can also see these patch in github:
>
https://github.com/zhaoqianli0202/crash-gcore/commits/upstream
>
> Please review.
>
>
>
> 2. For historical reasons,kernel reserved top 8/16 bytes of stacks,but after
> kernel-4.14, this reservation was cancelled,so gcore needs to improve
> compatibility.
>
> kernel change as below:
>
> commit 34be98f4944f99076f049a6806fc5f5207a755d3
>
> Author: Ard Biesheuvel < ard.biesheuvel(a)linaro.org >
>
> Date: Thu Jul 20 17:15:45 2017 +0100
>
>
>
> arm64: kernel: remove {THREAD,IRQ_STACK}_START_SP
>
>
>
> For historical reasons, we leave the top 16 bytes of our task and IRQ
>
> stacks unused, a practice used to ensure that the SP can always be
>
> masked to find the base of the current stack (historically, where
>
> thread_info could be found).™
>
> ====
>
> Patch for this issue:
>
> commit d1031df4617351a58b8edfb0121c306baaa34f9d
>
> Author: zhaoqianli <zhaoqianli(a)xiaomi.com>
>
> Date: Mon Mar 30 12:07:02 2020 +0800
>
>
>
> gcore: ARM/ARM64 reserved 8/16 byte in the top of stacks before 4.14,
>
> but this reservation was removed after 4.14
>
> Without this patch,gcore counld't parse full callstack in version after 4.14
>
>
>
> diff --git a/gcore.c b/gcore.c
>
> index f75701d..f6e1787 100644
>
> --- a/gcore.c
>
> +++ b/gcore.c
>
> @@ -558,4 +558,16 @@ static void gcore_machdep_init(void)
>
>
>
> if (!gcore_arch_vsyscall_has_vm_alwaysdump_flag())
>
> gcore_machdep->vm_alwaysdump = 0x00000000;
>
> +
> 172272617227261722726
> +#if defined(ARM) || defined(ARM64)
>
> +#ifdef ARM
>
> +#define STACK_RESERVE_SIZE 8
>
> +#else
>
> +#define STACK_RESERVE_SIZE 16
>
> +#endif
>
> + if (THIS_KERNEL_VERSION >= LINUX(4,14,0))
>
> + gcore_machdep->stack_reserve = 0;
>
> + else
>
> + gcore_machdep->stack_reserve = STACK_RESERVE_SIZE;
>
> +#endif
>
> }
>
> diff --git a/libgcore/gcore_arm.c b/libgcore/gcore_arm.c
>
> index 891d01e..c8aefdf 100644
>
> --- a/libgcore/gcore_arm.c
>
> +++ b/libgcore/gcore_arm.c
>
> @@ -29,7 +29,7 @@ static int gpr_get(struct task_context *target,
>
>
>
> BZERO(regs, sizeof(*regs));
>
>
>
> - readmem(machdep->get_stacktop(target->task) - 8 - SIZE(pt_regs), KVADDR,
>
> + readmem(machdep->get_stacktop(target->task) -
gcore_machdep->stack_reserve
> - SIZE(pt_regs), KVADDR,
>
> regs, SIZE(pt_regs), "genregs_get: pt_regs",
>
> gcore_verbose_error_handle());
>
>
>
> diff --git a/libgcore/gcore_arm64.c b/libgcore/gcore_arm64.c
>
> index 3257389..ed3fdc8 100644
>
> --- a/libgcore/gcore_arm64.c
>
> +++ b/libgcore/gcore_arm64.c
>
> @@ -28,7 +28,7 @@ static int gpr_get(struct task_context *target,
>
>
>
> BZERO(regs, sizeof(*regs));
>
>
>
> - readmem(machdep->get_stacktop(target->task) - 16 - SIZE(pt_regs), KVADDR,
>
> + readmem(machdep->get_stacktop(target->task) -
gcore_machdep->stack_reserve
> - SIZE(pt_regs), KVADDR,
>
> regs, sizeof(struct user_pt_regs), "gpr_get: user_pt_regs",
>
> gcore_verbose_error_handle());
>
>
>
> @@ -124,7 +124,7 @@ static int compat_gpr_get(struct task_context *target,
>
> BZERO(&pt_regs, sizeof(pt_regs));
>
> BZERO(regs, sizeof(*regs));
>
>
>
> - readmem(machdep->get_stacktop(target->task) - 16 - SIZE(pt_regs), KVADDR,
>
> + readmem(machdep->get_stacktop(target->task) -
gcore_machdep->stack_reserve
> - SIZE(pt_regs), KVADDR,
>
> &pt_regs, sizeof(struct pt_regs), "compat_gpr_get: pt_regs",
>
> gcore_verbose_error_handle());
>
>
>
> diff --git a/libgcore/gcore_defs.h b/libgcore/gcore_defs.h
>
> index b0f5603..f31036c 100644
>
> --- a/libgcore/gcore_defs.h
>
> +++ b/libgcore/gcore_defs.h
>
> @@ -1177,6 +1177,7 @@ extern struct gcore_size_table gcore_size_table;
>
> struct gcore_machdep_table
>
> {
>
> ulong vm_alwaysdump;
>
> + uint8_t stack_reserve;
>
> };
> #/******本邮件及其附件含有小米公司的保密信息,仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形
式使用(包括但不限于全部或部分地泄露、复制、或散发)本邮件中的信息。如果您错收了本邮件,请您立即电话或邮件通
知发件人并删除本邮件!
> This e-mail and its attachments contain confidential information from
> XIAOMI, which is intended only for the person or entity whose address is
> listed above. Any use of the information contained herein in any way
> (including, but not limited to, total or partial disclosure, reproduction,
> or dissemination) by persons other than the intended recipient(s) is
> prohibited. If you receive this e-mail in error, please notify the sender by
> phone or email immediately and delete it!******/#
>
> --
> Crash-utility mailing list
> Crash-utility(a)redhat.com
>
https://www.redhat.com/mailman/listinfo/crash-utility