From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id E16DAC48260 for ; Fri, 16 Feb 2024 10:10:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 72D888D0006; Fri, 16 Feb 2024 05:10:19 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 6DB378D0001; Fri, 16 Feb 2024 05:10:19 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5A3268D0006; Fri, 16 Feb 2024 05:10:19 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 4BAE68D0001 for ; Fri, 16 Feb 2024 05:10:19 -0500 (EST) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 1A804404B1 for ; Fri, 16 Feb 2024 10:10:19 +0000 (UTC) X-FDA: 81797246958.23.90296EE Received: from mail-ua1-f54.google.com (mail-ua1-f54.google.com [209.85.222.54]) by imf17.hostedemail.com (Postfix) with ESMTP id 2D41440017 for ; Fri, 16 Feb 2024 10:10:16 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="A7wA/GPU"; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf17.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.54 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1708078217; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=5JkzbFOHD1LBq/kZlIZ36OV9INmft/mRs0RusCRScXg=; b=i/JPZZuj/1fQPji5+iX7vMIUX5yxbTSwM3qcEIfL/ixPX/gSpuJlIDpFwaSb8BKB1iqwN9 urVaSJ7+87filku26USvFa4VUf8y+6q7zYF011gbgwaaDpe+Z+0Ua+AHw8ukSZ1hP4KtMK oz4P6fOH3v8NGFtysNMHWFXCGTdrLtQ= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="A7wA/GPU"; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf17.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.54 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1708078217; a=rsa-sha256; cv=none; b=NYeIFdayCLkN6QquAIJTEPU/emTOzdsADkgeJFNs2ebX+tme2kKWvNm5htTdG2Shsf48/y SFxybsj5IZ8jZlieqI7N4rb+qWsUuDskgiy3HWkizPZK9OKByqUpHxFCX20pjGCYcbeCOJ /zlF7HQr8mfJ7BtZT242koqZ1YSCydw= Received: by mail-ua1-f54.google.com with SMTP id a1e0cc1a2514c-7d5bfdd2366so807152241.3 for ; Fri, 16 Feb 2024 02:10:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1708078216; x=1708683016; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=5JkzbFOHD1LBq/kZlIZ36OV9INmft/mRs0RusCRScXg=; b=A7wA/GPUmR6u/dm0wH61fqMs9fPHwQNuUZvdipkGf284Q1/0X2/T7Fqp/NxncAQV8J p3E39m0RKfBtntgBAz4ibbnd3e2L+sHgXrYRP9dVljnvN3JSnaToBEt9sNnqqwIl+cMT Z5UQVvGtpXmvYrtSeBn3QCgstuGeSwlkm1b7YRc90Tca2P1x5IzC9CLmaJAbeZAeMrgp 577czTPWYmchY61Cj5qzrkCjVBj0RGsxqWlh+SP/SyLA9Pilg0sVwX7zaUNFO4WJ5lWf Ukya1KXKWFam7do0Cp5lcThR5CoLSSREv0xDb13c+hadoyMtD8/vnG9cj5JRO3iWMfpW zhzg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708078216; x=1708683016; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=5JkzbFOHD1LBq/kZlIZ36OV9INmft/mRs0RusCRScXg=; b=qjaUEdhN4A3aPwxKF5xuLYyB6XiSl/1Gdw5FUMor85hit/NAxteYJ9NUC0WYROFLLl PXlVQQFutUdJ8NXoSKgs+gjtg0OwkZK8lEu3yFWmpyzrVmzjIruTPY9CmGlfu3qCtREx Z58w2F1q7lYtus8SSlJjilabaclfjdoFHICI7ytQRHFIdGsoj6yhMZGJKW0syH6YOhYe 1e5drninukE8WPAnHidW0YVNikulVzPB3URCSWZGPVBfMWJfm2SixNzrYiaj5rqnyEU4 aP8Nhww+rRjbVDwWX6AWqlJHHJ+9AsduIm1nGgZq3tEaUs1O+uNvmfAA2B86Pu2pVcQH FpWg== X-Forwarded-Encrypted: i=1; AJvYcCWz7gge3PXPrUtNWy2lTs4h4NJQr7kW5VDEA9xWoVjW7hvIdKBVU+EYBb0JJux2ovWYhuphhNf3mNoWmLnXgqawLLA= X-Gm-Message-State: AOJu0Yxd2015IwI+Hr0MrALOieDuO09seTIu32RznvWquq0pzaTAHLbs URlA6TsESa6G9EefkG407XzkIcNIQcIrnI5nmITGEM73ud96lkpqxps5qbBt2VsH25yoOADNh1S /4nZZsBJZQbz3P2boHAGLqKIn0NE= X-Google-Smtp-Source: AGHT+IH1dSaXgm/Qiz9YTbYv81wbn+JFDDceA34zHrKDgZ/8eZ+F38uzQOKuRFzuO+hj41Yo+z3nNHAe2WBoNUtB3c0= X-Received: by 2002:a1f:4b07:0:b0:4c0:285e:79a with SMTP id y7-20020a1f4b07000000b004c0285e079amr3929989vka.3.1708078216129; Fri, 16 Feb 2024 02:10:16 -0800 (PST) MIME-Version: 1.0 References: <20240216040815.114202-1-21cnbao@gmail.com> <20240216040815.114202-3-21cnbao@gmail.com> In-Reply-To: From: Barry Song <21cnbao@gmail.com> Date: Fri, 16 Feb 2024 23:10:04 +1300 Message-ID: Subject: Re: [PATCH v2 2/3] mm/zswap: remove the memcpy if acomp is not sleepable To: Yosry Ahmed Cc: akpm@linux-foundation.org, davem@davemloft.net, hannes@cmpxchg.org, herbert@gondor.apana.org.au, linux-crypto@vger.kernel.org, linux-mm@kvack.org, nphamcs@gmail.com, zhouchengming@bytedance.com, chriscli@google.com, chrisl@kernel.org, ddstreet@ieee.org, linux-kernel@vger.kernel.org, sjenning@redhat.com, vitaly.wool@konsulko.com, Barry Song Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 2D41440017 X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: 83x19q68g5shcexsn14j71b9osfgz48a X-HE-Tag: 1708078216-646500 X-HE-Meta: U2FsdGVkX18qSvVEce/OoXtFoZVG3lFx+lOaN7IUmt3QzHl/QtApClnF6BvJii2Y6R7c6EepJi2xrUgYfW/DIw4uUTGnuoyhq+giyE121BQyObpOMLLdrwdEYOfrt+nB2LI1nQZPDQSqSNejN2e3P3zXlJBbjFx/VzLj/5glEqNT25daQ72mDgkV2fOZtcYogmnMV/QPQl8APyuRROOiV4cnkyu8sd5zp648oB+Xf4lwwHCSBOj+guUZAG72KSZyb4YABXa2yW3rQLABkuSymekIT5bHppZjiSRwYB9Osf/u8vDr8VRfuCKHXXTREUyM9L/3Rmrn/ivwkmGNnfBJw4O9ASsxJPezbNDfPE2X3KKhxIJMbsJ5XJxYVMz7cso2HqLmjId9KoR+P5hts3hVRqpnypcYmAdTtMzjT1HHd2RjizEsAGP0pWF1FRrfLGTJ/7oVZtZVniLWBcRiXVC9m6KBT1LtZakmxvvt1j5xYepPQ5lMI4pHEJ7OehHmGdclPI0LG4WEfBRKlbCVJX1u5JF67sDd4z6cG9h+L147pkC97zwDZ1I2F9vm6F/UJHIcf6OkuqxkO//iy71U9XHpkCokD00IwOJqLByl2+eCbmCMUnkvIsg/BCGvPoUGvXh1lbjTM7B7CYJPZrYCVm5yCsEHTXjrr6SmgHnJ+jufqlZpcy9SW1IqHyWYGE6TjfiFauTvWvzwnypgLi9xBfdbdw3RpmwOw/7Mrj+PHTOwAe+1CBLTj/Fi11YoAYne7rdFaaxk1PbT6EYYJqSfikdgSocdobLc5ZQ2W0/HXBwFMkBsFetic7KO56cqpaz1xOhxvdPXp+WIHOWpdzSm6tNaW/M9kBxF3t8S1YgKA3QAzdsRcNJvleeJ+6zl292/TfkHNws3wmF9ZYXmIRoaBRgs0zdQaoQM5cgZ4uFkiQLlFpTNXEqmpNCQ6HhteGEJ3FQDeN0nL7yocOkpDmNGpXg CfvZ+GGB SV6113cthJGtQLc81woDNHa5P/FYjpbyMF8NyBeHVJyuPZtc6oqROThQnmzZ631Ck9NXCJsKFYlmzFWUmuw6SUU3dOpABrz/qxMUizrqQ/nPC+PPi5/3aA8mZlxt8ErkphbOpvmcX9LFoP6gBmlyPe+FwBKlZ3y87V1Cuq7x/9/u7EFl4De7fiVrGsTogq63mKAkWzBZ5csLkKPE70eF4O2/Udd1d4Fdum2ufrIxn+X+bk4q2Lc1QIVG68ws9x42N9WDx49KNvC/D+PrJUyh+nF0fFhq5AdARYEgM0/hWt4ZwN0LnOPZVkNihSsMVMNqVv4khNT7WCv3VtD8gljBzU3ZnmpdUL10OsGmQPfsYE/og69XvURDy3rJxaRPy3yyfLW2Khrp81DNzCkvk+W/tRGilvAWBT0J+KFm9eOgzRvUeVOxu9EJYa/Ts/CJdnsM56X1lVD4ZXxbGvgOvJcvrDeD6I7FAN0xZrQtw6w7QdNfuk8bYcjqS4/p5ZGvIiJFqp8mknUOeNnGBt4Q= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Fri, Feb 16, 2024 at 9:30=E2=80=AFPM Yosry Ahmed = wrote: > > On Fri, Feb 16, 2024 at 05:08:14PM +1300, Barry Song wrote: > > From: Barry Song > > > > Most compressors are actually CPU-based and won't sleep during > > compression and decompression. We should remove the redundant > > memcpy for them. > > > > Signed-off-by: Barry Song > > Tested-by: Chengming Zhou > > Reviewed-by: Nhat Pham > > --- > > mm/zswap.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/mm/zswap.c b/mm/zswap.c > > index 350dd2fc8159..6319d2281020 100644 > > --- a/mm/zswap.c > > +++ b/mm/zswap.c > > @@ -168,6 +168,7 @@ struct crypto_acomp_ctx { > > struct crypto_wait wait; > > u8 *buffer; > > struct mutex mutex; > > + bool is_sleepable; > > }; > > > > /* > > @@ -716,6 +717,7 @@ static int zswap_cpu_comp_prepare(unsigned int cpu,= struct hlist_node *node) > > goto acomp_fail; > > } > > acomp_ctx->acomp =3D acomp; > > + acomp_ctx->is_sleepable =3D acomp_is_sleepable(acomp); > > Just one question here. In patch 1, sleepable seems to mean "not async". > IIUC, even a synchronous algorithm may sleep (e.g. if there is a > cond_resched or waiting for a mutex). Does sleepable in acomp terms the > same as "atomic" in scheduling/preemption terms? I think the answer is yes though async and sleepable are slightly different semantically generally speaking. but for comp cases, they are equal. We have two backends for compression/ decompression - scomp and acomp. if c= omp is using scomp backend, we can safely think they are not sleepable at least from the below three facts. 1. in zRAM, we are using scomp APIs only - crypto_comp_decompress()/ crypto_comp_compress(), which are definitely scomp, we have never consider= ed sleeping problem in zram drivers: static int zram_read_from_zspool(struct zram *zram, struct page *page, u32 index) { struct zcomp_strm *zstrm; unsigned long handle; unsigned int size; void *src, *dst; u32 prio; int ret; handle =3D zram_get_handle(zram, index); ... src =3D zs_map_object(zram->mem_pool, handle, ZS_MM_RO); if (size =3D=3D PAGE_SIZE) { dst =3D kmap_local_page(page); memcpy(dst, src, PAGE_SIZE); kunmap_local(dst); ret =3D 0; } else { dst =3D kmap_local_page(page); ret =3D zcomp_decompress(zstrm, src, size, dst); kunmap_local(dst); zcomp_stream_put(zram->comps[prio]); } zs_unmap_object(zram->mem_pool, handle); return ret; } 2. zswap used to only support scomp before we moved to use crypto_acomp_compress() and crypto_acomp_decompress() APIs whose backends can be either scomp or acomp, thus new hardware-based compression drivers can be used in zswap. But before we moved to these new APIs in commit 1ec3b5fe6eec782 ("mm/zswap= : move to use crypto_acomp API for hardware acceleration") , zswap had never considered sleeping problems just like zRAM. 3. There is no sleeping in drivers using scomp backend. $ git grep crypto_register_scomp crypto/842.c: ret =3D crypto_register_scomp(&scomp); crypto/deflate.c: ret =3D crypto_register_scomp(&scomp); crypto/lz4.c: ret =3D crypto_register_scomp(&scomp); crypto/lz4hc.c: ret =3D crypto_register_scomp(&scomp); crypto/lzo-rle.c: ret =3D crypto_register_scomp(&scomp); crypto/lzo.c: ret =3D crypto_register_scomp(&scomp); crypto/zstd.c: ret =3D crypto_register_scomp(&scomp); drivers/crypto/cavium/zip/zip_main.c: ret =3D crypto_register_scomp(&zip_scomp_deflate); drivers/crypto/cavium/zip/zip_main.c: ret =3D crypto_register_scomp(&zip_scomp_lzs); which are the most common cases. > > Also, was this tested with debug options to catch any possible sleeps in > atomic context? yes. i have enabled CONFIG_DEBUG_ATOMIC_SLEEP=3Dy. > > If the answer to both questions is yes, the change otherwise LGTM. Feel > free to add: > Acked-by: Yosry Ahmed Thanks! > > Thanks! > > > > > req =3D acomp_request_alloc(acomp_ctx->acomp); > > if (!req) { > > @@ -1368,7 +1370,7 @@ static void __zswap_load(struct zswap_entry *entr= y, struct page *page) > > mutex_lock(&acomp_ctx->mutex); > > > > src =3D zpool_map_handle(zpool, entry->handle, ZPOOL_MM_RO); > > - if (!zpool_can_sleep_mapped(zpool)) { > > + if (acomp_ctx->is_sleepable && !zpool_can_sleep_mapped(zpool)) { > > memcpy(acomp_ctx->buffer, src, entry->length); > > src =3D acomp_ctx->buffer; > > zpool_unmap_handle(zpool, entry->handle); > > @@ -1382,7 +1384,7 @@ static void __zswap_load(struct zswap_entry *entr= y, struct page *page) > > BUG_ON(acomp_ctx->req->dlen !=3D PAGE_SIZE); > > mutex_unlock(&acomp_ctx->mutex); > > > > - if (zpool_can_sleep_mapped(zpool)) > > + if (!acomp_ctx->is_sleepable || zpool_can_sleep_mapped(zpool)) > > zpool_unmap_handle(zpool, entry->handle); > > } > > > > -- > > 2.34.1 > > Thanks Barry