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

[Yosry Ahmed]

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.