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 73FA9C02194 for ; Thu, 6 Feb 2025 19:10:49 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DE10E280004; Thu, 6 Feb 2025 14:10:48 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D9130280002; Thu, 6 Feb 2025 14:10:48 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C5893280004; Thu, 6 Feb 2025 14:10:48 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id A7633280002 for ; Thu, 6 Feb 2025 14:10:48 -0500 (EST) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 5EF35161431 for ; Thu, 6 Feb 2025 19:10:48 +0000 (UTC) X-FDA: 83090461776.07.5CF168A Received: from out-189.mta0.migadu.com (out-189.mta0.migadu.com [91.218.175.189]) by imf30.hostedemail.com (Postfix) with ESMTP id 8AD148000B for ; Thu, 6 Feb 2025 19:10:46 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=mUJyifr9; spf=pass (imf30.hostedemail.com: domain of yosry.ahmed@linux.dev designates 91.218.175.189 as permitted sender) smtp.mailfrom=yosry.ahmed@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1738869046; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=HNnqh3naAnkb0jziaoULO1jiPfvp8Y4Xgu79YXRNMZE=; b=Ssd5Gnjl8bDVa36KOYasHuB3xL6RF0pcX0K+Yn+13wiNuFCzRp4QvbBphtUl+UxrKXOM5K 2iDanOy6819khltX55Kk8gIeKKZ6jJlqpCxz9wtXtmNF/opnEQITxuereGgtdJmzD4LdaM iP46zcJmn98J4HU1yMXgFGP42ltyBro= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=mUJyifr9; spf=pass (imf30.hostedemail.com: domain of yosry.ahmed@linux.dev designates 91.218.175.189 as permitted sender) smtp.mailfrom=yosry.ahmed@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1738869046; a=rsa-sha256; cv=none; b=KKGMqDWpBzGVASztD58Pm5pwI1RnFJDbkRqKWKPDBO5Axf3cgnPaA5k14jqbAFFvMBc8nU CycjMWOgN6VtxoiZlnN69JpDXafYqsWK16l0sywnXWAnv7Yy2kX4xPw0VLNIAFsDFW1Tm/ AJ+8ZeN0qVfOQfNdIwzRmCYTiKpqmSM= Date: Thu, 6 Feb 2025 19:10:33 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1738869039; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=HNnqh3naAnkb0jziaoULO1jiPfvp8Y4Xgu79YXRNMZE=; b=mUJyifr91pvsdyORJCqUrMLdn9rnWMZw6lXLOfaMoTsHV4qUiLmBeFLXWEv5J/7navMYOs Taq2/r0F4PhCkKPZjOuHRja7jjAFphwuPyzf7Bmj9FfUPZ0ZrkaHfH968/M1opw/9GpdqK 83CDmSdZ08bw5WrX5hZt1M4WSKn11Lg= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Yosry Ahmed To: Kanchana P Sridhar Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, hannes@cmpxchg.org, nphamcs@gmail.com, chengming.zhou@linux.dev, usamaarif642@gmail.com, ryan.roberts@arm.com, 21cnbao@gmail.com, akpm@linux-foundation.org, linux-crypto@vger.kernel.org, herbert@gondor.apana.org.au, davem@davemloft.net, clabbe@baylibre.com, ardb@kernel.org, ebiggers@google.com, surenb@google.com, kristen.c.accardi@intel.com, wajdi.k.feghali@intel.com, vinodh.gopal@intel.com Subject: Re: [PATCH v6 15/16] mm: zswap: Compress batching with Intel IAA in zswap_store() of large folios. Message-ID: References: <20250206072102.29045-1-kanchana.p.sridhar@intel.com> <20250206072102.29045-16-kanchana.p.sridhar@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250206072102.29045-16-kanchana.p.sridhar@intel.com> X-Migadu-Flow: FLOW_OUT X-Rspamd-Queue-Id: 8AD148000B X-Stat-Signature: zr3ybij4qhcnewrdwru3zkeu1mtr4qdp X-Rspam-User: X-Rspamd-Server: rspam01 X-HE-Tag: 1738869046-887149 X-HE-Meta: U2FsdGVkX19pXRgmJvIpIklU97CXQ6jZvjkkQCWht9ORrakNBOA0d/Ams3NJeLeGkQVkXEWGYGfbzayTchQ49Fp/ZPEttNIcGZM0INNq4dstGxag648TV2wyJUBV3eFIrSnOPXbci9YcHz+DvPmeh+j3vggS70//ZwTHRQ/rI+ExDhoTIUkt4tc6RKOJnbRln6zb3+Rp7hL9tpjSQLh6X82HnxzO5LigjpJjrB+VFnu4KglCOwp0kyrli/Hq10KSkVZetB6Rv6YLmLy2BLoAt7kIx6oP2lZ9X1gwuK0gcAFSrC+Wn+hTzZuqX006OiEy+f3OazSiLR/QcSbrKu6h+gZivop+9oi6IFfDAa/Zoea1DVcXYjuKC33/zjwnMRuU8M1XGTv8YX70T3Upa+Jj9Nb1pvCjSDxca0oTkpCYgghMX8C0n8uU8yUrdZImZjfHlUd/RP5wDdFd1C410nXDXkzYjclMRo10OdLi7uSG0P3Gfqt15YN9sx7f9LILJWzPGyAIHz7SkdExkByR3KwbTsuafvQwiCSVbNARt7j7AeVhkp9tWnGYtNX+lSUF6ETLTbGPVSxkupifAkoSo1a2pqmcDe1iTHFGzecICQ+GD47a6ERxkMhCVFwpfCPEvwtfVIAkLnh21N2si37B1DJbuSRTtP8WWklMCLCQSqquCMaMZ7bbOGTLSW1mR5SyKe5ddC79Hs8Z9MS98VqKmLNdiuW5Zo78ZfB+GcbKIHKAJKe0J5CwQmTDA+Ms7ddzh3GHcw4MtAqYfip/hs0gbrFvHPwo2rerXa1CjgzBv0x/IkgxX1SLABf5ljNUCNGOBXQUuI3puSmKJpaMAMEmi74yK+0FvPbGtsC1zrn8uONX6ZN/G1cOYGB7CZBMlE3hX1gZPeU4XU7C8e9p3mEWLOSiER+b2GsAhZevMmk6uPDXeF6wwg0Yug3hJdNwQWA4zhKwraqpTgoYwsWk7w5Rb22 RcDMKnPa h/Bu5uGjEU41u8qRQQids1xQ+qbfX3zB8uGVY4sZWYM4iAmu7NNAGFc7tZqELY9NaMb/fZjfAiSDSFkmbkFg40FFy2EeR3YFvbDRkr4/8YrC/9pfTMVw4wXXv/6RBQjmB3SrU4VsMT0hycWt9mEJpHqktGME2NvfP4AAMcFH+uyjMP7OyMNAcRvAyJXQn5+NJi+x/aKBNPVh7y+s3Wj/Y5EaXYs46/WqDeuh/B/ZnEwtxdjCoNcsGXOcq8hsGBGTxoVOFPZdNliigK3cLRIZDdmvD6LTr90aoH+fiyoltm8cBv29oqv9j8+yxULDV/ftEWDKQ 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 Wed, Feb 05, 2025 at 11:21:01PM -0800, Kanchana P Sridhar wrote: > zswap_compress_folio() is modified to detect if the pool's acomp_ctx has > more than one "nr_reqs", which will be the case if the cpu onlining code > has allocated multiple batching resources in the acomp_ctx. If so, it means > compress batching can be used with a batch-size of "acomp_ctx->nr_reqs". > > If compress batching can be used, zswap_compress_folio() will invoke the > newly added zswap_batch_compress() procedure to compress and store the > folio in batches of "acomp_ctx->nr_reqs" pages. > > With Intel IAA, the iaa_crypto driver will compress each batch of pages in > parallel in hardware. > > Hence, zswap_batch_compress() does the same computes for a batch, as > zswap_compress() does for a page; and returns true if the batch was > successfully compressed/stored, and false otherwise. > > If the pool does not support compress batching, or the folio has only one > page, zswap_compress_folio() calls zswap_compress() for each individual > page in the folio, as before. > > Signed-off-by: Kanchana P Sridhar > --- > mm/zswap.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 113 insertions(+), 9 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index 6563d12e907b..f1cba77eda62 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -985,10 +985,11 @@ static void acomp_ctx_put_unlock(struct crypto_acomp_ctx *acomp_ctx) > mutex_unlock(&acomp_ctx->mutex); > } > > +/* The per-cpu @acomp_ctx mutex should be locked/unlocked in the caller. */ Please use lockdep assertions rather than comments for internal locking rules. > static bool zswap_compress(struct page *page, struct zswap_entry *entry, > - struct zswap_pool *pool) > + struct zswap_pool *pool, > + struct crypto_acomp_ctx *acomp_ctx) > { > - struct crypto_acomp_ctx *acomp_ctx; > struct scatterlist input, output; > int comp_ret = 0, alloc_ret = 0; > unsigned int dlen = PAGE_SIZE; > @@ -998,7 +999,6 @@ static bool zswap_compress(struct page *page, struct zswap_entry *entry, > gfp_t gfp; > u8 *dst; > > - acomp_ctx = acomp_ctx_get_cpu_lock(pool); > dst = acomp_ctx->buffers[0]; > sg_init_table(&input, 1); > sg_set_page(&input, page, PAGE_SIZE, 0); > @@ -1051,7 +1051,6 @@ static bool zswap_compress(struct page *page, struct zswap_entry *entry, > else if (alloc_ret) > zswap_reject_alloc_fail++; > > - acomp_ctx_put_unlock(acomp_ctx); > return comp_ret == 0 && alloc_ret == 0; > } > > @@ -1509,20 +1508,125 @@ static void shrink_worker(struct work_struct *w) > * main API > **********************************/ > > +/* The per-cpu @acomp_ctx mutex should be locked/unlocked in the caller. */ > +static bool zswap_batch_compress(struct folio *folio, > + long index, > + unsigned int batch_size, > + struct zswap_entry *entries[], > + struct zswap_pool *pool, > + struct crypto_acomp_ctx *acomp_ctx) > +{ > + int comp_errors[ZSWAP_MAX_BATCH_SIZE] = { 0 }; > + unsigned int dlens[ZSWAP_MAX_BATCH_SIZE]; > + struct page *pages[ZSWAP_MAX_BATCH_SIZE]; > + unsigned int i, nr_batch_pages; > + bool ret = true; > + > + nr_batch_pages = min((unsigned int)(folio_nr_pages(folio) - index), batch_size); > + > + for (i = 0; i < nr_batch_pages; ++i) { > + pages[i] = folio_page(folio, index + i); > + dlens[i] = PAGE_SIZE; > + } > + > + /* > + * Batch compress @nr_batch_pages. If IAA is the compressor, the > + * hardware will compress @nr_batch_pages in parallel. > + */ Please do not specifically mention IAA in zswap.c, as batching could be supported in the future by other compressors. > + ret = crypto_acomp_batch_compress( > + acomp_ctx->reqs, > + NULL, > + pages, > + acomp_ctx->buffers, > + dlens, > + comp_errors, > + nr_batch_pages); Does crypto_acomp_batch_compress() not require calling crypto_wait_req()? > + > + if (ret) { > + /* > + * All batch pages were successfully compressed. > + * Store the pages in zpool. > + */ > + struct zpool *zpool = pool->zpool; > + gfp_t gfp = __GFP_NORETRY | __GFP_NOWARN | __GFP_KSWAPD_RECLAIM; > + > + if (zpool_malloc_support_movable(zpool)) > + gfp |= __GFP_HIGHMEM | __GFP_MOVABLE; > + > + for (i = 0; i < nr_batch_pages; ++i) { > + unsigned long handle; > + char *buf; > + int err; > + > + err = zpool_malloc(zpool, dlens[i], gfp, &handle); > + > + if (err) { > + if (err == -ENOSPC) > + zswap_reject_compress_poor++; > + else > + zswap_reject_alloc_fail++; > + > + ret = false; > + break; > + } > + > + buf = zpool_map_handle(zpool, handle, ZPOOL_MM_WO); > + memcpy(buf, acomp_ctx->buffers[i], dlens[i]); > + zpool_unmap_handle(zpool, handle); > + > + entries[i]->handle = handle; > + entries[i]->length = dlens[i]; > + } > + } else { > + /* Some batch pages had compression errors. */ > + for (i = 0; i < nr_batch_pages; ++i) { > + if (comp_errors[i]) { > + if (comp_errors[i] == -ENOSPC) > + zswap_reject_compress_poor++; > + else > + zswap_reject_compress_fail++; > + } > + } > + } This function is awfully close to zswap_compress(). It's essentially a vectorized version and uses crypto_acomp_batch_compress() instead of crypto_acomp_compress(). My questions are: - Can we use crypto_acomp_batch_compress() for the non-batched case as well to unify the code? Does it cause any regressions? - If we have to use different compressions APIs, can we at least reuse the rest of the code? We can abstract the compression call into a helper that chooses the appropriate API based on the batch size. The rest should be the same AFAICT. > + > + return ret; > +} > + > static bool zswap_compress_folio(struct folio *folio, > struct zswap_entry *entries[], > struct zswap_pool *pool) > { > long index, nr_pages = folio_nr_pages(folio); > + struct crypto_acomp_ctx *acomp_ctx; > + unsigned int batch_size; > + bool ret = true; > > - for (index = 0; index < nr_pages; ++index) { > - struct page *page = folio_page(folio, index); > + acomp_ctx = acomp_ctx_get_cpu_lock(pool); > + batch_size = acomp_ctx->nr_reqs; > + > + if ((batch_size > 1) && (nr_pages > 1)) { > + for (index = 0; index < nr_pages; index += batch_size) { > + > + if (!zswap_batch_compress(folio, index, batch_size, > + &entries[index], pool, acomp_ctx)) { > + ret = false; > + goto unlock_acomp_ctx; > + } > + } > + } else { > + for (index = 0; index < nr_pages; ++index) { > + struct page *page = folio_page(folio, index); > > - if (!zswap_compress(page, entries[index], pool)) > - return false; > + if (!zswap_compress(page, entries[index], pool, acomp_ctx)) { > + ret = false; > + goto unlock_acomp_ctx; > + } > + } > } > > - return true; > +unlock_acomp_ctx: > + acomp_ctx_put_unlock(acomp_ctx); > + return ret; > } > > /* > -- > 2.27.0 >