Received-SPF: pass (google.com: best guess record for domain of linux-crypto-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Nick Terrell <terrelln@fb.com>
To:     Vaneet Narang <v.narang@samsung.com>
CC:     Andrew Morton <akpm@linux-foundation.org>,
        "dsterba@suse.cz" <dsterba@suse.cz>,
        Maninder Singh <maninder1.s@samsung.com>,
        "herbert@gondor.apana.org.au" <herbert@gondor.apana.org.au>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "keescook@chromium.org" <keescook@chromium.org>,
        "gustavo@embeddedor.com" <gustavo@embeddedor.com>,
        "joe@perches.com" <joe@perches.com>,
        "linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        AMIT SAHRAWAT <a.sahrawat@samsung.com>,
        PANKAJ MISHRA <pankaj.m@samsung.com>, Chris Mason <clm@fb.com>,
        Josef Bacik <josef@toxicpanda.com>,
        David Sterba <dsterba@suse.com>,
        "linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>
Subject: Re: (2) [PATCH 1/4] zstd: pass pointer rathen than structure to
 functions
Thread-Topic: (2) [PATCH 1/4] zstd: pass pointer rathen than structure to
 functions
Thread-Index: AQHVHKR2hpo9a2qp3EKL1BeVjcuANQ==
Date:   Thu, 6 Jun 2019 20:14:36 +0000
Message-ID: <673B6F9E-5BB7-4EB2-9E6C-A44E09367ADE@fb.com>
References: <20190605143219.248ca514546f69946aa2e07e@linux-foundation.org>
 <1559552526-4317-1-git-send-email-maninder1.s@samsung.com>
 <1559552526-4317-2-git-send-email-maninder1.s@samsung.com>
 <20190604154326.8868a10f896c148a0ce804d1@linux-foundation.org>
 <20190605115703.GY15290@twin.jikos.cz> <20190605123253.GZ15290@suse.cz>
 <CGME20190603090232epcas5p1630d0584e8a1aa9495edc819605664fc@epcms5p1>
 <20190606141019epcms5p1e9c394d2c2ef37506c8004fe48edd29f@epcms5p1>
In-Reply-To: <20190606141019epcms5p1e9c394d2c2ef37506c8004fe48edd29f@epcms5p1>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: fb.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-ID: <F6236908BE37D04F92F6F255008B1A8C@namprd15.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: e05348c2-aaad-448b-6171-08d6eabb98d7
X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Jun 2019 20:14:36.1471
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: terrelln@fb.com
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW2PR1501MB2076
X-OriginatorOrg: fb.com
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2019-06-06_14:,,
 signatures=0
X-Proofpoint-Spam-Details: rule=fb_default_notspam policy=fb_default score=0 priorityscore=1501
 malwarescore=0 suspectscore=0 phishscore=0 bulkscore=0 spamscore=0
 clxscore=1011 lowpriorityscore=0 mlxscore=0 impostorscore=0
 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx
 scancount=1 engine=8.0.1-1810050000 definitions=main-1906060136
X-FB-Internal: deliver
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-crypto.vger.kernel.org>
X-Mailing-List: linux-crypto@vger.kernel.org


> On Jun 6, 2019, at 7:10 AM, Vaneet Narang <v.narang@samsung.com> wrote:
>=20
> Hi Andrew / David,
>=20
> =20
>>>  > > -        ZSTD_parameters params =3D ZSTD_getParams(level, src_len,=
 0);
>>>  > > +        static ZSTD_parameters params;
>>>  >=20
>>>  > > +
>>>  > > +        params =3D ZSTD_getParams(level, src_len, 0);
>>>  >=20
>>>  > 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?
>=20
> There is no need to make static for btrfs. We can keep it as a stack vari=
able.
> 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 t=
he benchmarks
I ran on my original patch to btrfs for reference https://lore.kernel.org/p=
atchwork/patch/802866/.

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

> crypto/zstd.c: =20
> static ZSTD_parameters zstd_params(void)
> {
>        return ZSTD_getParams(ZSTD_DEF_LEVEL, 0, 0);
> }
>=20
>=20
> Actually high stack usage problem with zstd compression patch gets exploi=
ted more incase of=20
> shrink path which gets triggered randomly from any call flow in case of l=
ow memory and adds overhead
> of more than 2000 byte of stack and results in stack overflow.
>=20
> Stack usage of alloc_page in case of low memory
>=20
>   72   HUF_compressWeights_wksp+0x140/0x200 =20
>   64   HUF_writeCTable_wksp+0xdc/0x1c8     =20
>   88   HUF_compress4X_repeat+0x214/0x450    =20
>  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
>=20
> Thanks & Regards,
> Vaneet Narang=20
> =20