[Crash-utility] Re: [PATCH] remove struct zspage_5_17 and use union to resolve issue

Hi Guanyou, Thanks for your patch, just some minor comments: On Wed, Apr 24, 2024 at 5:24 PM Guanyou Chen <chenguanyou9338(a)gmail.com&gt; 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(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