8:34 p.m.
This patch set is to make changes I found necessary during development
of zram support for crash gcore command.
HATAYAMA Daisuke (5):
diskdump, zram: cleanup try_zram_decompress()
diskdump, zram: initialize zram symbol information when needed
diskname, zram: fix fault error when reading zram disk with no symbol
information
diskname, zram: Notify necessity of loading zram module
memory, zram: introduce and export readswap()
defs.h | 1 +
diskdump.c | 220 ++++++++++++++++++++++++++++++++++++-------------------------
memory.c | 5 +-
3 files changed, 136 insertions(+), 90 deletions(-)
--
1.8.3.1
8:34 p.m.
New subject: [PATCH 1/5] diskdump, zram: cleanup try_zram_decompress()
This clean up makes later commits a bit readable.
Signed-off-by: HATAYAMA Daisuke <d.hatayama(a)fujitsu.com>
---
diskdump.c | 145 ++++++++++++++++++++++++++++++++-----------------------------
1 file changed, 75 insertions(+), 70 deletions(-)
diff --git a/diskdump.c b/diskdump.c
index 63a08a1..dcc7142 100644
--- a/diskdump.c
+++ b/diskdump.c
@@ -2743,85 +2743,90 @@ try_zram_decompress(ulonglong pte_val, unsigned char *buf, ulong
len, ulonglong
sizeof(void *), "block_device_bd_disk", FAULT_ON_ERROR);
readmem(bd_disk + OFFSET(gendisk_disk_name), KVADDR, name,
strlen("zram"), "gendisk_disk_name", FAULT_ON_ERROR);
- if (!strncmp(name, "zram", strlen("zram"))) {
+
+ if (strncmp(name, "zram", strlen("zram"))) {
if (CRASHDEBUG(2))
- error(WARNING, "this page has swapped to zram\n");
-
- readmem(bd_disk + OFFSET(gendisk_private_data), KVADDR, &zram,
- sizeof(void *), "gendisk_private_data", FAULT_ON_ERROR);
-
- readmem(zram + OFFSET(zram_compressor), KVADDR, name,
- sizeof(name), "zram compressor", FAULT_ON_ERROR);
- if (STREQ(name, "lzo")) {
- if (!(dd->flags & LZO_SUPPORTED)) {
- if (lzo_init() == LZO_E_OK)
- dd->flags |= LZO_SUPPORTED;
- else
- return 0;
- }
- decompressor = (void *)lzo1x_decompress_safe;
- } else {//todo,support more compressor
- error(WARNING, "only the lzo compressor is supported\n");
- return 0;
- }
+ error(WARNING,
+ "this page has been swapped to %s\n",
+ name);
+ return 0;
+ }
- if (THIS_KERNEL_VERSION >= LINUX(2, 6, 0)) {
- swp_offset = (ulonglong)__swp_offset(pte_val);
- } else {
- swp_offset = (ulonglong)SWP_OFFSET(pte_val);
- }
+ if (CRASHDEBUG(2))
+ error(WARNING, "this page has swapped to zram\n");
- zram_buf = (unsigned char *)GETBUF(PAGESIZE());
- /*lookup page from swap cache*/
- obj_addr = lookup_swap_cache(pte_val, zram_buf);
- if (obj_addr != NULL) {
- memcpy(buf, obj_addr + off, len);
- goto out;
- }
+ readmem(bd_disk + OFFSET(gendisk_private_data), KVADDR, &zram,
+ sizeof(void *), "gendisk_private_data", FAULT_ON_ERROR);
- sector = swp_offset << (PAGESHIFT() - 9);
- index = sector >> SECTORS_PER_PAGE_SHIFT;
- readmem(zram, KVADDR, &zram_table_entry,
- sizeof(void *), "zram_table_entry", FAULT_ON_ERROR);
- zram_table_entry += (index * SIZE(zram_table_entry));
- readmem(zram_table_entry, KVADDR, &entry,
- sizeof(void *), "entry of table", FAULT_ON_ERROR);
- 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);
- goto out;
- }
- size = flags & (ZRAM_FLAG_SHIFT -1);
- if (size == 0) {
- len = 0;
- goto out;
+ readmem(zram + OFFSET(zram_compressor), KVADDR, name,
+ sizeof(name), "zram compressor", FAULT_ON_ERROR);
+ if (STREQ(name, "lzo")) {
+ if (!(dd->flags & LZO_SUPPORTED)) {
+ if (lzo_init() == LZO_E_OK)
+ dd->flags |= LZO_SUPPORTED;
+ else
+ return 0;
}
+ decompressor = (void *)lzo1x_decompress_safe;
+ } else {//todo,support more compressor
+ error(WARNING, "only the lzo compressor is supported\n");
+ return 0;
+ }
- readmem(zram + OFFSET(zram_mempoll), KVADDR, &zram,
- sizeof(void *), "zram_mempoll", FAULT_ON_ERROR);
+ if (THIS_KERNEL_VERSION >= LINUX(2, 6, 0)) {
+ swp_offset = (ulonglong)__swp_offset(pte_val);
+ } else {
+ swp_offset = (ulonglong)SWP_OFFSET(pte_val);
+ }
- obj_addr = zram_object_addr(zram, entry, zram_buf);
- if (obj_addr == NULL) {
- len = 0;
- goto out;
- }
+ zram_buf = (unsigned char *)GETBUF(PAGESIZE());
+ /*lookup page from swap cache*/
+ obj_addr = lookup_swap_cache(pte_val, zram_buf);
+ if (obj_addr != NULL) {
+ memcpy(buf, obj_addr + off, len);
+ goto out;
+ }
- if (size == PAGESIZE()) {
- memcpy(buf, obj_addr + off, len);
- } else {
- outbuf = (unsigned char *)GETBUF(PAGESIZE());
- outsize = PAGESIZE();
- if (!decompressor(obj_addr, size, outbuf, &outsize, NULL))
- memcpy(buf, outbuf + off, len);
- else {
- error(WARNING, "zram decompress error\n");
- len = 0;
- }
- FREEBUF(outbuf);
- }
+ sector = swp_offset << (PAGESHIFT() - 9);
+ index = sector >> SECTORS_PER_PAGE_SHIFT;
+ readmem(zram, KVADDR, &zram_table_entry,
+ sizeof(void *), "zram_table_entry", FAULT_ON_ERROR);
+ zram_table_entry += (index * SIZE(zram_table_entry));
+ readmem(zram_table_entry, KVADDR, &entry,
+ sizeof(void *), "entry of table", FAULT_ON_ERROR);
+ 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);
+ goto out;
+ }
+ size = flags & (ZRAM_FLAG_SHIFT -1);
+ if (size == 0) {
+ len = 0;
+ goto out;
+ }
+
+ readmem(zram + OFFSET(zram_mempoll), KVADDR, &zram,
+ sizeof(void *), "zram_mempoll", FAULT_ON_ERROR);
+
+ obj_addr = zram_object_addr(zram, entry, zram_buf);
+ if (obj_addr == NULL) {
+ len = 0;
+ goto out;
+ }
+
+ if (size == PAGESIZE()) {
+ memcpy(buf, obj_addr + off, len);
} else {
- return 0;
+ outbuf = (unsigned char *)GETBUF(PAGESIZE());
+ outsize = PAGESIZE();
+ if (!decompressor(obj_addr, size, outbuf, &outsize, NULL))
+ memcpy(buf, outbuf + off, len);
+ else {
+ error(WARNING, "zram decompress error\n");
+ len = 0;
+ }
+ FREEBUF(outbuf);
}
out:
--
1.8.3.1
8:34 p.m.
New subject: [PATCH 2/5] diskdump, zram: initialize zram symbol information when needed
In the current code, symbol information related to zram is initialized
even when a given disk is not zram. It should be done after the disk
turns out to be zram.
Signed-off-by: HATAYAMA Daisuke <d.hatayama(a)fujitsu.com>
---
diskdump.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/diskdump.c b/diskdump.c
index dcc7142..047d8ad 100644
--- a/diskdump.c
+++ b/diskdump.c
@@ -2719,9 +2719,6 @@ try_zram_decompress(ulonglong pte_val, unsigned char *buf, ulong
len, ulonglong
unsigned char *zram_buf = NULL;
unsigned char *outbuf = NULL;
- if (INVALID_MEMBER(zram_compressor))
- zram_init();
-
off = PAGEOFFSET(vaddr);
if (!symbol_exists("swap_info"))
return 0;
@@ -2752,6 +2749,9 @@ try_zram_decompress(ulonglong pte_val, unsigned char *buf, ulong
len, ulonglong
return 0;
}
+ if (INVALID_MEMBER(zram_compressor))
+ zram_init();
+
if (CRASHDEBUG(2))
error(WARNING, "this page has swapped to zram\n");
--
1.8.3.1
8:34 p.m.
New subject: [PATCH 3/5] diskname, zram: fix fault error when reading zram disk with no symbol information
When no zram symbol information is initialized, reading memory swapped
out into zram disk results in fault error as follows:
crash> rd -u 0x00007f1cf6c37000
rd: invalid structure member offset: zram_compressor
FILE: diskdump.c LINE: 2753 FUNCTION: try_zram_decompress()
[./crash] error trace: 47a7b1 => 5766eb => 5401b7 => 540146
540146: OFFSET_verify.part.0+70
5401b7: OFFSET_verify+39
5766eb: try_zram_decompress+635
47a7b1: readmem+273
rd: invalid structure member offset: zram_compressor
FILE: diskdump.c LINE: 2753 FUNCTION: try_zram_decompress()
Before zram support, trying to read memory that is swapped out
resulted in inaccessible error as follows:
crash> rd -u 0x00007f1cf6c37000
rd: invalid user virtual address: 7f1cf6c37000 type: "64-bit UVADDR"
This behavior is problematic for crash gcore command to support zram.
The fault error terminates gcore command and then generating core file
fails; this is regression. On the other hand, in the previous one,
gcore command can continue by writing zero pages.
Signed-off-by: HATAYAMA Daisuke <d.hatayama(a)fujitsu.com>
---
diskdump.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/diskdump.c b/diskdump.c
index 047d8ad..304e9a1 100644
--- a/diskdump.c
+++ b/diskdump.c
@@ -2749,8 +2749,11 @@ try_zram_decompress(ulonglong pte_val, unsigned char *buf, ulong
len, ulonglong
return 0;
}
- if (INVALID_MEMBER(zram_compressor))
+ if (INVALID_MEMBER(zram_compressor)) {
zram_init();
+ if (INVALID_MEMBER(zram_compressor))
+ return 0;
+ }
if (CRASHDEBUG(2))
error(WARNING, "this page has swapped to zram\n");
--
1.8.3.1
8:34 p.m.
New subject: [PATCH 4/5] diskname, zram: Notify necessity of loading zram module
By the previous commit, trying to read swapped-out-into-zram-disk
pages results in inaccessible memory error that is apparently
irrelevant to zram and users cannot find it necessary to load zram
module. Thus, let's add a warning message to indicate that.
Signed-off-by: HATAYAMA Daisuke <d.hatayama(a)fujitsu.com>
---
diskdump.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/diskdump.c b/diskdump.c
index 304e9a1..cdbc2d5 100644
--- a/diskdump.c
+++ b/diskdump.c
@@ -2751,8 +2751,13 @@ try_zram_decompress(ulonglong pte_val, unsigned char *buf, ulong
len, ulonglong
if (INVALID_MEMBER(zram_compressor)) {
zram_init();
- if (INVALID_MEMBER(zram_compressor))
+ if (INVALID_MEMBER(zram_compressor)) {
+ error(WARNING,
+ "Some pages are swapped out to zram. "
+ "Please load zram module.\n",
+ name);
return 0;
+ }
}
if (CRASHDEBUG(2))
--
1.8.3.1
9:03 p.m.
New subject: [PATCH 4/5] diskname, zram: Notify necessity of loading zram module
-----Original Message-----
By the previous commit, trying to read swapped-out-into-zram-disk
pages results in inaccessible memory error that is apparently
irrelevant to zram and users cannot find it necessary to load zram
module. Thus, let's add a warning message to indicate that.
Signed-off-by: HATAYAMA Daisuke <d.hatayama(a)fujitsu.com>
---
diskdump.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/diskdump.c b/diskdump.c
index 304e9a1..cdbc2d5 100644
--- a/diskdump.c
+++ b/diskdump.c
@@ -2751,8 +2751,13 @@ try_zram_decompress(ulonglong pte_val, unsigned char *buf, ulong
len, ulonglong
if (INVALID_MEMBER(zram_compressor)) {
zram_init();
- if (INVALID_MEMBER(zram_compressor))
+ if (INVALID_MEMBER(zram_compressor)) {
+ error(WARNING,
+ "Some pages are swapped out to zram. "
+ "Please load zram module.\n",
+ name);
Hi Hatayama-san,
Only a slight thing from me, this "name" looks unnecessary.
Otherwise, except for the compilation issue, looks good to me.
Thanks!
Kazu
8:34 p.m.
New subject: [PATCH 5/5] memory, zram: introduce and export readswap()
try_zram_decompress() is currently exported to extension modules, but
from a viewpoint of author of extension modules, it's better to export
an interface to read memory on swap; difference of decompressor are
then hidden within the interface and there is no need for extension
modules to update accordingly each time new decompressor are added in
the future.
So let's introduce function readswap() as an interface to read memory
on swap.
Signed-off-by: HATAYAMA Daisuke <d.hatayama(a)fujitsu.com>
---
defs.h | 1 +
diskdump.c | 67 ++++++++++++++++++++++++++++++++++++++++++++------------------
memory.c | 5 ++++-
3 files changed, 53 insertions(+), 20 deletions(-)
diff --git a/defs.h b/defs.h
index c899fe2..8bb5618 100644
--- a/defs.h
+++ b/defs.h
@@ -6506,6 +6506,7 @@ int diskdump_kaslr_check(void);
QEMUCPUState *diskdump_get_qemucpustate(int);
void diskdump_device_dump_info(FILE *);
void diskdump_device_dump_extract(int, char *, FILE *);
+ulong readswap(ulonglong pte_val, char *buf, ulong len, ulonglong vaddr);
/*support for zram*/
ulong try_zram_decompress(ulonglong pte_val, unsigned char *buf, ulong len, ulonglong
vaddr);
#ifdef LZO
diff --git a/diskdump.c b/diskdump.c
index cdbc2d5..3762314 100644
--- a/diskdump.c
+++ b/diskdump.c
@@ -2704,24 +2704,15 @@ lookup_swap_cache(ulonglong pte_val, unsigned char *zram_buf)
return NULL;
}
-ulong (*decompressor)(unsigned char *in_addr, ulong in_size, unsigned char *out_addr,
ulong *out_size, void *other/* NOT USED */);
-/*
- * If userspace address was swapped out to zram, this function is called to decompress
the object.
- * try_zram_decompress returns decompressed page data and data length
- */
-ulong
-try_zram_decompress(ulonglong pte_val, unsigned char *buf, ulong len, ulonglong vaddr)
+static int get_disk_name_private_data(ulonglong pte_val,
+ ulonglong vaddr,
+ char *name,
+ ulong *private_data)
{
- char name[32] = {0};
- ulonglong swp_offset;
- ulong swap_info, bdev, bd_disk, zram, zram_table_entry, sector, index, entry, flags,
size, outsize, off;
- unsigned char *obj_addr = NULL;
- unsigned char *zram_buf = NULL;
- unsigned char *outbuf = NULL;
+ ulong swap_info, bdev, bd_disk;
- off = PAGEOFFSET(vaddr);
if (!symbol_exists("swap_info"))
- return 0;
+ return FALSE;
swap_info = symbol_value("swap_info");
@@ -2738,16 +2729,53 @@ try_zram_decompress(ulonglong pte_val, unsigned char *buf, ulong
len, ulonglong
sizeof(void *), "swap_info_struct_bdev", FAULT_ON_ERROR);
readmem(bdev + OFFSET(block_device_bd_disk), KVADDR, &bd_disk,
sizeof(void *), "block_device_bd_disk", FAULT_ON_ERROR);
- readmem(bd_disk + OFFSET(gendisk_disk_name), KVADDR, name,
+ if (name)
+ readmem(bd_disk + OFFSET(gendisk_disk_name), KVADDR, name,
strlen("zram"), "gendisk_disk_name", FAULT_ON_ERROR);
+ if (private_data)
+ readmem(bd_disk + OFFSET(gendisk_private_data), KVADDR,
+ private_data, sizeof(void *), "gendisk_private_data",
+ FAULT_ON_ERROR);
+
+ return TRUE;
+}
+
+ulong readswap(ulonglong pte_val, char *buf, ulong len, ulonglong vaddr)
+{
+ char name[32] = {0};
- if (strncmp(name, "zram", strlen("zram"))) {
+ if (!get_disk_name_private_data(pte_val, vaddr, name, NULL))
+ return 0;
+
+ if (!strncmp(name, "zram", 4)) {
+ return try_zram_decompress(pte_val,
+ (unsigned char *)buf,
+ len,
+ vaddr);
+ } else {
if (CRASHDEBUG(2))
error(WARNING,
"this page has been swapped to %s\n",
name);
return 0;
}
+}
+
+ulong (*decompressor)(unsigned char *in_addr, ulong in_size, unsigned char *out_addr,
ulong *out_size, void *other/* NOT USED */);
+/*
+ * If userspace address was swapped out to zram, this function is called to decompress
the object.
+ * try_zram_decompress returns decompressed page data and data length
+ */
+ulong
+try_zram_decompress(ulonglong pte_val, unsigned char *buf, ulong len, ulonglong vaddr)
+{
+ char name[32] = {0};
+ ulonglong swp_offset;
+ unsigned char *obj_addr = NULL;
+ unsigned char *zram_buf = NULL;
+ unsigned char *outbuf = NULL;
+ ulong zram, zram_table_entry, sector, index, entry, flags, size,
+ outsize, off;
if (INVALID_MEMBER(zram_compressor)) {
zram_init();
@@ -2763,8 +2791,8 @@ try_zram_decompress(ulonglong pte_val, unsigned char *buf, ulong
len, ulonglong
if (CRASHDEBUG(2))
error(WARNING, "this page has swapped to zram\n");
- readmem(bd_disk + OFFSET(gendisk_private_data), KVADDR, &zram,
- sizeof(void *), "gendisk_private_data", FAULT_ON_ERROR);
+ if (!get_disk_name_private_data(pte_val, vaddr, NULL, &zram))
+ return 0;
readmem(zram + OFFSET(zram_compressor), KVADDR, name,
sizeof(name), "zram compressor", FAULT_ON_ERROR);
@@ -2789,6 +2817,7 @@ try_zram_decompress(ulonglong pte_val, unsigned char *buf, ulong
len, ulonglong
zram_buf = (unsigned char *)GETBUF(PAGESIZE());
/*lookup page from swap cache*/
+ off = PAGEOFFSET(vaddr);
obj_addr = lookup_swap_cache(pte_val, zram_buf);
if (obj_addr != NULL) {
memcpy(buf, obj_addr + off, len);
diff --git a/memory.c b/memory.c
index c951827..44213bb 100644
--- a/memory.c
+++ b/memory.c
@@ -2296,7 +2296,10 @@ readmem(ulonglong addr, int memtype, void *buffer, long size,
if (cnt > size)
cnt = size;
- cnt = try_zram_decompress(paddr, (unsigned char *)bufptr, cnt, addr);
+ cnt = readswap(addr,
+ bufptr,
+ cnt,
+ addr);
if (cnt) {
bufptr += cnt;
addr += cnt;
--
1.8.3.1
6:29 a.m.
Here are tiny script and program I made to test the zram feature:
https://github.com/d-hatayama/some_stuff_for_crash_zram_devel
This may help your reviewing.
Thanks.
HATAYAMA, Daisuke
________________________________________
From: HATAYAMA Daisuke <d.hatayama(a)fujitsu.com>
Sent: Sunday, October 11, 2020 10:34
To: crash-utility(a)redhat.com
Cc: zhaoqianli(a)xiaomi.com; Hatayama, Daisuke/畑山 大輔
Subject: [PATCH 0/5] zram related changes for zram support of crash gcore command
This patch set is to make changes I found necessary during development
of zram support for crash gcore command.
HATAYAMA Daisuke (5):
diskdump, zram: cleanup try_zram_decompress()
diskdump, zram: initialize zram symbol information when needed
diskname, zram: fix fault error when reading zram disk with no symbol
information
diskname, zram: Notify necessity of loading zram module
memory, zram: introduce and export readswap()
defs.h | 1 +
diskdump.c | 220 ++++++++++++++++++++++++++++++++++++-------------------------
memory.c | 5 +-
3 files changed, 136 insertions(+), 90 deletions(-)
--
1.8.3.1
11:52 p.m.
Hi,
Are there any comments for this patch set?
Without this patch set, I'll drop zram feature in the next release of crash gcore
command for now.
Thanks.
HATAYAMA, Daisuke
________________________________________
From: HATAYAMA Daisuke <d.hatayama(a)fujitsu.com>
Sent: Sunday, October 11, 2020 10:34
To: crash-utility(a)redhat.com
Cc: zhaoqianli(a)xiaomi.com; Hatayama, Daisuke/畑山 大輔
Subject: [PATCH 0/5] zram related changes for zram support of crash gcore command
This patch set is to make changes I found necessary during development
of zram support for crash gcore command.
HATAYAMA Daisuke (5):
diskdump, zram: cleanup try_zram_decompress()
diskdump, zram: initialize zram symbol information when needed
diskname, zram: fix fault error when reading zram disk with no symbol
information
diskname, zram: Notify necessity of loading zram module
memory, zram: introduce and export readswap()
defs.h | 1 +
diskdump.c | 220 ++++++++++++++++++++++++++++++++++++-------------------------
memory.c | 5 +-
3 files changed, 136 insertions(+), 90 deletions(-)
--
1.8.3.1
4:15 a.m.
Hi Hatayama-san,
sorry, we've not been able to make a time to review this patchset.
probably I can next week.
Thanks,
Kazu
-----Original Message-----
Hi,
Are there any comments for this patch set?
Without this patch set, I'll drop zram feature in the next release of crash gcore
command for now.
Thanks.
HATAYAMA, Daisuke
________________________________________
From: HATAYAMA Daisuke <d.hatayama(a)fujitsu.com>
Sent: Sunday, October 11, 2020 10:34
To: crash-utility(a)redhat.com
Cc: zhaoqianli(a)xiaomi.com; Hatayama, Daisuke/畑山 大輔
Subject: [PATCH 0/5] zram related changes for zram support of crash gcore command
This patch set is to make changes I found necessary during development
of zram support for crash gcore command.
HATAYAMA Daisuke (5):
diskdump, zram: cleanup try_zram_decompress()
diskdump, zram: initialize zram symbol information when needed
diskname, zram: fix fault error when reading zram disk with no symbol
information
diskname, zram: Notify necessity of loading zram module
memory, zram: introduce and export readswap()
defs.h | 1 +
diskdump.c | 220 ++++++++++++++++++++++++++++++++++++-------------------------
memory.c | 5 +-
3 files changed, 136 insertions(+), 90 deletions(-)
--
1.8.3.1
--
Crash-utility mailing list
Crash-utility(a)redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
1485
days inactive
1507
days old
devel@lists.crash-utility.osci.io
9 comments
3 participants
participants (3)
-
d.hatayama＠fujitsu.com -
HAGIO KAZUHITO(萩尾 一仁) -
HATAYAMA Daisuke