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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2023/10/18 21:05, lijiang wrote:
> On Tue, Oct 17, 2023 at 3:52 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@xxxxxxx>
> 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.

Ah, agree.

The former ZRAM_LOCK was a shift value and set by enumerator_value(char 
*, long *) so it was long.  I changed it to a mask value, so a vestige.

> 
> +
>   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.

Sounds good, then I would like to show the shift value instead of mask 
value, so changed a little.  Is the attached patch OK?

6.1:
   crash> rd 7fb60bdff000
   zram_flag_shift: 13
   ...

RHEL8:
   crash> rd 7ff668048000
   zram_flag_shift: 24

4.18 and later kernels has ZRAM_LOCK, so I think usually the else-if 
block isn't used and it's kind of a fail-safe for a future change.

Anyway, thank you for the good comments.

Thanks,
Kazu
From 00d469a3e21472b47ae767e08795d8159a16aac0 Mon Sep 17 00:00:00 2001
From: chenguanyou <chenguanyou@xxxxxxxxxx>
Date: Mon, 11 Sep 2023 20:59:39 +0800
Subject: [PATCH] Fix "rd" command to display data on zram on Linux 5.17 and
 later

Fix "rd" command to display data on zram on Linux 5.17 and later kernels
that contain commits
  a41ec880aa7b ("zsmalloc: move huge compressed obj from page to zspage"),
  ffedd09fa9b0 ("zsmalloc: Stop using slab fields in struct page"),
and on Linux 6.1 and later that contain commit
  f635725c3905 ("zram: do not waste zram_table_entry flags bits").

Also, fix a bug that sets the same "byte" by memset() instead to pages
containing the same "unsigned long" elements.

Before:
  crash> mod -s zram zram.ko
       MODULE       NAME                                          BASE           SIZE  OBJECT FILE
  ffffffde224db800  zram                                    ffffffde224d2000    57344  zram.ko
  crash> mod -s zsmalloc zsmalloc.ko
       MODULE       NAME                                          BASE           SIZE  OBJECT FILE
  ffffffde224c5180  zsmalloc                                ffffffde224bf000    40960  zsmalloc.ko
  crash> rd 0x13d89fb0
  rd: zspage magic incorrect: b0

After:
  crash> rd 0x13d89fb0
          13d89fb0:  c2b54f7170883b20                     ;.pqO..

Signed-off-by: chenguanyou <chenguanyou@xxxxxxxxxx>
Signed-off-by: Kazuhito Hagio <k-hagio-ab@xxxxxxx>
---
 defs.h     | 16 ++++++++++++--
 diskdump.c | 65 +++++++++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 69 insertions(+), 12 deletions(-)

diff --git a/defs.h b/defs.h
index 96a7a2a31471..122d3a969bf9 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 */
@@ -7208,8 +7209,7 @@ ulong try_zram_decompress(ulonglong pte_val, unsigned char *buf, ulong len, ulon
 #define SECTOR_SHIFT     9
 #define SECTORS_PER_PAGE_SHIFT  (PAGESHIFT() - SECTOR_SHIFT)
 #define SECTORS_PER_PAGE        (1 << SECTORS_PER_PAGE_SHIFT)
-#define ZRAM_FLAG_SHIFT         (1<<24)
-#define ZRAM_FLAG_SAME_BIT      (1<<25)
+
 struct zspage {
     struct {
         unsigned int fullness : 2;
@@ -7221,6 +7221,18 @@ struct zspage {
     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 2c284ff3f97f..3c533ba73c6e 100644
--- a/diskdump.c
+++ b/diskdump.c
@@ -2745,15 +2745,36 @@ diskdump_device_dump_info(FILE *ofp)
 	}
 }
 
+static ulong ZRAM_FLAG_SHIFT;
+static ulong ZRAM_FLAG_SAME_BIT;
+
 static void
 zram_init(void)
 {
+	long zram_flag_shift;
+
 	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_flag_shift))
+		;
+	else if (THIS_KERNEL_VERSION >= LINUX(6,1,0))
+		zram_flag_shift = PAGESHIFT() + 1;
+	else
+		zram_flag_shift = 24;
+
+	ZRAM_FLAG_SHIFT = 1 << zram_flag_shift;
+	ZRAM_FLAG_SAME_BIT = 1 << (zram_flag_shift+1);
+
+	if (CRASHDEBUG(1))
+		fprintf(fp, "zram_flag_shift: %lu\n", zram_flag_shift);
 }
 
 static unsigned char *
@@ -2761,9 +2782,11 @@ 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];
+	ulong zs_magic;
 
 	readmem(handle, KVADDR, &obj, sizeof(void *), "zram entry", FAULT_ON_ERROR);
 	obj >>= OBJ_TAG_BITS;
@@ -2772,11 +2795,19 @@ 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);
 
-	class_idx = zspage_s.class;
-	if (zspage_s.magic != ZSPAGE_MAGIC)
-		error(FATAL, "zspage magic incorrect: %x\n", zspage_s.magic);
+	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;
+	} else {
+		readmem(zspage, KVADDR, &zspage_s, sizeof(struct zspage), "zspage", FAULT_ON_ERROR);
+		class_idx = zspage_s.class;
+		zs_magic = zspage_s.magic;
+	}
+
+	if (zs_magic != ZSPAGE_MAGIC)
+		error(FATAL, "zspage magic incorrect: %x\n", zs_magic);
 
 	class = pool + OFFSET(zspoll_size_class);
 	class += (class_idx * sizeof(void *));
@@ -2794,8 +2825,13 @@ zram_object_addr(ulong pool, ulong handle, unsigned char *zram_buf)
 	}
 
 	pages[0] = page;
-	readmem(page + OFFSET(page_freelist), KVADDR, &pages[1],
-			sizeof(void *), "page_freelist", FAULT_ON_ERROR);
+	if (VALID_MEMBER(page_freelist))
+		readmem(page + OFFSET(page_freelist), KVADDR, &pages[1],
+				sizeof(void *), "page_freelist", FAULT_ON_ERROR);
+	else
+		readmem(page + OFFSET(page_index), KVADDR, &pages[1],
+				sizeof(void *), "page_index", FAULT_ON_ERROR);
+
 	sizes[0] = PAGESIZE() - off;
 	sizes[1] = size - sizes[0];
 	if (!is_page_ptr(pages[0], &paddr)) {
@@ -2812,9 +2848,13 @@ zram_object_addr(ulong pool, ulong handle, unsigned char *zram_buf)
 	readmem(paddr, PHYSADDR, zram_buf + sizes[0], sizes[1], "zram buffer[1]", FAULT_ON_ERROR);
 
 out:
-	readmem(page, KVADDR, &obj, sizeof(void *), "page flags", FAULT_ON_ERROR);
-	if (!(obj & (1<<10))) { //PG_OwnerPriv1 flag
-		return (zram_buf + ZS_HANDLE_SIZE);
+	if (VALID_MEMBER(zspage_huge)) {
+		if (!zspage_5_17_s.huge)
+			return (zram_buf + ZS_HANDLE_SIZE);
+	} else {
+		readmem(page, KVADDR, &obj, sizeof(void *), "page flags", FAULT_ON_ERROR);
+		if (!(obj & (1<<10))) // PG_OwnerPriv1 flag
+			return (zram_buf + ZS_HANDLE_SIZE);
 	}
 
 	return zram_buf;
@@ -2993,7 +3033,12 @@ try_zram_decompress(ulonglong pte_val, unsigned char *buf, ulong len, ulonglong
 	readmem(zram_table_entry + OFFSET(zram_table_flag), KVADDR, &flags,
 		sizeof(void *), "zram_table_flag", FAULT_ON_ERROR);
 	if (!entry || (flags & ZRAM_FLAG_SAME_BIT)) {
-		memset(buf, entry, len);
+		ulong *same_buf = (ulong *)GETBUF(PAGESIZE());
+		for (int count = 0; count < PAGESIZE() / sizeof(ulong); count++) {
+			same_buf[count] = entry;
+		}
+		memcpy(buf, same_buf + off, len);
+		FREEBUF(same_buf);
 		goto out;
 	}
 	size = flags & (ZRAM_FLAG_SHIFT -1);
-- 
2.31.1

--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki

[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux