Re: [PATCH] mm/zswap: Remove zswap_pools_counter

[Yosry Ahmed <yosryahmed@google.com>]

On Mon, Jan 13, 2025 at 7:21 AM Joshua Hahn <joshua.hahnjy@gmail.com> wrote:
>
> Hi Yosry,
> I hope you've had a great start to 2025! (Is it too late to still be doing
> New Years greetings? : -))

It's never too late, hope you had a great start too :)

>
> On Fri, 10 Jan 2025 13:53:06 -0800 Yosry Ahmed <yosryahmed@google.com> wrote:
>
> > On Fri, Jan 10, 2025 at 12:33 PM Joshua Hahn <joshua.hahnjy@gmail.com> wrote:
> > >
> > > Commit 8edc9c4 [1] reduced the number of pools used by zswap from 32 to 1.
> >
> > When referring to commits we use the first 12 characters in the SHA1
> > and the commit subject, rather than the link (although adding the link
> > as well does not hurt). So this should be 'Commit 8edc9c4e72fe
> > ("mm/zswap: use only one pool in zswap")'.
>
> Thank you for letting me know. I grabbed the 6-character commit-ID from
> Github, but I'll be sure to include the first 12 characters instead in
> the future.
>
> > > As such, we no longer need to have unique names for zpool (zsmalloc).
> >
> > More importantly, I don't think this is accurate. zswap_pools_count
> > was introduced by commit 32a4e1690399 ("mm/zswap: provide unique zpool
> > name") long before we increased the number of concurrent zpools to 32.
> > It is needed because even though we used to have a single zpool per
> > zswap_pool (as we returned to doing after [1]), we may have multiple
> > zswap_pool's (e.g. if the compressor is changed, a new zswap_pool is
> > created).
>
> Ah, thank you for the correction. In retrospect, this was a bit of a
> naitve patch, it had completely passed my mind that we can still in fact
> have multiple pools at the same time. This could have been avoided if I had
> taken a look at where this code was introduced, rather than the patch
> that (I thought) removed the last users.

It is a bit confusing that we have zswap pools with pools inside of
them to be fair :)