Re: [Crash-utility] 答复: [External Mail]Re: zram decompress support for gcore/crash-utility

2020-04-03 Thread Dave Anderson



- Original Message -
> Hi,Dave
> 
> As per your suggestion, I updated the patch,and compiled successfully on
> arm64, x86,ppc64 architecture
> 

The patch does compile cleanly, although I haven't tried compiling it on an 
s390x, but
I'll do that soon.

A couple issues with the patch:

  +   if (!strncmp(name, "lzo", strlen("lzo"))) {
  +   lzo_init();
  +   decompressor = (void *)lzo1x_decompress_safe;
  +   } else {//todo,support more compressor
  +   error(WARNING, "Only support lzo compressor\n");
  +   return 0;
  +   }

lzo_init() will have already been called by is_diskdump() here during session
initialization if the dumpfile is a compressed kdump:

  #ifdef LZO
  if (lzo_init() == LZO_E_OK)
  dd->flags |= LZO_SUPPORTED;
  #endif

So try_zram_decompress() should check if (dd->flags & LZO_SUPPORTED) has been 
set before
calling lzo_init() again.

And while I don't object to exporting swap_info_init(), I do have a problem 
with pfn_to_map():

  +void swap_info_init(void);
  +ulong pfn_to_map(ulong);

  ...

  +   obj >>= OBJ_TAG_BITS;
  +   page = pfn_to_map(obj >> OBJ_INDEX_BITS);
  +   obj_idx = (obj & OBJ_INDEX_MASK);

The pfn_to_map() function is only relevant if the kernel is configured with 
SPARSEMEM.
I don't see why the exported phys_to_page() function could not be used here?

Thanks,
  Dave

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



Re: [Crash-utility] 答复: [External Mail]Re: zram decompress support for gcore/crash-utility

2020-04-03 Thread 赵乾利
Hi,Dave

As per your suggestion, I updated the patch,and compiled successfully on arm64, 
x86,ppc64 architecture


From: Dave Anderson 
Sent: Friday, April 3, 2020 0:16
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 & hatayama
>
> I made two patchs in crash-utility and gcore to support zram decompress
> 1.In crash-utility,I add patch in readmem to support zram decompression,
> readmem interface automatically recognizes and decompresses zram data.
> There are some limitations to zram support,only support lzo decompress,kernel
> support lzo,lz4,lz4hc,842,zstd,but lzo is default.
>
> use "rd" command also read data even if mapping to zram
> [without patch]
> crash> rd 144fc000 2
> rd: invalid user virtual address: 144fc000  type: "64-bit UVADDR"
> [with patch]
> crash> rd 144fc000 2
> 144fc000:  06ecdc6b06ecb280 06f027f906eebebe   k'..

With respect to the crash utility patch:

Apparently you wrote this patch to only support ARM64?  Here's what happens on 
an x86_64:

  $ patch -p1 < $bos/0001-support-zram-decompress-in-readmem.patch
  patching file defs.h
  Hunk #5 succeeded at 5304 (offset 2 lines).
  patching file memory.c
  $ make warn
  ... [ cut ] ...
  cc -c -g -DX86_64 -DLZO -DSNAPPY -DGDB_7_6  memory.c -Wall -O2 
-Wstrict-prototypes -Wmissing-prototypes -fstack-protector -Wformat-security
  In file included from memory.c:19:0:
  memory.c: In function 'zram_object_addr':
  defs.h:5310:27: error: 'PHYS_MASK_SHIFT' undeclared (first use in this 
function)
   #define _PFN_BITS(PHYS_MASK_SHIFT - PAGESHIFT())
 ^
  defs.h:5311:43: note: in expansion of macro '_PFN_BITS'
   #define OBJ_INDEX_BITS   (BITS_PER_LONG - _PFN_BITS - OBJ_TAG_BITS)
 ^
  memory.c:19838:27: note: in expansion of macro 'OBJ_INDEX_BITS'
page = pfn_to_map(obj >> OBJ_INDEX_BITS);
 ^
  defs.h:5310:27: note: each undeclared identifier is reported only once for 
each function it appears in
   #define _PFN_BITS(PHYS_MASK_SHIFT - PAGESHIFT())
 ^
  defs.h:5311:43: note: in expansion of macro '_PFN_BITS'
   #define OBJ_INDEX_BITS   (BITS_PER_LONG - _PFN_BITS - OBJ_TAG_BITS)
 ^
  memory.c:19838:27: note: in expansion of macro 'OBJ_INDEX_BITS'
page = pfn_to_map(obj >> OBJ_INDEX_BITS);
 ^
  memory.c: In function 'try_zram_decompress':
  memory.c:19940:16: error: 'PTE_VALID' undeclared (first use in this function)
if (pte_val & PTE_VALID)
  ^
  memory.c:19932:8: warning: unused variable 'ret' [-Wunused-variable]
ulong ret = 0;
  ^
  make[4]: *** [memory.o] Error 1
  make[3]: *** [gdb] Error 2
  make[2]: *** [rebuild] Error 2
  make[1]: *** [gdb_merge] Error 2
  make: *** [warn] Error 2
  $

So that's a non-starter.  If it can't be made architecture-neutral, then at
least the other major architectures need to be supported.  At a minimum all
architectures need to be able to be compiled with LZO enabled.

If you can do that, other suggestions I have for the patch are:

 (1) Move all the new offset_table entries to the end of the structure to 
prevent
 the breakage of previously-compiled extension modules that use OFFSET().

 (2) Move the new LZO specific functions to diskdump.c, which is the only C file
 that is set up to deal with LZO being #define'd on the fly with "make lzo".

 (3) Create a dummy try_zram_decompress() function in diskdump.c that just
 returns 0.  Put it outside of the LZO function block, e.g.:

#ifdef LZO
zram_object_addr(args... )
...
lookup_swap_cache(args...)
...
try_zram_decompress(args...)
...
#else
try_zram_decompress(args...) { return 0; }
#endif

 Alternatively, you could create a try_zram_decompress() macro in defs.h 
the same way.

 (4) Remove the #ifdef/#endif LZO section of readmem().

 (5) PLEASE do not make all the white-space changes in memory.c.  It's annoying
 to have to review the patch when it's cluttered with changes that are
 irrelevant to the task at hand.

Thanks,
  Dave






>
> 2.In gcore, I have to make a small change ,change parameter of readmem from
> PHYADDR to UVADDR, other work will be done by crash
>
> Please help review.
> Thanks
>
> -邮件原件-
> 发件人: Dave Anderson 
> 发送时间: 2020年4月2日 0:29
> 收件人: 赵乾利 
> 抄送: d hatayama ; Discussion list for crash utility
> usage, maintenance and development 
> 主题: Re: [External Mail]Re: [Crash-utility] zram decompress support for
> gcore/crash-utility
>
>
>
> - Original Message -
> > Hi,Dave
> > Zram is a virtual device,it simulated as a block device,it's