linux-kernel - Re: [PATCH] mm/zswap: Remove zswap_pools

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAJD7tkaxvinAEQheSay+6+mo8UGFCjdtRGiVDadXC3S_xaKHFg@mail.gmail.com>
Date: Mon, 13 Jan 2025 08:20:51 -0800
From: Yosry Ahmed <yosryahmed@...gle.com>
To: Joshua Hahn <joshua.hahnjy@...il.com>
Cc: Johannes Weiner <hannes@...xchg.org>, Chengming Zhou <chengming.zhou@...ux.dev>, 
	Andrew Morton <akpm@...ux-foundation.org>, Nhat Pham <nphamcs@...il.com>, 
	linux-kernel@...r.kernel.org, linux-mm@...ck.org, kernel-team@...a.com
Subject: Re: [PATCH] mm/zswap: Remove zswap_pools_counter

On Mon, Jan 13, 2025 at 7:21 AM Joshua Hahn <joshua.hahnjy@...il.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@...gle.com> wrote:
>
> > On Fri, Jan 10, 2025 at 12:33 PM Joshua Hahn <joshua.hahnjy@...il.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 :)