[PATCH v4 0/2] rust: zpool: add abstraction for zpool drivers

Vitaly Wool posted 2 patches 1 month, 1 week ago
There is a newer version of this series
bindings/bindings_helper.h |    1
helpers/helpers.c          |    1
helpers/zpool.c            |    6 +
kernel/alloc.rs            |    5
kernel/lib.rs              |    2
kernel/zpool.rs            |  338 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 353 insertions(+)
[PATCH v4 0/2] rust: zpool: add abstraction for zpool drivers
Posted by Vitaly Wool 1 month, 1 week ago
Zpool is a common frontend for memory storage pool implementations.
These pools are typically used to store compressed memory objects,
e. g. for Zswap, the lightweight compressed cache for swap pages.

This patch provides the interface to use Zpool in Rust kernel code,
thus enabling Rust implementations of Zpool allocators for Zswap.

Co-developed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.se>
---
Changelog:
v1 -> v2:
* reworked to stick to the existing Rust driver infrastructure
* removed raw pointers from the Rust API
v2 -> v3:
* detailed safety requirements for unsafe API functions
* removed unwrap()
* some typo corrections
v3 -> v4:
* added a working example of zpool Rust API usage in the
  documentation part
* change to Flags arranged as a separate patch
* improved safety requirements for ZpoolDriver trait
---
 bindings/bindings_helper.h |    1 
 helpers/helpers.c          |    1 
 helpers/zpool.c            |    6 +
 kernel/alloc.rs            |    5 
 kernel/lib.rs              |    2 
 kernel/zpool.rs            |  338 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 353 insertions(+)
Re: [PATCH v4 0/2] rust: zpool: add abstraction for zpool drivers
Posted by Johannes Weiner 1 month, 1 week ago
On Sat, Aug 23, 2025 at 03:04:19PM +0200, Vitaly Wool wrote:
> Zpool is a common frontend for memory storage pool implementations.
> These pools are typically used to store compressed memory objects,
> e. g. for Zswap, the lightweight compressed cache for swap pages.
> 
> This patch provides the interface to use Zpool in Rust kernel code,
> thus enabling Rust implementations of Zpool allocators for Zswap.

The zpool indirection is on its way out.

When you submitted an alternate allocator backend recently, the
resounding feedback from the zswap maintainers was that improvements
should happen to zsmalloc incrementally. It is a lot of code and has a
lot of features that go beyond allocation strategy. We do not want to
fork it and fragment this space again with niche, incomplete backends.

It's frustrating that you not only ignored this, but then went ahead
and made other people invest their time and effort into this as well.
Re: [PATCH v4 0/2] rust: zpool: add abstraction for zpool drivers
Posted by Vitaly Wool 1 month, 1 week ago

> On Aug 26, 2025, at 2:44 PM, Johannes Weiner <hannes@cmpxchg.org> wrote:
> 
> On Sat, Aug 23, 2025 at 03:04:19PM +0200, Vitaly Wool wrote:
>> Zpool is a common frontend for memory storage pool implementations.
>> These pools are typically used to store compressed memory objects,
>> e. g. for Zswap, the lightweight compressed cache for swap pages.
>> 
>> This patch provides the interface to use Zpool in Rust kernel code,
>> thus enabling Rust implementations of Zpool allocators for Zswap.
> 
> The zpool indirection is on its way out.
> 
> When you submitted an alternate allocator backend recently, the
> resounding feedback from the zswap maintainers was that improvements
> should happen to zsmalloc incrementally. It is a lot of code and has a
> lot of features that go beyond allocation strategy. We do not want to
> fork it and fragment this space again with niche, incomplete backends.
> 
> It's frustrating that you not only ignored this, but then went ahead
> and made other people invest their time and effort into this as well.
> 

I don’t think we have a consensus on that.

And zblock is, after some additional improvements, just better than zsmalloc in all meaningful aspects, let alone the simplicity. It is fas easier to implement in Rust than zsmalloc, too. Besides, zram is a good candidate to be rewritten in Rust as well and after that is done, zblock will be even safer and faster. So while not being “incomplete", it’s zsmalloc that is becoming a niche backend moving forward, and I would argue that it could make more sense to eventually obsolete *it* rather than the zpool API.

~Vitaly
Re: [PATCH v4 0/2] rust: zpool: add abstraction for zpool drivers
Posted by Vlastimil Babka 4 weeks ago
On 8/26/25 16:56, Vitaly Wool wrote:
> 
> 
>> On Aug 26, 2025, at 2:44 PM, Johannes Weiner <hannes@cmpxchg.org> wrote:
>> 
>> On Sat, Aug 23, 2025 at 03:04:19PM +0200, Vitaly Wool wrote:
>>> Zpool is a common frontend for memory storage pool implementations.
>>> These pools are typically used to store compressed memory objects,
>>> e. g. for Zswap, the lightweight compressed cache for swap pages.
>>> 
>>> This patch provides the interface to use Zpool in Rust kernel code,
>>> thus enabling Rust implementations of Zpool allocators for Zswap.
>> 
>> The zpool indirection is on its way out.
>> 
>> When you submitted an alternate allocator backend recently, the
>> resounding feedback from the zswap maintainers was that improvements
>> should happen to zsmalloc incrementally. It is a lot of code and has a
>> lot of features that go beyond allocation strategy. We do not want to
>> fork it and fragment this space again with niche, incomplete backends.
>> 
>> It's frustrating that you not only ignored this, but then went ahead
>> and made other people invest their time and effort into this as well.
>> 
> 
> I don’t think we have a consensus on that.
> 
> And zblock is, after some additional improvements, just better than
> zsmalloc in all meaningful aspects, let alone the simplicity. It is fas
> easier to implement in Rust than zsmalloc, too. Besides, zram is a good
> candidate to be rewritten in Rust as well and after that is done, zblock

If your target is zram (not zswap) then I don't understand why insist on the
zpool layer, as zram refused to adopt it in the first place?

> will be even safer and faster. So while not being “incomplete", it’s
> zsmalloc that is becoming a niche backend moving forward, and I would
> argue that it could make more sense to eventually obsolete *it* rather
> than the zpool API.
> 
> ~Vitaly

Re: [PATCH v4 0/2] rust: zpool: add abstraction for zpool drivers
Posted by Vitaly Wool 4 weeks ago

On 9/5/25 08:58, Vlastimil Babka wrote:
> On 8/26/25 16:56, Vitaly Wool wrote:
>>
>>
>>> On Aug 26, 2025, at 2:44 PM, Johannes Weiner <hannes@cmpxchg.org> wrote:
>>>
>>> On Sat, Aug 23, 2025 at 03:04:19PM +0200, Vitaly Wool wrote:
>>>> Zpool is a common frontend for memory storage pool implementations.
>>>> These pools are typically used to store compressed memory objects,
>>>> e. g. for Zswap, the lightweight compressed cache for swap pages.
>>>>
>>>> This patch provides the interface to use Zpool in Rust kernel code,
>>>> thus enabling Rust implementations of Zpool allocators for Zswap.
>>>
>>> The zpool indirection is on its way out.
>>>
>>> When you submitted an alternate allocator backend recently, the
>>> resounding feedback from the zswap maintainers was that improvements
>>> should happen to zsmalloc incrementally. It is a lot of code and has a
>>> lot of features that go beyond allocation strategy. We do not want to
>>> fork it and fragment this space again with niche, incomplete backends.
>>>
>>> It's frustrating that you not only ignored this, but then went ahead
>>> and made other people invest their time and effort into this as well.
>>>
>>
>> I don’t think we have a consensus on that.
>>
>> And zblock is, after some additional improvements, just better than
>> zsmalloc in all meaningful aspects, let alone the simplicity. It is fas
>> easier to implement in Rust than zsmalloc, too. Besides, zram is a good
>> candidate to be rewritten in Rust as well and after that is done, zblock
> 
> If your target is zram (not zswap) then I don't understand why insist on the
> zpool layer, as zram refused to adopt it in the first place?

The target is zswap in the first place.

Then, I apologize for quoting myself, but what I wrote was "zram is a 
good candidate to be rewritten in Rust", and I still hold on to this 
idea. zram can be greatly simplified by that rewrite and the code line 
savings will be far more than 700.

So, back to my point in the initial post. When/if zram is rewritten in 
Rust it will make more sense for it to use an allocator which is also 
written in Rust rather then zsmalloc, provided that the allocator 
written in Rust is good enough. And *that* will leave zsmalloc a niche 
backend.

You see,

>> will be even safer and faster. So while not being “incomplete", it’s
>> zsmalloc that is becoming a niche backend moving forward, and I would
>> argue that it could make more sense to eventually obsolete *it* rather
>> than the zpool API.
>>
>> ~Vitaly
> 
> 

Re: [PATCH v4 0/2] rust: zpool: add abstraction for zpool drivers
Posted by Johannes Weiner 1 month, 1 week ago
On Tue, Aug 26, 2025 at 04:56:46PM +0200, Vitaly Wool wrote:
> 
> 
> > On Aug 26, 2025, at 2:44 PM, Johannes Weiner <hannes@cmpxchg.org> wrote:
> > 
> > On Sat, Aug 23, 2025 at 03:04:19PM +0200, Vitaly Wool wrote:
> >> Zpool is a common frontend for memory storage pool implementations.
> >> These pools are typically used to store compressed memory objects,
> >> e. g. for Zswap, the lightweight compressed cache for swap pages.
> >> 
> >> This patch provides the interface to use Zpool in Rust kernel code,
> >> thus enabling Rust implementations of Zpool allocators for Zswap.
> > 
> > The zpool indirection is on its way out.
> > 
> > When you submitted an alternate allocator backend recently, the
> > resounding feedback from the zswap maintainers was that improvements
> > should happen to zsmalloc incrementally. It is a lot of code and has a
> > lot of features that go beyond allocation strategy. We do not want to
> > fork it and fragment this space again with niche, incomplete backends.
> > 
> > It's frustrating that you not only ignored this, but then went ahead
> > and made other people invest their time and effort into this as well.
> > 
> 
> I don’t think we have a consensus on that.
> 
> And zblock is, after some additional improvements, just better than
> zsmalloc in all meaningful aspects, let alone the simplicity. It is
> fas easier to implement in Rust than zsmalloc, too. Besides, zram is
> a good candidate to be rewritten in Rust as well and after that is
> done, zblock will be even safer and faster. So while not being
> “incomplete", it’s zsmalloc that is becoming a niche backend moving
> forward, and I would argue that it could make more sense to
> eventually obsolete *it* rather than the zpool API.

That's your opinion, and I disagree with all of these claims. I would
also be surprised if you found much alignment on this with the other
folks who develop and use these features on a daily basis.

That being said, by all means, you can propose alternate
allocators. But you don't need the zpool API for that. Just provide
alternate implementations of the "zs_*" API and make it compile-time
selectable.

As it stands, it's hard to justify the almost 700 lines of code to
support *runtime-switching* of zswap backends when there is only one
backend in-tree (and even you suggest there should only be one, albeit
a different one).
Re: [PATCH v4 0/2] rust: zpool: add abstraction for zpool drivers
Posted by Vitaly Wool 1 month ago

> On Aug 27, 2025, at 3:07 PM, Johannes Weiner <hannes@cmpxchg.org> wrote:
> 
> On Tue, Aug 26, 2025 at 04:56:46PM +0200, Vitaly Wool wrote:
>> 
>> 
>>> On Aug 26, 2025, at 2:44 PM, Johannes Weiner <hannes@cmpxchg.org> wrote:
>>> 
>>> On Sat, Aug 23, 2025 at 03:04:19PM +0200, Vitaly Wool wrote:
>>>> Zpool is a common frontend for memory storage pool implementations.
>>>> These pools are typically used to store compressed memory objects,
>>>> e. g. for Zswap, the lightweight compressed cache for swap pages.
>>>> 
>>>> This patch provides the interface to use Zpool in Rust kernel code,
>>>> thus enabling Rust implementations of Zpool allocators for Zswap.
>>> 
>>> The zpool indirection is on its way out.
>>> 
>>> When you submitted an alternate allocator backend recently, the
>>> resounding feedback from the zswap maintainers was that improvements
>>> should happen to zsmalloc incrementally. It is a lot of code and has a
>>> lot of features that go beyond allocation strategy. We do not want to
>>> fork it and fragment this space again with niche, incomplete backends.
>>> 
>>> It's frustrating that you not only ignored this, but then went ahead
>>> and made other people invest their time and effort into this as well.
>>> 
>> 
>> I don’t think we have a consensus on that.
>> 
>> And zblock is, after some additional improvements, just better than
>> zsmalloc in all meaningful aspects, let alone the simplicity. It is
>> fas easier to implement in Rust than zsmalloc, too. Besides, zram is
>> a good candidate to be rewritten in Rust as well and after that is
>> done, zblock will be even safer and faster. So while not being
>> “incomplete", it’s zsmalloc that is becoming a niche backend moving
>> forward, and I would argue that it could make more sense to
>> eventually obsolete *it* rather than the zpool API.
> 
> That's your opinion, and I disagree with all of these claims. I would
> also be surprised if you found much alignment on this with the other
> folks who develop and use these features on a daily basis.

By features you mean zsmalloc? I certainly respect the effort put in it but if not for it being rather sluggish and error prone in non-4K page environments, we’d not come up with zblock.

> That being said, by all means, you can propose alternate
> allocators. But you don't need the zpool API for that. Just provide
> alternate implementations of the "zs_*" API and make it compile-time
> selectable.
> 
zpool API is neutral and well-defined, I don’t see *any* good reason for it to be phased out.

> As it stands, it's hard to justify the almost 700 lines of code to
> support *runtime-switching* of zswap backends when there is only one
> backend in-tree (and even you suggest there should only be one, albeit
> a different one).

No, I don’t. What I said was that zsmalloc was arguably becoming a niche allocator. And the point here is, the toy allocator written in Rust (just as an example of how zpool API could be used) shows in some tests similar results to zsmalloc both performance and compression density wise. 
Re: [PATCH v4 0/2] rust: zpool: add abstraction for zpool drivers
Posted by Miguel Ojeda 1 month, 1 week ago
On Sat, Aug 23, 2025 at 3:04 PM Vitaly Wool <vitaly.wool@konsulko.se> wrote:
>
> This patch provides the interface to use Zpool in Rust kernel code,
> thus enabling Rust implementations of Zpool allocators for Zswap.

In v1 the usual use case question was asked -- could we get some more
details in the cover letter or ideally in the patch itself?

>  bindings/bindings_helper.h |    1
>  helpers/helpers.c          |    1
>  helpers/zpool.c            |    6 +
>  kernel/alloc.rs            |    5
>  kernel/lib.rs              |    2
>  kernel/zpool.rs            |  338 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

Should a `MAINTAINERS` change be added? Was maintenance in general discussed?

By the way, the diffstat here in the cover letter seems to be
generated w.r.t. `rust/` for some reason.

Thanks!

Cheers,
Miguel
Re: [PATCH v4 0/2] rust: zpool: add abstraction for zpool drivers
Posted by Danilo Krummrich 1 month, 1 week ago
On 8/26/25 12:43 PM, Miguel Ojeda wrote:
> On Sat, Aug 23, 2025 at 3:04 PM Vitaly Wool <vitaly.wool@konsulko.se> wrote:
>>
>> This patch provides the interface to use Zpool in Rust kernel code,
>> thus enabling Rust implementations of Zpool allocators for Zswap.
> 
> In v1 the usual use case question was asked -- could we get some more
> details in the cover letter or ideally in the patch itself?
> 
>>   bindings/bindings_helper.h |    1
>>   helpers/helpers.c          |    1
>>   helpers/zpool.c            |    6 +
>>   kernel/alloc.rs            |    5
>>   kernel/lib.rs              |    2
>>   kernel/zpool.rs            |  338 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 
> Should a `MAINTAINERS` change be added? Was maintenance in general discussed?

@Alice: I assume this should go under the Rust MM entry?
Re: [PATCH v4 0/2] rust: zpool: add abstraction for zpool drivers
Posted by Alice Ryhl 1 month, 1 week ago
On Tue, Aug 26, 2025 at 01:37:22PM +0200, Danilo Krummrich wrote:
> On 8/26/25 12:43 PM, Miguel Ojeda wrote:
> > On Sat, Aug 23, 2025 at 3:04 PM Vitaly Wool <vitaly.wool@konsulko.se> wrote:
> > > 
> > > This patch provides the interface to use Zpool in Rust kernel code,
> > > thus enabling Rust implementations of Zpool allocators for Zswap.
> > 
> > In v1 the usual use case question was asked -- could we get some more
> > details in the cover letter or ideally in the patch itself?
> > 
> > >   bindings/bindings_helper.h |    1
> > >   helpers/helpers.c          |    1
> > >   helpers/zpool.c            |    6 +
> > >   kernel/alloc.rs            |    5
> > >   kernel/lib.rs              |    2
> > >   kernel/zpool.rs            |  338 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 
> > Should a `MAINTAINERS` change be added? Was maintenance in general discussed?
> 
> @Alice: I assume this should go under the Rust MM entry?

That makes sense to me.

Alice