Hi Tao,
Tao Liu <ltao@xxxxxxxxxx> 于2024年5月3日周五 17:58写道：
Hi Guanyou,
Thanks for your patch, just some minor comments:
On Wed, Apr 24, 2024 at 5:24 PM Guanyou Chen <chenguanyou9338@xxxxxxxxx> wrote:
>
> Hi LianBo
>
> We don't need struct zspage_5_17.
Could you please rewrite your commit message, so people can know what
the patch is trying to do without diving into the code? E.g.
refactoring the code by combining 2 structures into one etc.
>
> ---
> defs.h | 32 +++++++++++++++-----------------
> diskdump.c | 15 ++++++---------
> 2 files changed, 21 insertions(+), 26 deletions(-)
>
> diff --git a/defs.h b/defs.h
> index 3cb8e63..01f316e 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -7407,28 +7407,26 @@ ulong try_zram_decompress(ulonglong pte_val, unsigned char *buf, ulong len, ulon
> #define SECTORS_PER_PAGE (1 << SECTORS_PER_PAGE_SHIFT)
>
> struct zspage {
> - struct {
> - unsigned int fullness : 2;
> - unsigned int class : 9;
> - unsigned int isolated : 3;
> - unsigned int magic : 8;
> + union {
> + unsigned int flag_bits;
> + struct {
> + unsigned int fullness : 2;
> + unsigned int class : 9;
> + unsigned int isolated : 3;
> + unsigned int magic : 8;
> + } v0;
> + struct {
> + unsigned int huge : 1;
> + unsigned int fullness : 2;
> + unsigned int class : 9;
> + unsigned int isolated : 3;
> + unsigned int magic : 8;
> + } v5_17;
> };
> unsigned int inuse;
> unsigned int freeobj;
> };
>
> -struct zspage_5_17 {
> - struct {
> - unsigned int huge : 1;
> - unsigned int fullness : 2;
> - unsigned int class : 9;
> - unsigned int isolated : 3;
> - unsigned int magic : 8;
> - };
> - unsigned int inuse;
> - unsigned int freeobj;
> -};
> -
> /*
> * makedumpfile.c
> */
> diff --git a/diskdump.c b/diskdump.c
> index 3ae7bf2..a928a0e 100644
> --- a/diskdump.c
> +++ b/diskdump.c
> @@ -2819,7 +2819,6 @@ zram_object_addr(ulong pool, ulong handle, unsigned char *zram_buf)
> {
> ulong obj, off, class, page, zspage;
> struct zspage zspage_s;
> - struct zspage_5_17 zspage_5_17_s;
> physaddr_t paddr;
> unsigned int obj_idx, class_idx, size;
> ulong pages[2], sizes[2];
> @@ -2833,15 +2832,13 @@ zram_object_addr(ulong pool, ulong handle, unsigned char *zram_buf)
> readmem(page + OFFSET(page_private), KVADDR, &zspage,
> sizeof(void *), "page_private", FAULT_ON_ERROR);
>
> + readmem(zspage, KVADDR, &zspage_s, sizeof(struct zspage), "zspage", FAULT_ON_ERROR);
^^^^^
In your patch file attached, here is space instead of tab.
Thanks,
Tao Liu
> if (VALID_MEMBER(zspage_huge)) {
> - readmem(zspage, KVADDR, &zspage_5_17_s,
> - sizeof(struct zspage_5_17), "zspage_5_17", FAULT_ON_ERROR);
> - class_idx = zspage_5_17_s.class;
> - zs_magic = zspage_5_17_s.magic;
> + class_idx = zspage_s.v5_17.class;
> + zs_magic = zspage_s.v5_17.magic;
> } else {
> - readmem(zspage, KVADDR, &zspage_s, sizeof(struct zspage), "zspage", FAULT_ON_ERROR);
> - class_idx = zspage_s.class;
> - zs_magic = zspage_s.magic;
> + class_idx = zspage_s.v0.class;
> + zs_magic = zspage_s.v0.magic;
> }
>
> if (zs_magic != ZSPAGE_MAGIC)
> @@ -2887,7 +2884,7 @@ zram_object_addr(ulong pool, ulong handle, unsigned char *zram_buf)
>
> out:
> if (VALID_MEMBER(zspage_huge)) {
> - if (!zspage_5_17_s.huge)
> + if (!zspage_s.v5_17.huge)
> return (zram_buf + ZS_HANDLE_SIZE);
> } else {
> readmem(page, KVADDR, &obj, sizeof(void *), "page flags", FAULT_ON_ERROR);
> --
> 2.39.0
> --
> Crash-utility mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx
> To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx
> https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
> Contribution Guidelines: https://github.com/crash-utility/crash/wiki
From 29d40ce3c853a1f19b164179de1c8b75a7ece7e9 Mon Sep 17 00:00:00 2001
From: chenguanyou <chenguanyou@xxxxxxxxxx>
Date: Wed, 24 Apr 2024 17:00:20 +0800
Subject: [PATCH V2] remove struct zspage_5_17 and use union to resolve issue
This patch is an addition to that commit [1], The reason
is that the structure of zspage has not changed, just new
bits have been introduced. So use union to resolve issue.
[1] 0172e35083b5 ("Fix "rd" command to display data on zram on Linux 5.17 and later")
Signed-off-by: chenguanyou <chenguanyou@xxxxxxxxxx>
---
defs.h | 32 +++++++++++++++-----------------
diskdump.c | 15 ++++++---------
2 files changed, 21 insertions(+), 26 deletions(-)
diff --git a/defs.h b/defs.h
index 3cb8e63..01f316e 100644
--- a/defs.h
+++ b/defs.h
@@ -7407,28 +7407,26 @@ ulong try_zram_decompress(ulonglong pte_val, unsigned char *buf, ulong len, ulon
#define SECTORS_PER_PAGE (1 << SECTORS_PER_PAGE_SHIFT)
struct zspage {
- struct {
- unsigned int fullness : 2;
- unsigned int class : 9;
- unsigned int isolated : 3;
- unsigned int magic : 8;
+ union {
+ unsigned int flag_bits;
+ struct {
+ unsigned int fullness : 2;
+ unsigned int class : 9;
+ unsigned int isolated : 3;
+ unsigned int magic : 8;
+ } v0;
+ struct {
+ unsigned int huge : 1;
+ unsigned int fullness : 2;
+ unsigned int class : 9;
+ unsigned int isolated : 3;
+ unsigned int magic : 8;
+ } v5_17;
};
unsigned int inuse;
unsigned int freeobj;
};
-struct zspage_5_17 {
- struct {
- unsigned int huge : 1;
- unsigned int fullness : 2;
- unsigned int class : 9;
- unsigned int isolated : 3;
- unsigned int magic : 8;
- };
- unsigned int inuse;
- unsigned int freeobj;
-};
-
/*
* makedumpfile.c
*/
diff --git a/diskdump.c b/diskdump.c
index 3ae7bf2..e87c40f 100644
--- a/diskdump.c
+++ b/diskdump.c
@@ -2819,7 +2819,6 @@ zram_object_addr(ulong pool, ulong handle, unsigned char *zram_buf)
{
ulong obj, off, class, page, zspage;
struct zspage zspage_s;
- struct zspage_5_17 zspage_5_17_s;
physaddr_t paddr;
unsigned int obj_idx, class_idx, size;
ulong pages[2], sizes[2];
@@ -2833,15 +2832,13 @@ zram_object_addr(ulong pool, ulong handle, unsigned char *zram_buf)
readmem(page + OFFSET(page_private), KVADDR, &zspage,
sizeof(void *), "page_private", FAULT_ON_ERROR);
+ readmem(zspage, KVADDR, &zspage_s, sizeof(struct zspage), "zspage", FAULT_ON_ERROR);
if (VALID_MEMBER(zspage_huge)) {
- readmem(zspage, KVADDR, &zspage_5_17_s,
- sizeof(struct zspage_5_17), "zspage_5_17", FAULT_ON_ERROR);
- class_idx = zspage_5_17_s.class;
- zs_magic = zspage_5_17_s.magic;
+ class_idx = zspage_s.v5_17.class;
+ zs_magic = zspage_s.v5_17.magic;
} else {
- readmem(zspage, KVADDR, &zspage_s, sizeof(struct zspage), "zspage", FAULT_ON_ERROR);
- class_idx = zspage_s.class;
- zs_magic = zspage_s.magic;
+ class_idx = zspage_s.v0.class;
+ zs_magic = zspage_s.v0.magic;
}
if (zs_magic != ZSPAGE_MAGIC)
@@ -2887,7 +2884,7 @@ zram_object_addr(ulong pool, ulong handle, unsigned char *zram_buf)
out:
if (VALID_MEMBER(zspage_huge)) {
- if (!zspage_5_17_s.huge)
+ if (!zspage_s.v5_17.huge)
return (zram_buf + ZS_HANDLE_SIZE);
} else {
readmem(page, KVADDR, &obj, sizeof(void *), "page flags", FAULT_ON_ERROR);
--
2.39.0
-- Crash-utility mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/ Contribution Guidelines: https://github.com/crash-utility/crash/wiki
