[Crash-utility] [External Mail]Re: [PATCH v4] Fix: move huge compressed obj from page to zspage

[lijiang]

On Tue, Oct 17, 2023 at 3:52 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab at 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/crash-utility/attachments/20231018/010b25ba/attachment.htm>