Re: [PATCH 0/2] zswap pool per-CPU acomp_ctx simplifications

[Kanchana P. Sridhar]

On Mon, Mar 16, 2026 at 8:09 AM Yosry Ahmed <yosry@xxxxxxxxxx> wrote:
>
> On Mon, Mar 16, 2026 at 03:06:32PM +0000, Yosry Ahmed wrote:
> > > > > @@ -786,7 +786,7 @@ static int zswap_cpu_comp_prepare(unsigned int cpu, struct hlist_node *node)
> > > > > return ret;
> > > > >
> > > > > acomp_ctx->acomp = crypto_alloc_acomp_node(pool->tfm_name, 0, 0, cpu_to_node(cpu));
> > > > > - if (IS_ERR(acomp_ctx->acomp)) {
> > > > > + if (IS_ERR_OR_NULL(acomp_ctx->acomp)) {
> > > > Does crypto_alloc_acomp_node() ever return NULL?
> > > > Looking at the error handling just below this check, if this were to
> > > > actually return NULL, PTR_ERR(NULL) evaluates to 0. This would cause
> > > > the function to incorrectly return 0 (success) instead of an error code,
> > > > hiding the allocation failure.
> > >
> > > This is a good catch. Just to provide context, this patch was
> > > introduced based on Yosry's earlier comments in [1].
> > >
> > > [1]: https://patchwork.kernel.org/comment/26282128/
> > >
> > > crypto_alloc_acomp_node() currently does not return NULL. However, it
> > > could, in future.
> > > Since the rest of zswap_cpu_comp_prepare() dereferences
> > > acomp_ctx->acomp, it depends on whether we want to future-proof the
> > > code to handle a possible eventuality of crypto_alloc_acomp_node()
> > > returning NULL.
> >
> > Hmm upon revisiting this, I think keeping this as IS_ERR() here is a
> > better documentation for the API, and the incossitency between this code
> > and acomp_ctx_dealloc() is arguably documenting that the function can
> > only return an ERR, but it can also be NULL-initialized by zswap.
>
> Also, sorry for leading you astray in the first place. Looking at this
> initially I thought the inconsistency was confusing, but looking at it
> with fresh eyes I think it better documents the API and the different
> callsites.

No worries.