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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 6 Jun 2019 20:14:36 +0000
From:   Nick Terrell <terrelln@...com>
To:     Vaneet Narang <v.narang@...sung.com>
CC:     Andrew Morton <akpm@...ux-foundation.org>,
        "dsterba@...e.cz" <dsterba@...e.cz>,
        Maninder Singh <maninder1.s@...sung.com>,
        "herbert@...dor.apana.org.au" <herbert@...dor.apana.org.au>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "keescook@...omium.org" <keescook@...omium.org>,
        "gustavo@...eddedor.com" <gustavo@...eddedor.com>,
        "joe@...ches.com" <joe@...ches.com>,
        "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        AMIT SAHRAWAT <a.sahrawat@...sung.com>,
        PANKAJ MISHRA <pankaj.m@...sung.com>, Chris Mason <clm@...com>,
        Josef Bacik <josef@...icpanda.com>,
        David Sterba <dsterba@...e.com>,
        "linux-btrfs@...r.kernel.org" <linux-btrfs@...r.kernel.org>
Subject: Re: (2) [PATCH 1/4] zstd: pass pointer rathen than structure to
 functions



> On Jun 6, 2019, at 7:10 AM, Vaneet Narang <v.narang@...sung.com> 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 
>