Re: [PATCH] mm/zswap: Remove zswap_pools_counter

[Joshua Hahn <joshua.hahnjy@gmail.com>]

Hi Yosry,
I hope you've had a great start to 2025! (Is it too late to still be doing
New Years greetings? : -))

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.

Thank you for the review as always! Have a great day,
Joshua