----- 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];
+       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
+               return (zram_buf + ZS_HANDLE_SIZE);
+       }
+
+       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)){
+               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;
+
+
+       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;
+               } 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);
+       return len;
+}
+#else
+ulong try_zram_decompress(ulong pte_val, unsigned char *buf, ulong len, 
uint32_t off)
+{
+       return 0;
+}
+#endif

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Reply via email to