Re: [PATCH v8 12/14] mm: zswap: Simplify acomp_ctx resource allocation/deletion and mutex lock usage.

[Yosry Ahmed]

On Tue, Mar 18, 2025 at 09:09:05PM +0000, Sridhar, Kanchana P wrote:
[..]
> > > > >
> > > > >     The only other fallback solution in lieu of the proposed simplification
> > that
> > > > >     I can think of is to keep the lifespan of these resources from pool
> > creation
> > > > >     to deletion, using the CPU hotplug code. Although, it is not totally
> > clear
> > > > >     to me if there is potential for deadlock during pool transitions, as
> > noted
> > > > above.
> > > >
> > > > I am not sure what's the deadlock scenario you're worried about, please
> > > > clarify.
> > >
> > > My apologies: I was referring to triggering direct reclaim during pool
> > creation,
> > > which could, in theory, run into a scenario where the pool switching would
> > > have to wait for reclaim to free up enough memory for the acomp_ctx
> > > resources allocation: this could cause the system to hang, but not a
> > deadlock.
> > > This can happen even today, hence trying to see if a better design is
> > possible.
> > 
> > I think the concern here is unfounded. We shouldn't care about the
> > performance of zswap compressor switching, especially under memory
> > pressure. A lot of things will slow down under memory pressure, and
> > compressor switching should be the least of our concerns.
> 
> Sounds good. It then appears that making the per-cpu acomp_ctx resources'
> lifetime track that of the zswap_pool, is the way to go. These resources
> will be allocated as per the requirements of the compressor, i.e., zswap_pool,
> and will persist through CPU online/offline transitions through the hotplug
> interface. If this plan is acceptable, it appears that acomp_ctx_get_cpu_lock()
> and acomp_ctx_put_unlock() can be replaced with mutex_lock()/unlock() in
> zswap_[de]compress()? I will incorporate these changes in v9 if this sounds Ok.

Sounds good to me. Thanks!