Linux cryptographic layer development
 help / color / mirror / Atom feed
From: Yosry Ahmed <yosry@kernel.org>
To: "Kanchana P. Sridhar" <kanchanapsridhar2026@gmail.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>,
	hannes@cmpxchg.org,  nphamcs@gmail.com, chengming.zhou@linux.dev,
	akpm@linux-foundation.org,  linux-mm@kvack.org,
	davem@davemloft.net, linux-crypto@vger.kernel.org,
	 linux-kernel@vger.kernel.org, rientjes@google.com,
	willy@infradead.org,  senozhatsky@chromium.org
Subject: Re: [PATCH v1 0/8] Batched zswap_store() with compression batching
Date: Mon, 19 Oct 2026 22:50:43 +0000	[thread overview]
Message-ID: <atadE_kZz45KAeuE@google.com> (raw)
In-Reply-To: <CACpmpocZGHpTTOq+xa2bTnS-=gdUa8+E_KKJm41H_dSfbGikYQ@mail.gmail.com>

On Sun, Oct 18, 2026 at 06:35:01PM -0700, Kanchana P. Sridhar wrote:
> On Fri, Oct 16, 2026 at 10:07 PM Herbert Xu <herbert@gondor.apana.org.au> wrote:
> >
> > On Fri, Oct 16, 2026 at 10:02:13PM -0700, Kanchana P. Sridhar wrote:
> > [ .. ]
> >
> > That isn't quite right though.  We need to ensure that it really
> > is a singleton before taking the short-cut.  So you need at
> > least one conditional:
> >
> >         if (likely(req->unit_size == req->slen))
> >                 take short-cut
> 
> I added the check for the singleton req [2]. I hope this looks Ok.
> 
> With the latest crypto_acomp_compress(), the batch compress latency
> improves considerably:
> 
> usemem 10 iterations average throughput:
> 
>    patch 6 (store batching only): 19,119 KB/s
> patch 8 (+ compress batching): 24,076 KB/s (+25.93%)

This is exactly what I mean by the numbers varying too much to draw
any conclusions. I am not sure if it's the VM setup or the usemem test
itself, but previously it was argued that the short-circuit in
crypto_acomp_compress() was ~10% faster than the one in
acomp_do_req_chain() (20,946 vs 19,058 KB/s) due to avoiding a function
call and two conditionals, and now *adding* a conditional improves
performance by 15% (20,946 -> 24,076 KB/s)?

Moreover, for zswap with ZSTD the added check is always true: the batch
size is 1, so slen == unit_size == PAGE_SIZE for every request. The
previous version and this one execute the same code, so the 15%
difference can only be noise. With the stddev you reported (~5.4k KB/s
over 10 iterations), both differences are well within the noise.

I think something is wrong with the performance testing methodology
here. The results are self-contradicting and do not make much sense to
me. Did I miss something?

> 
> bpftrace measurement of zswap compress latency per-page:
> 
> Sequential_zswap_compress  nr_samples     p25     p50      p75
> p95       p99
> =================================================================================
> zswap_compress_ns_per_page  1,722,489  43,947  57,791   88,489
> 130,165   323,961
> 
> Batched_zswap_compress     nr_samples     p25     p50      p75
> p95       p99
> =================================================================================
> zswap_compress_ns_per_page    665,267  41,511  54,915   77,707
> 124,648   213,962
> =================================================================================
>  Latency saving (ns)                   -2,435  -2,876  -10,782
> -5,517  -109,999
> =================================================================================

I measured zswap_store() (not zswap_compress()), to get the per-folio
latency, and it didn't seem to change much beyond noise in my testing. I
also notice that the number of samples is very different between cases.


  parent reply	other threads:[~2026-10-19 22:50 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-08 18:28 [PATCH v1 0/8] Batched zswap_store() with compression batching Kanchana P. Sridhar
2026-10-08 18:28 ` [PATCH v1 1/8] crypto: acomp - Define a unit_size in struct acomp_req to enable batching. mm: zswap: Set the unit size for zswap to PAGE_SIZE Kanchana P. Sridhar
2026-10-08 18:28 ` [PATCH v1 2/8] crypto: acomp - Add bit to indicate segmentation support Kanchana P. Sridhar
2026-10-08 18:28 ` [PATCH v1 3/8] crypto: acomp - Add trivial segmentation wrapper Kanchana P. Sridhar
2026-10-08 18:28 ` [PATCH v1 4/8] crypto: acomp - Add API to get a compression algorithm's batch-size Kanchana P. Sridhar
2026-10-08 18:28 ` [PATCH v1 5/8] mm: zswap: Store the folio's node id in the zswap_entry Kanchana P. Sridhar
2026-10-14  0:18   ` Yosry Ahmed
2026-10-14  1:17     ` Kanchana P. Sridhar
2026-10-14 15:17       ` Yosry Ahmed
2026-10-15 21:30         ` Kanchana P. Sridhar
2026-10-15 21:39           ` Yosry Ahmed
2026-10-15 22:45             ` Kanchana P. Sridhar
2026-10-15 22:48               ` Yosry Ahmed
2026-10-15 22:55                 ` Kanchana P. Sridhar
2026-10-08 18:28 ` [PATCH v1 6/8] mm: zswap: Store large folios in batches Kanchana P. Sridhar
2026-10-08 18:28 ` [PATCH v1 7/8] mm: zswap: Optimally place members of struct zswap_pool, struct zswap_entry Kanchana P. Sridhar
2026-10-14 15:42   ` Yosry Ahmed
2026-10-15 21:48     ` Kanchana P. Sridhar
2026-10-15 22:06       ` Yosry Ahmed
2026-10-15 22:53         ` Kanchana P. Sridhar
2026-10-08 18:28 ` [PATCH v1 8/8] mm: zswap: Batched zswap_compress() for storing large folios in batches Kanchana P. Sridhar
2026-10-14 15:45 ` [PATCH v1 0/8] Batched zswap_store() with compression batching Yosry Ahmed
2026-10-15 22:00   ` Kanchana P. Sridhar
2026-10-15 22:04     ` Yosry Ahmed
2026-10-15 22:51       ` Kanchana P. Sridhar
2026-10-15 22:59         ` Yosry Ahmed
2026-10-15 23:40           ` Kanchana P. Sridhar
2026-10-15 23:44             ` Yosry Ahmed
2026-10-16  3:32               ` Kanchana P. Sridhar
2026-10-16  4:38                 ` Herbert Xu
2026-10-16  4:42                   ` Kanchana P. Sridhar
2026-10-17  3:03                   ` Kanchana P. Sridhar
2026-10-17  4:12                     ` Herbert Xu
2026-10-17  5:02                       ` Kanchana P. Sridhar
2026-10-17  5:07                         ` Herbert Xu
2026-10-17  5:10                           ` Kanchana P. Sridhar
2026-10-19  1:35                           ` Kanchana P. Sridhar
2026-10-19  1:38                             ` Herbert Xu
2026-10-19  1:47                               ` Kanchana P. Sridhar
2026-10-19 22:50                             ` Yosry Ahmed [this message]
2026-10-20  2:13                               ` Kanchana P. Sridhar
2026-10-16 16:14 ` Yosry Ahmed
2026-10-16 19:04   ` Kanchana P. Sridhar
2026-10-16 21:48     ` Yosry Ahmed
2026-10-17  3:53       ` Kanchana P. Sridhar
2026-10-19 22:42         ` Yosry Ahmed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=atadE_kZz45KAeuE@google.com \
    --to=yosry@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=chengming.zhou@linux.dev \
    --cc=davem@davemloft.net \
    --cc=hannes@cmpxchg.org \
    --cc=herbert@gondor.apana.org.au \
    --cc=kanchanapsridhar2026@gmail.com \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=nphamcs@gmail.com \
    --cc=rientjes@google.com \
    --cc=senozhatsky@chromium.org \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox