linux-kernel - Re: [PATCH 0/3] mm: remove zpool

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKEwX=MMNSg-QdA2XdBpzhi_d-o9Pv4OOPR5nqrvUVWRdnnpLA@mail.gmail.com>
Date: Fri, 5 Sep 2025 11:02:29 -0700
From: Nhat Pham <nphamcs@...il.com>
To: Vitaly Wool <vitaly.wool@...sulko.se>
Cc: Vlastimil Babka <vbabka@...e.cz>, hannes@...xchg.org, linux-kernel@...r.kernel.org, 
	linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>, 
	Christoph Hellwig <hch@...radead.org>
Subject: Re: [PATCH 0/3] mm: remove zpool

On Thu, Sep 4, 2025 at 4:49 PM Vitaly Wool <vitaly.wool@...sulko.se> wrote:
>
>
>
> On 9/4/25 12:13, Vlastimil Babka wrote:
> > On 9/4/25 11:33, Vitaly Wool wrote:
> >>> With zswap using zsmalloc directly, there are no more in-tree users of
> >>> this code. Remove it.
> >>>
> >>> Signed-off-by: Johannes Weiner <hannes@...xchg.org>
> >>
> >> Per the previous discussions, this gets a *NACK* from my side. There is
> >> hardly anything _technical_ preventing new in-tree users of zpool API.
> >> zpool API is neutral and well-defined, I don’t see *any* good reason for
> >> it to be phased out.
> >
> > AFAIK it's a policy that unused code should be removed ASAP. And that's the
> > case for zpool after Patch 1, no? It could be different if another user was
> > about to be merged (to avoid unnecessary churn), but that doesn't seem the
> > case for zblock?
>
> For the C implementation of zblock, no. But there's another
> implementation which is in Rust and it's nearing the submission.
>
> > My concern would be if the removal breaks any existing installations relying
> > on zswap. Presumably not as a make oldconfig will produce a config where
> > nothing important is missing, and existing boot options such as
> > "zswap.zpool=" or attempts to write to in the init scripts to
> > "/sys/module/zswap/parameters/zpool" will cause some errors/noise but not
> > prevent booting correctly?
>
> I don't expect heavy breakage but misconfigurations will definitely occur.
> > I mean if we were paranoid and anticipated somebody would break their
> > booting if writing to /sys/module/zswap/parameters/zpool failed, we could
> > keep the file (for a while) and just produce a warning in dmesg that it's
> > deprecated and does nothing?
> >
> >  From Patch 1:
> >
> >> Note that this does not preclude future improvements and experiments
> >> with different allocation strategies. Should it become necessary, it's
> >> possible to provide an alternate implementation for the zsmalloc API,
> >> selectable at compile time. However, zsmalloc is also rather mature
> >> and feature rich, with years of widespread production exposure; it's
> >> encouraged to make incremental improvements rather than fork it.
> >
> > With my history of maintaining the slab allocators I can only support this
> > approach.
>
> There was the time when slab was the best option and it was more mature
> than slub, which is now the best and only option. Thus, the "maturity"
> point is indeed valid but not being backed by anything else it doesn't
> weigh too much. Besides, zsmalloc's production exposure from all I know
> is limited to the 4K page case, and zsmalloc is truly struggling when
> the system is configured for 16K pages.

That doesn't sound unfixable, if I recall our conversation correctly.
Perhaps all of this effort is better off being spent fixing zsmalloc's
inefficiencies :)

>
> Things keep changing, and some of the proven solutions may not be a good
> fit moving forward. While not suggesting that we should have a handful
> of zpool backends just for the sake of variety, I'd like to emphasize
> that there are good reasons to have zblock (especially the Rust one),
> and there are good reasons to keep zsmalloc. That leads to the
> conclusion that zpool should stay.

IMHO, the needs for multiple allocators do not necessitate the zpool API.

The zpool API is only needed if you want to switch the allocators
arbitrarily at runtime. This one is a much harder sell.

We can always add zblock, and select the backend via build options.
Overtime, as zsmalloc improves to acquire zblock's advances, or zblock
implements the missing features (migratability, compaction, etc.), we
can unify/remove one of them.