Re: [PATCH 0/3] mm: remove zpool

[Vitaly Wool <vitaly.wool@konsulko.se>]

On 9/5/25 20:02, Nhat Pham wrote:
> On Thu, Sep 4, 2025 at 4:49 PM Vitaly Wool <vitaly.wool@konsulko.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@cmpxchg.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 :)

Well, there are 2 main issues with zsmalloc on bigger pages as I see it:
1) the granularity in allocations goes down due to the limited number of 
zsmalloc classes
2) the idea that an object can span 2 pages on a large page system turns 
out to be costly and inefficient.

And if you think about solving these problems you end up with something 
zblock like.

>>
>> 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.

I think migratability is no longer an issue. Compaction will be added 
along the way but it isn't a crucial feature for zblock because its 
fragmentation is harmless in the sense that temporarily empty slots act 
like a cache as opposed to being an unusable free space.

Best regards,
Vitaly