> 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 >