Re: (2) [PATCH 1/4] zstd: pass pointer rathen than structure to functions

[Nick Terrell]

> On Jun 6, 2019, at 7:10 AM, Vaneet Narang <v.narang@xxxxxxxxxxx> wrote:
> 
> Hi Andrew / David,
> 
>  
>>>  > > -        ZSTD_parameters params = ZSTD_getParams(level, src_len, 0);
>>>  > > +        static ZSTD_parameters params;
>>>  > 
>>>  > > +
>>>  > > +        params = ZSTD_getParams(level, src_len, 0);
>>>  > 
>>>  > No thats' broken, the params can't be static as it depends on level and
>>>  > src_len. What happens if there are several requests in parallel with
>>>  > eg. different levels?
> 
> There is no need to make static for btrfs. We can keep it as a stack variable.
> This patch set  focussed on reducing stack usage of zstd compression when triggered
> through zram. ZRAM internally uses crypto and currently crpto uses fixed level and also
> not dependent upon source length.

Can we measure the performance of these patches on btrfs and/or zram? See the benchmarks
I ran on my original patch to btrfs for reference https://lore.kernel.org/patchwork/patch/802866/.

I don't expect a speed difference, but I think it is worth measuring.

> crypto/zstd.c:  
> static ZSTD_parameters zstd_params(void)
> {
>        return ZSTD_getParams(ZSTD_DEF_LEVEL, 0, 0);
> }
> 
> 
> Actually high stack usage problem with zstd compression patch gets exploited more incase of 
> shrink path which gets triggered randomly from any call flow in case of low memory and adds overhead
> of more than 2000 byte of stack and results in stack overflow.
> 
> Stack usage of alloc_page in case of low memory
> 
>   72   HUF_compressWeights_wksp+0x140/0x200  
>   64   HUF_writeCTable_wksp+0xdc/0x1c8      
>   88   HUF_compress4X_repeat+0x214/0x450     
>  208   ZSTD_compressBlock_internal+0x224/0x137c
>  136   ZSTD_compressContinue_internal+0x210/0x3b0
>  192   ZSTD_compressCCtx+0x6c/0x144
>  144   zstd_compress+0x40/0x58
>   32   crypto_compress+0x2c/0x34
>   32   zcomp_compress+0x3c/0x44
>   80   zram_bvec_rw+0x2f8/0xa7c
>   64   zram_rw_page+0x104/0x170
>   48   bdev_write_page+0x80/0xb4
>  112   __swap_writepage+0x160/0x29c
>   24   swap_writepage+0x3c/0x58
>  160   shrink_page_list+0x788/0xae0
>  128   shrink_inactive_list+0x210/0x4a8
>  184   shrink_zone+0x53c/0x7c0
>  160   try_to_free_pages+0x2fc/0x7cc
>   80   __alloc_pages_nodemask+0x534/0x91c
> 
> Thanks & Regards,
> Vaneet Narang 
>