Re: [Crash-utility] [External Mail]Re: [PATCH v4] Fix: move huge compressed obj from page to zspage
On Tue, Oct 17, 2023 at 3:52 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com>
wrote:
> ---
> defs.h | 26 +++++++++++++++++++++
> diskdump.c | 66 ++++++++++++++++++++++++++++++++++++++++++++----------
> 2 files changed, 80 insertions(+), 12 deletions(-)
>
> diff --git a/defs.h b/defs.h
> index 96a7a2a..2038351 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -2225,6 +2225,7 @@ struct offset_table { /* stash
of commonly-used offsets */
> long module_memory_base;
> long module_memory_size;
> long irq_data_irq;
> + long zspage_huge;
> };
>
> struct size_table { /* stash of commonly-used sizes */
> @@ -7210,6 +7211,19 @@ ulong try_zram_decompress(ulonglong pte_val,
unsigned char *buf, ulong len, ulon
> #define SECTORS_PER_PAGE (1 << SECTORS_PER_PAGE_SHIFT)
> #define ZRAM_FLAG_SHIFT (1<<24)
> #define ZRAM_FLAG_SAME_BIT (1<<25)
> +
> +struct zram_pageflags {
> + long ZRAM_LOCK;
> + long ZRAM_SAME;
> +};
> +
> +/*
> + * diskdump.c
> + */
> +extern struct zram_pageflags zram_pageflags;
> +#define ZRAM_PAGEFLAG_INIT(X, Y) (zram_pageflags.X = Y)
> +#define ZRAM_PAGEFLAG_VALUE(X) (zram_pageflags.X)
Thank you for the update.
I don't think these macros are necessary, so simplified the patch on my
end. With rethinking the names, kept "ZRAM_FLAG_SHIFT" and
"ZRAM_FLAG_SAME_BIT" as they are, and made some tweaks.
Guanyou and Lianbo, is this attached patch OK?
The attached patch looks good to me, only two suggestions:
(I copied the following code block from the attached patch)
+static long ZRAM_FLAG_SHIFT;
+static long ZRAM_FLAG_SAME_BIT;
The ulong is good to me.
+
static void
zram_init(void)
{
+ long zram_lock;
+
MEMBER_OFFSET_INIT(zram_mempoll, "zram", "mem_pool");
MEMBER_OFFSET_INIT(zram_compressor, "zram", "compressor");
MEMBER_OFFSET_INIT(zram_table_flag, "zram_table_entry",
"flags");
if (INVALID_MEMBER(zram_table_flag))
MEMBER_OFFSET_INIT(zram_table_flag, "zram_table_entry",
"value");
STRUCT_SIZE_INIT(zram_table_entry, "zram_table_entry");
+ MEMBER_OFFSET_INIT(zspoll_size_class, "zs_pool",
"size_class");
+ MEMBER_OFFSET_INIT(size_class_size, "size_class", "size");
+ MEMBER_OFFSET_INIT(zspage_huge, "zspage", "huge");
+
+ if (enumerator_value("ZRAM_LOCK", &zram_lock))
+ ZRAM_FLAG_SHIFT = 1 << zram_lock;
+ else if (THIS_KERNEL_VERSION >= LINUX(6,1,0))
This depends on kernel version number, it might be more friendly for the
distribution to output some debug info as below:
if (CRASHDEBUG(1))
error(INFO, "ZRAM_FLAG_SHIFT=%lu\n", ZRAM_FLAG_SHIFT);
Once the related kernel patches are backported to an old kernel, the
similar checking may fail. But, at least we can know a little more with the
crash debug info.
+ ZRAM_FLAG_SHIFT = 1 << (PAGESHIFT() + 1);
+ else
+ ZRAM_FLAG_SHIFT = 1 << 24;
+
+ ZRAM_FLAG_SAME_BIT = ZRAM_FLAG_SHIFT << 1;
}
Anyway, that is just my thoughts, you could decide if they need to be
improved a bit when merging.
Other changes are fine to me, so: Ack.
I tested this on Linux 6.1.
crash> vtop 00007fb60be00000
VIRTUAL PHYSICAL
7fb60be00000 (not mapped)
PGD: 108e807f8 => 1107c4067
PUD: 1107c46c0 => 106ec9067
PMD: 106ec92f8 => 1060f2067
PTE: 1060f2000 => 7fffffffdfffe0a
PTE SWAP OFFSET
7fffffffdfffe0a /dev/zram0 65536
VMA START END FLAGS FILE
ffff935206cda980 7fb60bdfe000 7fb64be00000 8100073
SWAP: /dev/zram0 OFFSET: 65536
crash> rd 00007fb60be00000
7fb60be00000: 0000000000001000 ........
(Note that it looks like Linux 6.2 and later have other changes related
to zram, so this patch supports up to 6.1. We can fix them later.)
Ok, it's another issue.
Thanks.
Lianbo
Thanks,
Kazu
Attachments:
- attachment.html (text/html — 5.8 KB)