Re: [PATCH 1/2] crypto: zstd - Avoid redundant cstream initialization

[Nick Terrell]

On Tue, Aug 25, 2026 at 6:06 PM Usama Arif <usama.arif@xxxxxxxxx> 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@xxxxxxxxx>

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!

> ---
>  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
>