----- 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@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/crash-utility