Re: [PATCH 1/2] crypto: zstd - Avoid redundant cstream initialization
From: Nick Terrell <hidden>
Date: 2026-09-10 22:59:13
Also in:
lkml
On Tue, Aug 25, 2026 at 6:06 PM Usama Arif [off-list ref] wrote:
zstd_compress() initializes the shared workspace as a CStream before entering the walk loop. If the first source and destination fragments each span the whole request it then hands off to zstd_compress_one(), which initializes that same ctx->wksp as a CCtx, discarding the CStream setup without having compressed a byte. zswap always takes this one-shot path when storing, so every page it stores paid for both. Neither is cheap: zstd_init_cstream() redoes the cwksp layout, zeroes the ZSTD_CCtx, probes for BMI2 through ZSTD_cpuid(), then resets the session and parameters and replays ten validated ZSTD_CCtx_setParameter() calls. Defer the CStream initialization to the first walk iteration that needs it, guarded by a flag because that iteration can be reached more than once. The first inner iteration either takes the one-shot path and returns or initializes the CStream, so the trailing zstd_end_stream() cannot pick up the stale context left in ctx->cctx by an earlier request. Unlike the old call site the new one runs with the walk's fragments mapped, so it has to release them before failing. For a 4 KB crypto_acomp benchmark for compression, twelve runs of nine 30K operation rounds, on the bare-metal host the median per-round mean request time fell from 52,283 ns to 51,038 ns (2.4%). In the one-vCPU KVM guest it fell from 16,675 ns to 15,050 ns (9.8%). The larger improvement in guest is because of the pair of CPUID instructions in ZSTD_cpuid() that the removed initialization runs. Signed-off-by: Usama Arif <usama.arif@linux.dev>
This optimization is a clear win, and makes sense to apply even after the BMI2 patches in zstd, as it also removes other initialization work such as zeroing buffers. Thanks for finding & fixing this!
quoted hunk ↗ jump to hunk
--- crypto/zstd.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)diff --git a/crypto/zstd.c b/crypto/zstd.c index 556f5d2bdd5fb..d64472f3e11e5 100644 --- a/crypto/zstd.c +++ b/crypto/zstd.c@@ -96,6 +96,7 @@ static int zstd_compress_one(struct acomp_req *req, struct zstd_ctx *ctx, static int zstd_compress(struct acomp_req *req) { + bool stream_initialized = false; struct crypto_acomp_stream *s; unsigned int pos, scur, dcur; unsigned int total_out = 0;@@ -115,12 +116,6 @@ static int zstd_compress(struct acomp_req *req) if (ret) goto out; - ctx->cctx = zstd_init_cstream(&ctx->params, 0, ctx->wksp, ctx->wksp_size); - if (!ctx->cctx) { - ret = -EINVAL; - goto out; - } - do { dcur = acomp_walk_next_dst(&walk); if (!dcur) {@@ -142,6 +137,19 @@ static int zstd_compress(struct acomp_req *req) goto out; } + if (!stream_initialized) { + ctx->cctx = zstd_init_cstream(&ctx->params, 0, + ctx->wksp, ctx->wksp_size); + if (!ctx->cctx) { + /* Release in the reverse of the map order. */ + acomp_walk_done_src(&walk, 0); + acomp_walk_done_dst(&walk, 0); + ret = -EINVAL; + goto out;
The other error paths, e.g. when ZSTD_isError() is true below, don't call acomp_walk_done_src() or acomp_walk_done_dst(). The difference is that they are returning -EIO rather than -EINVAL. I don't know enough about the acomp API to know which behavior is correct, but want to flag this as it seems like one of them may be incorrect. The same question applies to the next patch for decompression.
+ }
+ stream_initialized = true;
+ }
+
if (scur) {
inbuf.pos = 0;
inbuf.src = walk.src.virt.addr;
--
2.53.0-Meta