10:15 p.m.
Hi,Dave
I modified RETURN_ON_ERROR to FAULT_ON_ERROR in all readmem,patch has attached.
The reason for adding "decompress success" is that i want indicate that zram
address is different from the normal address,and calling zram_decompress not only readmem
failures,but also decompression failures etc.i add "This page has swaped to
zram"/"zram decompress success" as the signature log,and these log only
printed when pc->debug >= 2.
When gcore read UVA address,there may be many page fault happened,we can only deal with
zram swap,"not mapped" and other types of swap will not be processed.For debug
request,we may need the signature log for zram.
so i think it's better to keep this log.
Thanks.
________________________________________
From: Dave Anderson <anderson(a)redhat.com>
Sent: Wednesday, April 8, 2020 0:44
To: 赵乾利
Cc: d hatayama; Discussion list for crash utility usage, maintenance and development
Subject: Re: 答复: [External Mail]Re: [Crash-utility] zram decompress support for
gcore/crash-utility
----- Original Message -----
Hi,Dave
The latest patch have been attached,please check
Thanks.
In diskdump.c, I should have noticed this before, but there a fundamental
problem with *all* of your readmem() calls, because you qualify each of them
with RETURN_ON_ERROR -- and then just continue on presuming that they were
successful. If RETURN_ON_ERROR is specified, you need to check the return
value of readmem(), and then proceed appropriately.
However, as far as I can tell, all of your readmem() calls should be FAULT_ON_ERROR.
That way, if the readmem() call fails, you will see the error message using your
supplied "type" string, and the command will be aborted immediately.
And speaking of your readmem() error message type strings, please remove
the "readmem " part of the strings. For example,
+ readmem(page + OFFSET(page_private), KVADDR, &zspage,
+ sizeof(void *), "readmem page_private",
RETURN_ON_ERROR);
+ readmem(zspage, KVADDR, &zspage_s, sizeof(struct zspage), "readmem
zspage", RETURN_ON_ERROR);
Just make them "page_private", "zspage", etc.
And re-thinking memory.c, there really is no point in displaying the
"zram decompress success" message. If try_zram_compress() fails,
you will see the reason why it failed based upon the error message
generated by the failing embedded/secondary readmem() call. If it
works OK, you will see the requested data. Why bother to show "success"?
+ cnt = try_zram_decompress(paddr, (unsigned char
*)bufptr, cnt, PAGEOFFSET(addr));
+ if (cnt) {
+ if (CRASHDEBUG(2))
+ error(INFO, "0x%lx zram
decompress success\n", addr);
+ bufptr += cnt;
+ addr += cnt;
+ size -= cnt;
+ continue;
+ }
Thanks,
Dave
________________________________________
From: Dave Anderson <anderson(a)redhat.com>
Sent: Monday, April 6, 2020 22:38
To: 赵乾利
Cc: d hatayama; Discussion list for crash utility usage, maintenance and
development
Subject: Re: 答复: [External Mail]Re: [Crash-utility] zram decompress support
for gcore/crash-utility
----- Original Message -----
> patch update
> Found a better way translate pfn to page,PTOB.
> Besides,fix a issue with low probability of decompression failure
>
The changes to defs.h and memory.c look good. My comments for diskdump.c
are interspersed below, where many of them are redundant:
+#ifdef LZO
+static unsigned char *zram_object_addr(ulong pool, ulong handle, unsigned
char *zram_buf)
+{
+ ulong obj, off, class, page, zspage;
+ struct zspage zspage_s;
+ physaddr_t paddr;
+ unsigned int obj_idx, class_idx, size;
+ ulong pages[2], sizes[2];
+
+ readmem(handle, KVADDR, &obj, sizeof(void *), "readmem address",
RETURN_ON_ERROR);
Can you make the error message type string something more helpful than
"readmem address"?
+ obj >>= OBJ_TAG_BITS;
+ phys_to_page(PTOB(obj >> OBJ_INDEX_BITS), &page);
+ obj_idx = (obj & OBJ_INDEX_MASK);
+
+ readmem(page + OFFSET(page_private), KVADDR, &zspage,
+ sizeof(void *), "readmem address", RETURN_ON_ERROR);
Can you make the error message type string something more helpful than
"readmem address"?
+ readmem(zspage, KVADDR, &zspage_s, sizeof(struct zspage), "readmem
address",RETURN_ON_ERROR);
Can you make the error message type string something more helpful than
"readmem address"?
+
+ class_idx = zspage_s.class;
+ if (zspage_s.magic != ZSPAGE_MAGIC)
+ error(FATAL, "zspage magic incorrect:0x%x\n",
zspage_s.magic);
+
+ class = pool + OFFSET(zspoll_size_class);
+ class += (class_idx * sizeof(void *));
+ readmem(class, KVADDR, &class, sizeof(void *), "readmem
address",RETURN_ON_ERROR);
Can you make the error message type string something more helpful than
"readmem address"?
+ readmem(class + OFFSET(size_class_size), KVADDR,
+ &size, sizeof(unsigned int), "readmem address",
RETURN_ON_ERROR);
Can you make the error message type string something more helpful than
"readmem address"?
+ off = (size * obj_idx) & (~machdep->pagemask);
+ if (off + size <= PAGESIZE()) {
+ if (!is_page_ptr(page, &paddr)) {
+ error(WARNING, "zspage not a page pointer:%lx\n",
page);
+ return NULL;
+ }
+ readmem(paddr + off, PHYSADDR, zram_buf, size, "readmem zram
buffer", RETURN_ON_ERROR);
+ goto out;
+ }
+
+ pages[0] = page;
+ readmem(page + OFFSET(page_freelist), KVADDR, &pages[1],
+ sizeof(void *), "readmem address",RETURN_ON_ERROR);
Can you make the error message type string something more helpful than
"readmem address"?
+ sizes[0] = PAGESIZE() - off;
+ sizes[1] = size - sizes[0];+ readmem(page + OFFSET(page_private), KVADDR,
&zspage,
+ sizeof(void *), "readmem page_private",
RETURN_ON_ERROR);
+ readmem(zspage, KVADDR, &zspage_s, sizeof(struct zspage), "readmem
zspage", RETURN_ON_ERROR);+ readmem(page + OFFSET(page_private), KVADDR,
&zspage,
+ sizeof(void *), "readmem page_private",
RETURN_ON_ERROR);
+ readmem(zspage, KVADDR, &zspage_s, sizeof(struct zspage), "readmem
zspage", RETURN_ON_ERROR);
+ if (!is_page_ptr(pages[0], &paddr)) {
+ error(WARNING, "pages[0] not a page pointer\n");
Maybe display the bogus value in pages[0] in the message?
+ return NULL;
+ }
+
+ readmem(paddr + off, PHYSADDR, zram_buf, sizes[0], "readmem zram
buffer", RETURN_ON_ERROR);
+ if (!is_page_ptr(pages[1], &paddr)) {
+ error(WARNING, "pages[1] not a page pointer\n");
Maybe display the bogus value in pages[1] in the message?
+ return NULL;
+ }
+
+ readmem(paddr, PHYSADDR, zram_buf + sizes[0], sizes[1], "readmem zram
buffer", RETURN_ON_ERROR);
+
+out:
+ readmem(page, KVADDR, &obj, sizeof(void *), "readmem
address",RETURN_ON_ERROR);
Can you make the error message type string something more helpful than
"readmem address"?
+ if (!(obj & (1<<10))) { //PG_OwnerPriv1 flag+ readmem(page +
OFFSET(page_private), KVADDR, &zspage,
+ sizeof(void *), "readmem page_private",
RETURN_ON_ERROR);
+ readmem(zspage, KVADDR, &zspage_s, sizeof(struct zspage), "readmem
zspage", RETURN_ON_ERROR);+ readmem(page + OFFSET(page_private), KVADDR,
&zspage,
+ sizeof(void *), "readmem page_private",
RETURN_ON_ERROR);
+ readmem(zspage, KVADDR, &zspage_s, sizeof(struct zspage), "readmem
zspage", RETURN_ON_ERROR);
+ return (zram_buf + ZS_HANDLE_SIZE);
+ }+ readmem(page + OFFSET(page_private), KVADDR, &zspage,
+ sizeof(void *), "readmem page_private",
RETURN_ON_ERROR);
+ readmem(zspage, KVADDR, &zspage_s, sizeof(struct zspage), "readmem
zspage", RETURN_ON_ERROR);+ readmem(page + OFFSET(page_private), KVADDR,
&zspage,
+ sizeof(void *), "readmem page_private",
RETURN_ON_ERROR);
+ readmem(zspage, KVADDR, &zspage_s, sizeof(struct zspage), "readmem
zspage", RETURN_ON_ERROR);+ readmem(page + OFFSET(page_private), KVADDR,
&zspage,
+ sizeof(void *), "readmem page_private",
RETURN_ON_ERROR);
+ readmem(zspage, KVADDR, &zspage_s, sizeof(struct zspage), "readmem
zspage", RETURN_ON_ERROR);
+
+ return zram_buf;
+}
+
+static unsigned char *lookup_swap_cache(ulong pte_val, unsigned char
*zram_buf)
+{
+ ulong swp_type, swp_offset, swp_space;
+ struct list_pair lp;
+ physaddr_t paddr;
+ swp_type = __swp_type(pte_val);
+ if (THIS_KERNEL_VERSION >= LINUX(2,6,0)) {
+ swp_offset = (ulonglong)__swp_offset(pte_val);
+ } else {
+ swp_offset = (ulonglong)SWP_OFFSET(pte_val);
+ }
+
+ if (!symbol_exists("swapper_spaces"))
+ return NULL;
+ swp_space = symbol_value("swapper_spaces");
+ swp_space += swp_type * sizeof(void *);
+
+ readmem(swp_space, KVADDR, &swp_space, sizeof(void *),
+ "readmem address",RETURN_ON_ERROR);
Can you make the error message type string something more helpful than
"readmem address"?
+ swp_space += (swp_offset >> SWAP_ADDRESS_SPACE_SHIFT) *
SIZE(address_space);
+
+ lp.index = swp_offset;
+ if (do_radix_tree(swp_space, RADIX_TREE_SEARCH, &lp)){+ readmem(page +
OFFSET(page_private), KVADDR, &zspage,
+ sizeof(void *), "readmem page_private",
RETURN_ON_ERROR);
+ readmem(zspage, KVADDR, &zspage_s, sizeof(struct zspage), "readmem
zspage", RETURN_ON_ERROR);
+ fprintf(fp, "Find page in swap cache\n");
I don't think you meant to leave this message, right?
+ if (!is_page_ptr((ulong)lp.value, &paddr)) {
+ error(WARNING, "radix page not a page pointer\n");
+ return NULL;
+ }
+ readmem(paddr, PHYSADDR, zram_buf, PAGESIZE(), "readmem zram
buffer", RETURN_ON_ERROR);
+ return zram_buf;
+ }
+ return NULL;
+}
+
+ulong (*decompressor)(unsigned char *in_addr, ulong in_size, unsigned char
*out_addr, ulong *out_size, void *other/* NOT USED */);
+/*
+try_zram_decompress returns decompressed page data and data length
+If userspace address was swaped out to zram,call this function to decompress
this object.
+*/
+ulong try_zram_decompress(ulong pte_val, unsigned char *buf, ulong len,
uint32_t off)
+{
+ char name[32] = {0};
+ ulonglong swp_offset;
+ ulong swap_info, bdev, bd_disk, zram, zram_table_entry, sector,
index, entry, flags, size, outsize;
+ unsigned char *obj_addr = NULL;
+ unsigned char *zram_buf = NULL;
+ unsigned char *outbuf = NULL;+ readmem(page + OFFSET(page_private), KVADDR,
&zspage,
+ sizeof(void *), "readmem page_private",
RETURN_ON_ERROR);
+ readmem(zspage, KVADDR, &zspage_s, sizeof(struct zspage), "readmem
zspage", RETURN_ON_ERROR);
+
+
+ if (!symbol_exists("swap_info"))
+ return 0;
+
+ swap_info = symbol_value("swap_info");
+
+ swap_info_init();
+ if (vt->flags & SWAPINFO_V2) {
+ swap_info += (__swp_type(pte_val) * sizeof(void *));
+ readmem(swap_info, KVADDR, &swap_info,
+ sizeof(void *), "readmem address",
RETURN_ON_ERROR);
Can you make the error message type string something more helpful than
"readmem address"?
+ } else {
+ swap_info += (SIZE(swap_info_struct) * __swp_type(pte_val));
+ }
+
+ readmem(swap_info + OFFSET(swap_info_struct_bdev), KVADDR, &bdev,
+ sizeof(void *), "read swap_info_struct_bdev",
RETURN_ON_ERROR);
+ readmem(bdev + OFFSET(block_device_bd_disk), KVADDR, &bd_disk,
+ sizeof(void *), "read block_device_bd_disk",
RETURN_ON_ERROR);
+ readmem(bd_disk + OFFSET(gendisk_disk_name), KVADDR, name,
+ strlen("zram"), "read gendisk_disk_name",
RETURN_ON_ERROR);
+ if (!strncmp(name, "zram", strlen("zram"))) {
+ if (CRASHDEBUG(2))
+ error(WARNING, "This page has swaped to zram\n");
+
+ readmem(bd_disk + OFFSET(gendisk_private_data), KVADDR,
&zram,
+ sizeof(void *), "gendisk_private_data",
RETURN_ON_ERROR);
+
+ readmem(zram + OFFSET(zram_compressor), KVADDR, name,
+ sizeof(name), "zram compressor", RETURN_ON_ERROR);
+ if (!strncmp(name, "lzo", strlen("lzo"))) {
+ if (!(dd->flags & LZO_SUPPORTED)) {
+ if (lzo_init() == LZO_E_OK)
+ dd->flags |= LZO_SUPPORTED;
+ else
+ return 0;
+ }
+ decompressor = (void *)lzo1x_decompress_safe;+ readmem(page
+ OFFSET(page_private), KVADDR, &zspage,
+ sizeof(void *), "readmem page_private",
RETURN_ON_ERROR);
+ readmem(zspage, KVADDR, &zspage_s, sizeof(struct zspage), "readmem
zspage", RETURN_ON_ERROR);+ readmem(page + OFFSET(page_private), KVADDR,
&zspage,
+ sizeof(void *), "readmem page_private",
RETURN_ON_ERROR);
+ readmem(zspage, KVADDR, &zspage_s, sizeof(struct zspage), "readmem
zspage", RETURN_ON_ERROR);
+ } else {//todo,support more compressor
+ error(WARNING, "Only support lzo compressor\n");
+ 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 ((zram_buf = (unsigned char*)malloc(PAGESIZE())) == NULL)
+ error(FATAL, "cannot malloc zram_buf space.");
Never use malloc/free for temporary buffers during runtime. Either use
GETBUF()/FREEBUF()
or just put it on the stack.
+
+ /*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;
+ }
+
+ sector = swp_offset << (PAGESHIFT() - 9);
+ index = sector >> SECTORS_PER_PAGE_SHIFT;
+ readmem(zram, KVADDR, &zram_table_entry,
+ sizeof(void *), "readmem address",
RETURN_ON_ERROR);
Can you make the error message type string something more helpful than
"readmem address"?
+ zram_table_entry += (index * SIZE(zram_table_entry));
+ readmem(zram_table_entry, KVADDR, &entry,
+ sizeof(void *), "readmem address",
RETURN_ON_ERROR);
Can you make the error message type string something more helpful than
"readmem address"?
+ readmem(zram_table_entry + OFFSET(zram_table_flag), KVADDR,
&flags,
+ sizeof(void *), "readmem address",
RETURN_ON_ERROR);
Can you make the error message type string something more helpful than
"readmem address"?
+ 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 *), "readmem address",
RETURN_ON_ERROR);
Can you make the error message type string something more helpful than
"readmem address"?
+
+ 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 {
+ if ((outbuf = (unsigned char*)malloc(PAGESIZE())) ==
NULL)
+ error(FATAL, "cannot malloc outbuf space.");
Never use malloc/free for temporary buffers during runtime. Either use
GETBUF()/FREEBUF()
or just put it on the stack.
+
+ if (!decompressor(obj_addr, size, outbuf, &outsize,
NULL))
+ memcpy(buf, outbuf + off, len);
+ else {
+ error(WARNING, "zram decompress error\n");
+ len = 0;
+ }
+ free(outbuf);
+ }
+ }
+
+out:
+ free(zram_buf);+ readmem(page + OFFSET(page_private), KVADDR, &zspage,
+ sizeof(void *), "readmem page_private",
RETURN_ON_ERROR);
+ readmem(zspage, KVADDR, &zspage_s, sizeof(struct zspage), "readmem
zspage", RETURN_ON_ERROR);
+ return len;
+}
+#else
+ulong try_zram_decompress(ulong pte_val, unsigned char *buf, ulong len,
uint32_t off)
+{
+ return 0;
+}
+#endif
#/******本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！
This e-mail and its attachments contain confidential information from
XIAOMI, which is intended only for the person or entity whose address is
listed above. Any use of the information contained herein in any way
(including, but not limited to, total or partial disclosure, reproduction,
or dissemination) by persons other than the intended recipient(s) is
prohibited. If you receive this e-mail in error, please notify the sender by
phone or email immediately and delete it!******/#
#/******本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！
This e-mail and its attachments contain confidential information from XIAOMI, which is
intended only for the person or entity whose address is listed above. Any use of the
information contained herein in any way (including, but not limited to, total or partial
disclosure, reproduction, or dissemination) by persons other than the intended
recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender
by phone or email immediately and delete it!******/#