[PATCH] rust: zpool: add abstraction for zpool drivers

[PATCH] rust: zpool: add abstraction for zpool drivers

[Vitaly Wool]

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.

Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.se>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/bindings/bindings_helper.h |   1 +
 rust/helpers/helpers.c          |   1 +
 rust/helpers/zpool.c            |   6 +
 rust/kernel/alloc.rs            |   5 +
 rust/kernel/lib.rs              |   2 +
 rust/kernel/zpool.rs            | 269 ++++++++++++++++++++++++++++++++
 6 files changed, 284 insertions(+)
 create mode 100644 rust/helpers/zpool.c
 create mode 100644 rust/kernel/zpool.rs

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 84d60635e8a9..f0c4c454882b 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -75,6 +75,7 @@
 #include <linux/wait.h>
 #include <linux/workqueue.h>
 #include <linux/xarray.h>
+#include <linux/zpool.h>
 #include <trace/events/rust_sample.h>
 
 #if defined(CONFIG_DRM_PANIC_SCREEN_QR_CODE)
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 7cf7fe95e41d..e1a7556cc700 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -51,3 +51,4 @@
 #include "wait.c"
 #include "workqueue.c"
 #include "xarray.c"
+#include "zpool.c"
diff --git a/rust/helpers/zpool.c b/rust/helpers/zpool.c
new file mode 100644
index 000000000000..71ba173f917a
--- /dev/null
+++ b/rust/helpers/zpool.c
@@ -0,0 +1,6 @@
+#include <linux/zpool.h>
+
+void rust_helper_zpool_register_driver(struct zpool_driver *driver)
+{
+	zpool_register_driver(driver);
+}
diff --git a/rust/kernel/alloc.rs b/rust/kernel/alloc.rs
index b39c279236f5..0fec5337908c 100644
--- a/rust/kernel/alloc.rs
+++ b/rust/kernel/alloc.rs
@@ -41,6 +41,11 @@
 pub struct Flags(u32);
 
 impl Flags {
+    /// Create from the raw representation
+    pub fn new(f: u32) -> Self {
+        Self(f)
+    }
+
     /// Get the raw representation of this flag.
     pub(crate) fn as_raw(self) -> u32 {
         self.0
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index ed53169e795c..165d52feeea4 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -129,6 +129,8 @@
 pub mod uaccess;
 pub mod workqueue;
 pub mod xarray;
+#[cfg(CONFIG_ZPOOL)]
+pub mod zpool;
 
 #[doc(hidden)]
 pub use bindings;
diff --git a/rust/kernel/zpool.rs b/rust/kernel/zpool.rs
new file mode 100644
index 000000000000..91926c2e99e8
--- /dev/null
+++ b/rust/kernel/zpool.rs
@@ -0,0 +1,269 @@
+use crate::{
+    bindings,
+    error::Result,
+    kernel::alloc::Flags,
+    str::CStr,
+    types::{ForeignOwnable, Opaque},
+};
+use core::ffi::{c_int, c_uchar, c_void};
+use core::ptr::null_mut;
+use kernel::alloc::NumaNode;
+use kernel::ThisModule;
+
+/// zpool API
+pub trait Zpool {
+    /// Opaque Rust representation of `struct zpool`.
+    type Pool: ForeignOwnable;
+
+    /// Create a pool.
+    fn create(name: *const c_uchar, gfp: Flags) -> Result<Self::Pool>;
+
+    /// Destroy the pool.
+    fn destroy(pool: Self::Pool);
+
+    /// Allocate an object of size `size` using GFP flags `gfp` from the pool `pool`, wuth the
+    /// preferred NUMA node `nid`. If the allocation is successful, an opaque handle is returned.
+    fn malloc(
+        pool: <Self::Pool as ForeignOwnable>::BorrowedMut<'_>,
+        size: usize,
+        gfp: Flags,
+        nid: NumaNode,
+    ) -> Result<usize>;
+
+    /// Free a previously allocated from the `pool` object, represented by `handle`.
+    fn free(pool: <Self::Pool as ForeignOwnable>::Borrowed<'_>, handle: usize);
+
+    /// Make all the necessary preparations for the caller to be able to read from the object
+    /// represented by `handle` and return a valid pointer to the `handle` memory to be read.
+    fn read_begin(pool: <Self::Pool as ForeignOwnable>::Borrowed<'_>, handle: usize)
+        -> *mut c_void;
+
+    /// Finish reading from a previously allocated `handle`. `handle_mem` must be the pointer
+    /// previously returned by `read_begin`.
+    fn read_end(
+        pool: <Self::Pool as ForeignOwnable>::Borrowed<'_>,
+        handle: usize,
+        handle_mem: *mut c_void,
+    );
+
+    /// Write to the object represented by a previously allocated `handle`. `handle_mem` points
+    /// to the memory to copy data from, and `mem_len` defines the length of the data block to
+    /// be copied.
+    fn write(
+        pool: <Self::Pool as ForeignOwnable>::Borrowed<'_>,
+        handle: usize,
+        handle_mem: *mut c_void,
+        mem_len: usize,
+    );
+
+    /// Get the number of pages used by the `pool`.
+    fn total_pages(pool: <Self::Pool as ForeignOwnable>::Borrowed<'_>) -> u64;
+}
+
+/// Zpool driver registration trait.
+pub trait Registration {
+    /// Register a zpool driver.
+    fn register(&self, name: &'static CStr, module: &'static ThisModule) -> Result;
+
+    /// Pool creation callback.
+    extern "C" fn _create(name: *const c_uchar, gfp: u32) -> *mut c_void;
+
+    /// Pool destruction callback.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that `pool` is a valid pointer to `struct zpool`.
+    unsafe extern "C" fn _destroy(pool: *mut c_void);
+
+    /// Callback for object allocation in the pool.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that `pool` is a valid pointer to `struct zpool` and that `handle`
+    /// is a valid pointer to usize.
+    unsafe extern "C" fn _malloc(
+        pool: *mut c_void,
+        size: usize,
+        gfp: u32,
+        handle: *mut usize,
+        nid: c_int,
+    ) -> c_int;
+
+    /// Callback for object release.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that `pool` is a valid pointer to `struct zpool`.
+    unsafe extern "C" fn _free(pool: *mut c_void, handle: usize);
+
+    /// Callback to prepare the object for reading.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that `pool` is a valid pointer to `struct zpool`.
+    unsafe extern "C" fn _obj_read_begin(
+        pool: *mut c_void,
+        handle: usize,
+        local_copy: *mut c_void,
+    ) -> *mut c_void;
+
+    /// Callback to signal the end of reading from an object.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that `pool` is a valid pointer to `struct zpool`.
+    unsafe extern "C" fn _obj_read_end(pool: *mut c_void, handle: usize, handle_mem: *mut c_void);
+
+    /// Callback for writing to an object.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that `pool` is a valid pointer to `struct zpool`.
+    unsafe extern "C" fn _obj_write(
+        pool: *mut c_void,
+        handle: usize,
+        handle_mem: *mut c_void,
+        mem_len: usize,
+    );
+
+    /// Callback to return the number of pages in the pool.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that `pool` is a valid pointer to `struct zpool`.
+    unsafe extern "C" fn _total_pages(pool: *mut c_void) -> u64;
+}
+
+/// Zpool driver structure.
+pub struct ZpoolDriver<T: Zpool> {
+    inner: Opaque<bindings::zpool_driver>,
+
+    /// Zpool callback functions that a zpool driver must provide
+    pub callbacks: T,
+}
+
+impl<T: Zpool> Clone for ZpoolDriver<T> {
+    fn clone(&self) -> Self {
+        todo!()
+    }
+}
+
+// SAFETY: zpool driver must ensure that ZpoolDriver's `callbacks` are thread safe
+unsafe impl<T: Zpool> Sync for ZpoolDriver<T> {}
+
+impl<T: Zpool> ZpoolDriver<T> {
+    /// create an instance of a zpool driver
+    pub const fn new(t: T) -> Self {
+        Self {
+            inner: Opaque::uninit(),
+            callbacks: t,
+        }
+    }
+}
+
+impl<T: Zpool> Registration for ZpoolDriver<T> {
+    extern "C" fn _create(name: *const c_uchar, gfp: u32) -> *mut c_void {
+        let pool = T::create(name, Flags::new(gfp));
+        match pool {
+            Err(_) => null_mut(),
+            Ok(p) => T::Pool::into_foreign(p),
+        }
+    }
+    unsafe extern "C" fn _destroy(pool: *mut c_void) {
+        // SAFETY: The pointer originates from an `into_foreign` call.
+        T::destroy(unsafe { T::Pool::from_foreign(pool) })
+    }
+    unsafe extern "C" fn _malloc(
+        pool: *mut c_void,
+        size: usize,
+        gfp: u32,
+        handle: *mut usize,
+        nid: c_int,
+    ) -> c_int {
+        // SAFETY: The pointer originates from an `into_foreign` call. If `pool` is passed to
+        // `from_foreign`, then that happens in `_destroy` which will not be called during this
+        // method.
+        let pool = unsafe { T::Pool::borrow_mut(pool) };
+        let real_nid = match nid {
+            bindings::NUMA_NO_NODE => Ok(NumaNode::NO_NODE),
+            _ => NumaNode::new(nid),
+        };
+        if real_nid.is_err() {
+            return -(bindings::EINVAL as i32);
+        }
+
+        let result = T::malloc(pool, size, Flags::new(gfp), real_nid.unwrap());
+        match result {
+            Err(_) => -(bindings::ENOMEM as i32),
+            Ok(h) => {
+                // SAFETY: handle is guaranteed to be a valid pointer by zpool
+                unsafe { *handle = h };
+                0
+            }
+        }
+    }
+    unsafe extern "C" fn _free(pool: *mut c_void, handle: usize) {
+        // SAFETY: The pointer originates from an `into_foreign` call. If `pool` is passed to
+        // `from_foreign`, then that happens in `_destroy` which will not be called during this
+        // method.
+        let pool = unsafe { T::Pool::borrow(pool) };
+        T::free(pool, handle)
+    }
+    unsafe extern "C" fn _obj_read_begin(
+        pool: *mut c_void,
+        handle: usize,
+        _local_copy: *mut c_void,
+    ) -> *mut c_void {
+        // SAFETY: The pointer originates from an `into_foreign` call. If `pool` is passed to
+        // `from_foreign`, then that happens in `_destroy` which will not be called during this
+        // method.
+        let pool = unsafe { T::Pool::borrow(pool) };
+        T::read_begin(pool, handle)
+    }
+    unsafe extern "C" fn _obj_read_end(pool: *mut c_void, handle: usize, handle_mem: *mut c_void) {
+        // SAFETY: The pointer originates from an `into_foreign` call. If `pool` is passed to
+        // `from_foreign`, then that happens in `_destroy` which will not be called during this
+        // method.
+        let pool = unsafe { T::Pool::borrow(pool) };
+        T::read_end(pool, handle, handle_mem)
+    }
+    unsafe extern "C" fn _obj_write(
+        pool: *mut c_void,
+        handle: usize,
+        handle_mem: *mut c_void,
+        mem_len: usize,
+    ) {
+        // SAFETY: The pointer originates from an `into_foreign` call. If `pool` is passed to
+        // `from_foreign`, then that happens in `_destroy` which will not be called during this
+        // method.
+        let pool = unsafe { T::Pool::borrow(pool) };
+        T::write(pool, handle, handle_mem, mem_len);
+    }
+    unsafe extern "C" fn _total_pages(pool: *mut c_void) -> u64 {
+        // SAFETY: The pointer originates from an `into_foreign` call. If `pool` is passed to
+        // `from_foreign`, then that happens in `_destroy` which will not be called during this
+        // method.
+        let pool = unsafe { T::Pool::borrow(pool) };
+        T::total_pages(pool)
+    }
+
+    fn register(&self, name: &'static CStr, module: &'static ThisModule) -> Result {
+        // SAFETY: `ZpoolDriver::new()` ensures that `self.inner` is a valid pointer
+        unsafe {
+            (*(self.inner.get())).create = Some(Self::_create);
+            (*(self.inner.get())).destroy = Some(Self::_destroy);
+            (*(self.inner.get())).malloc = Some(Self::_malloc);
+            (*(self.inner.get())).free = Some(Self::_free);
+            (*(self.inner.get())).obj_read_begin = Some(Self::_obj_read_begin);
+            (*(self.inner.get())).obj_read_end = Some(Self::_obj_read_end);
+            (*(self.inner.get())).obj_write = Some(Self::_obj_write);
+            (*(self.inner.get())).total_pages = Some(Self::_total_pages);
+
+            (*(self.inner.get())).owner = module.0;
+            (*(self.inner.get())).type_ = name.as_char_ptr().cast_mut();
+
+            bindings::zpool_register_driver(self.inner.get());
+        }
+        Ok(())
+    }
+}
-- 
2.39.2

Re: [PATCH] rust: zpool: add abstraction for zpool drivers

[Danilo Krummrich]

On Wed Aug 20, 2025 at 11:15 AM CEST, 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.

Do you work on such a user? Do you have code using this API already?

More specifically, do you plan to re-implement Zswap in Rust?

> Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.se>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

If Alice contributed to the patch you may want to add a Co-developed-by: tag,
otherwise this doesn't need her SoB.

> +/// zpool API

It would be nice to have some more documentation on this trait, including a
doc-test illustrating some example usage.

> +pub trait Zpool {
> +    /// Opaque Rust representation of `struct zpool`.
> +    type Pool: ForeignOwnable;

Something that embedds a struct zpool, such as struct zswap_pool? If so, isn't
this type simply Self?

> +
> +    /// Create a pool.
> +    fn create(name: *const c_uchar, gfp: Flags) -> Result<Self::Pool>;

This shouldn't be a raw pointer, but rather &CStr.

> +
> +    /// Destroy the pool.
> +    fn destroy(pool: Self::Pool);
> +
> +    /// Allocate an object of size `size` using GFP flags `gfp` from the pool `pool`, wuth the
> +    /// preferred NUMA node `nid`. If the allocation is successful, an opaque handle is returned.
> +    fn malloc(
> +        pool: <Self::Pool as ForeignOwnable>::BorrowedMut<'_>,
> +        size: usize,
> +        gfp: Flags,
> +        nid: NumaNode,
> +    ) -> Result<usize>;
> +
> +    /// Free a previously allocated from the `pool` object, represented by `handle`.
> +    fn free(pool: <Self::Pool as ForeignOwnable>::Borrowed<'_>, handle: usize);
> +
> +    /// Make all the necessary preparations for the caller to be able to read from the object
> +    /// represented by `handle` and return a valid pointer to the `handle` memory to be read.
> +    fn read_begin(pool: <Self::Pool as ForeignOwnable>::Borrowed<'_>, handle: usize)
> +        -> *mut c_void;

Why does this return a raw pointer? I think this needs a proper type
representation.

> +
> +    /// Finish reading from a previously allocated `handle`. `handle_mem` must be the pointer
> +    /// previously returned by `read_begin`.
> +    fn read_end(
> +        pool: <Self::Pool as ForeignOwnable>::Borrowed<'_>,
> +        handle: usize,
> +        handle_mem: *mut c_void,
> +    );

Same here...

> +
> +    /// Write to the object represented by a previously allocated `handle`. `handle_mem` points
> +    /// to the memory to copy data from, and `mem_len` defines the length of the data block to
> +    /// be copied.
> +    fn write(
> +        pool: <Self::Pool as ForeignOwnable>::Borrowed<'_>,
> +        handle: usize,
> +        handle_mem: *mut c_void,

...and here.

> +        mem_len: usize,
> +    );
> +
> +    /// Get the number of pages used by the `pool`.
> +    fn total_pages(pool: <Self::Pool as ForeignOwnable>::Borrowed<'_>) -> u64;
> +}
> +
> +/// Zpool driver registration trait.
> +pub trait Registration {

I think you should use the kernel::driver::Registration instead, it's
specifically for the purpose you defined this trait and ZpoolDriver for.

As for the C callbacks, they should go into the Adapter type (which implements
kernel::driver::RegistrationOps) directly, they don't need to be in a trait.

This way a new Zpool Registration is created with:

	driver::Registration<zpool::Adapter>::new()

This also allows you to take advantage of the module_driver!() macro to provide
your own module_zpool_driver!() macro.

> +    /// Register a zpool driver.
> +    fn register(&self, name: &'static CStr, module: &'static ThisModule) -> Result;
> +
> +    /// Pool creation callback.
> +    extern "C" fn _create(name: *const c_uchar, gfp: u32) -> *mut c_void;
> +
> +    /// Pool destruction callback.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The caller must ensure that `pool` is a valid pointer to `struct zpool`.
> +    unsafe extern "C" fn _destroy(pool: *mut c_void);
> +
> +    /// Callback for object allocation in the pool.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The caller must ensure that `pool` is a valid pointer to `struct zpool` and that `handle`
> +    /// is a valid pointer to usize.
> +    unsafe extern "C" fn _malloc(
> +        pool: *mut c_void,
> +        size: usize,
> +        gfp: u32,
> +        handle: *mut usize,
> +        nid: c_int,
> +    ) -> c_int;
> +
> +    /// Callback for object release.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The caller must ensure that `pool` is a valid pointer to `struct zpool`.
> +    unsafe extern "C" fn _free(pool: *mut c_void, handle: usize);
> +
> +    /// Callback to prepare the object for reading.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The caller must ensure that `pool` is a valid pointer to `struct zpool`.
> +    unsafe extern "C" fn _obj_read_begin(
> +        pool: *mut c_void,
> +        handle: usize,
> +        local_copy: *mut c_void,
> +    ) -> *mut c_void;
> +
> +    /// Callback to signal the end of reading from an object.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The caller must ensure that `pool` is a valid pointer to `struct zpool`.
> +    unsafe extern "C" fn _obj_read_end(pool: *mut c_void, handle: usize, handle_mem: *mut c_void);
> +
> +    /// Callback for writing to an object.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The caller must ensure that `pool` is a valid pointer to `struct zpool`.
> +    unsafe extern "C" fn _obj_write(
> +        pool: *mut c_void,
> +        handle: usize,
> +        handle_mem: *mut c_void,
> +        mem_len: usize,
> +    );
> +
> +    /// Callback to return the number of pages in the pool.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The caller must ensure that `pool` is a valid pointer to `struct zpool`.
> +    unsafe extern "C" fn _total_pages(pool: *mut c_void) -> u64;
> +}
> +
> +/// Zpool driver structure.
> +pub struct ZpoolDriver<T: Zpool> {
> +    inner: Opaque<bindings::zpool_driver>,

I think this needs pin-init, another reason to use kernel::driver::Registration
instead. :)

> +
> +    /// Zpool callback functions that a zpool driver must provide
> +    pub callbacks: T,
> +}
> +
> +impl<T: Zpool> Clone for ZpoolDriver<T> {
> +    fn clone(&self) -> Self {
> +        todo!()
> +    }
> +}

Cloning the driver structure? Why? Please also consider that struct zpool_driver
needs to be pinned.

Re: [PATCH] rust: zpool: add abstraction for zpool drivers

[Vitaly Wool]

> On Aug 20, 2025, at 12:16 PM, Danilo Krummrich <dakr@kernel.org> wrote:
> 
> On Wed Aug 20, 2025 at 11:15 AM CEST, 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.
> 
> Do you work on such a user? Do you have code using this API already?
> 
> More specifically, do you plan to re-implement Zswap in Rust?

I don’t plan to reimplement zswap as a whole, just the allocation backend for it. I do have the code but it’s not quite ready for submission yet. However, it does work and is publicly available at https://github.com/vwool/linux-mm/commit/6fde78b24c8f8716c17ffa20cec390b516c7882b
> 
>> Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.se>
>> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> 
> If Alice contributed to the patch you may want to add a Co-developed-by: tag,
> otherwise this doesn't need her SoB.

Will do, thanks.

> 
>> +/// zpool API
> 
> It would be nice to have some more documentation on this trait, including a
> doc-test illustrating some example usage.
> 
>> +pub trait Zpool {
>> +    /// Opaque Rust representation of `struct zpool`.
>> +    type Pool: ForeignOwnable;
> 
> Something that embedds a struct zpool, such as struct zswap_pool? If so, isn't
> this type simply Self?

I think ForeignOwnable provides a good representation of 'struct zpool’ and it’s convenient to borrow it, as done later in the patch.

> 
>> +
>> +    /// Create a pool.
>> +    fn create(name: *const c_uchar, gfp: Flags) -> Result<Self::Pool>;
> 
> This shouldn't be a raw pointer, but rather &CStr.
> 
>> +
>> +    /// Destroy the pool.
>> +    fn destroy(pool: Self::Pool);
>> +
>> +    /// Allocate an object of size `size` using GFP flags `gfp` from the pool `pool`, wuth the
>> +    /// preferred NUMA node `nid`. If the allocation is successful, an opaque handle is returned.
>> +    fn malloc(
>> +        pool: <Self::Pool as ForeignOwnable>::BorrowedMut<'_>,
>> +        size: usize,
>> +        gfp: Flags,
>> +        nid: NumaNode,
>> +    ) -> Result<usize>;
>> +
>> +    /// Free a previously allocated from the `pool` object, represented by `handle`.
>> +    fn free(pool: <Self::Pool as ForeignOwnable>::Borrowed<'_>, handle: usize);
>> +
>> +    /// Make all the necessary preparations for the caller to be able to read from the object
>> +    /// represented by `handle` and return a valid pointer to the `handle` memory to be read.
>> +    fn read_begin(pool: <Self::Pool as ForeignOwnable>::Borrowed<'_>, handle: usize)
>> +        -> *mut c_void;
> 
> Why does this return a raw pointer? I think this needs a proper type
> representation.

The zpool API wants a raw pointer here so I decided not to overcomplicate it. I thought of using NonNull<[u8]> but it doesn’t seem to be a good fit. We’re basically returning a pointer to a place in memory which is guaranteed to be allocated and owned by us, but it is a raw pointer at the end of the day. What would you recommend here instead?

> 
>> +
>> +    /// Finish reading from a previously allocated `handle`. `handle_mem` must be the pointer
>> +    /// previously returned by `read_begin`.
>> +    fn read_end(
>> +        pool: <Self::Pool as ForeignOwnable>::Borrowed<'_>,
>> +        handle: usize,
>> +        handle_mem: *mut c_void,
>> +    );
> 
> Same here...
> 
>> +
>> +    /// Write to the object represented by a previously allocated `handle`. `handle_mem` points
>> +    /// to the memory to copy data from, and `mem_len` defines the length of the data block to
>> +    /// be copied.
>> +    fn write(
>> +        pool: <Self::Pool as ForeignOwnable>::Borrowed<'_>,
>> +        handle: usize,
>> +        handle_mem: *mut c_void,
> 
> ...and here.
> 
>> +        mem_len: usize,
>> +    );
>> +
>> +    /// Get the number of pages used by the `pool`.
>> +    fn total_pages(pool: <Self::Pool as ForeignOwnable>::Borrowed<'_>) -> u64;
>> +}
>> +
>> +/// Zpool driver registration trait.
>> +pub trait Registration {
> 
> I think you should use the kernel::driver::Registration instead, it's
> specifically for the purpose you defined this trait and ZpoolDriver for.
> 
> As for the C callbacks, they should go into the Adapter type (which implements
> kernel::driver::RegistrationOps) directly, they don't need to be in a trait.
> 
> This way a new Zpool Registration is created with:
> 
> driver::Registration<zpool::Adapter>::new()
> 
> This also allows you to take advantage of the module_driver!() macro to provide
> your own module_zpool_driver!() macro.

There was once a problem with that but I don’t remember what the problem was :) I’ll try that one more time, thanks.

> 
>> +    /// Register a zpool driver.
>> +    fn register(&self, name: &'static CStr, module: &'static ThisModule) -> Result;
>> +
>> +    /// Pool creation callback.
>> +    extern "C" fn _create(name: *const c_uchar, gfp: u32) -> *mut c_void;
>> +
>> +    /// Pool destruction callback.
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// The caller must ensure that `pool` is a valid pointer to `struct zpool`.
>> +    unsafe extern "C" fn _destroy(pool: *mut c_void);
>> +
>> +    /// Callback for object allocation in the pool.
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// The caller must ensure that `pool` is a valid pointer to `struct zpool` and that `handle`
>> +    /// is a valid pointer to usize.
>> +    unsafe extern "C" fn _malloc(
>> +        pool: *mut c_void,
>> +        size: usize,
>> +        gfp: u32,
>> +        handle: *mut usize,
>> +        nid: c_int,
>> +    ) -> c_int;
>> +
>> +    /// Callback for object release.
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// The caller must ensure that `pool` is a valid pointer to `struct zpool`.
>> +    unsafe extern "C" fn _free(pool: *mut c_void, handle: usize);
>> +
>> +    /// Callback to prepare the object for reading.
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// The caller must ensure that `pool` is a valid pointer to `struct zpool`.
>> +    unsafe extern "C" fn _obj_read_begin(
>> +        pool: *mut c_void,
>> +        handle: usize,
>> +        local_copy: *mut c_void,
>> +    ) -> *mut c_void;
>> +
>> +    /// Callback to signal the end of reading from an object.
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// The caller must ensure that `pool` is a valid pointer to `struct zpool`.
>> +    unsafe extern "C" fn _obj_read_end(pool: *mut c_void, handle: usize, handle_mem: *mut c_void);
>> +
>> +    /// Callback for writing to an object.
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// The caller must ensure that `pool` is a valid pointer to `struct zpool`.
>> +    unsafe extern "C" fn _obj_write(
>> +        pool: *mut c_void,
>> +        handle: usize,
>> +        handle_mem: *mut c_void,
>> +        mem_len: usize,
>> +    );
>> +
>> +    /// Callback to return the number of pages in the pool.
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// The caller must ensure that `pool` is a valid pointer to `struct zpool`.
>> +    unsafe extern "C" fn _total_pages(pool: *mut c_void) -> u64;
>> +}
>> +
>> +/// Zpool driver structure.
>> +pub struct ZpoolDriver<T: Zpool> {
>> +    inner: Opaque<bindings::zpool_driver>,
> 
> I think this needs pin-init, another reason to use kernel::driver::Registration
> instead. :)
> 
>> +
>> +    /// Zpool callback functions that a zpool driver must provide
>> +    pub callbacks: T,
>> +}
>> +
>> +impl<T: Zpool> Clone for ZpoolDriver<T> {
>> +    fn clone(&self) -> Self {
>> +        todo!()
>> +    }
>> +}
> 
> Cloning the driver structure? Why? Please also consider that struct zpool_driver
> needs to be pinned.

Totally unnecessary, I agree. Will remove.

Thanks for the quick review!

~Vitaly

Re: [PATCH] rust: zpool: add abstraction for zpool drivers

[Danilo Krummrich]

On Wed Aug 20, 2025 at 12:44 PM CEST, Vitaly Wool wrote:
>> On Aug 20, 2025, at 12:16 PM, Danilo Krummrich <dakr@kernel.org> wrote:
>>> +/// zpool API
>> 
>> It would be nice to have some more documentation on this trait, including a
>> doc-test illustrating some example usage.
>> 
>>> +pub trait Zpool {
>>> +    /// Opaque Rust representation of `struct zpool`.
>>> +    type Pool: ForeignOwnable;
>> 
>> Something that embedds a struct zpool, such as struct zswap_pool? If so, isn't
>> this type simply Self?
>
> I think ForeignOwnable provides a good representation of 'struct zpool’ and it’s convenient to borrow it, as done later in the patch.

ForeignOwnable is not a representation for a specific type, but rather something
that originates from the Rust side and is owned by the C side. But that's not
the case here.

Regarding the "convenient to borrow" part, why can't Zpool::create() return
Result<Self> and e.g. malloc be defined as

	fn malloc(
	   &self,
	   size: usize,
	   gfp: Flags,
	   nid: NumaNode,
	) -> Result<usize>;

i.e. why does it need to be ForeignOwnable in the semantic sense of the trait?

>>> +    /// Make all the necessary preparations for the caller to be able to read from the object
>>> +    /// represented by `handle` and return a valid pointer to the `handle` memory to be read.
>>> +    fn read_begin(pool: <Self::Pool as ForeignOwnable>::Borrowed<'_>, handle: usize)
>>> +        -> *mut c_void;
>> 
>> Why does this return a raw pointer? I think this needs a proper type
>> representation.
>
> The zpool API wants a raw pointer here so I decided not to overcomplicate it. I thought of using NonNull<[u8]> but it doesn’t seem to be a good fit. We’re basically returning a pointer to a place in memory which is guaranteed to be allocated and owned by us, but it is a raw pointer at the end of the day. What would you recommend here instead?

I don't know the exact semantics behind read_begin(), but we should at least
encapsulate the pointer into a new type and restrict its lifetime to the
validity of the encapsulated pointer, such that it can't be used in the wrong
way.

More general, if we don't cover such things and use raw pointers for convenience
instead, we may not provide enough of a benefit compared to what we can do in
a C API.

>>> +
>>> +    /// Finish reading from a previously allocated `handle`. `handle_mem` must be the pointer
>>> +    /// previously returned by `read_begin`.
>>> +    fn read_end(
>>> +        pool: <Self::Pool as ForeignOwnable>::Borrowed<'_>,
>>> +        handle: usize,
>>> +        handle_mem: *mut c_void,
>>> +    );
>> 
>> Same here...
>> 
>>> +
>>> +    /// Write to the object represented by a previously allocated `handle`. `handle_mem` points
>>> +    /// to the memory to copy data from, and `mem_len` defines the length of the data block to
>>> +    /// be copied.
>>> +    fn write(
>>> +        pool: <Self::Pool as ForeignOwnable>::Borrowed<'_>,
>>> +        handle: usize,
>>> +        handle_mem: *mut c_void,
>> 
>> ...and here.
>> 
>>> +        mem_len: usize,
>>> +    );
>>> +
>>> +    /// Get the number of pages used by the `pool`.
>>> +    fn total_pages(pool: <Self::Pool as ForeignOwnable>::Borrowed<'_>) -> u64;
>>> +}
>>> +
>>> +/// Zpool driver registration trait.
>>> +pub trait Registration {
>> 
>> I think you should use the kernel::driver::Registration instead, it's
>> specifically for the purpose you defined this trait and ZpoolDriver for.
>> 
>> As for the C callbacks, they should go into the Adapter type (which implements
>> kernel::driver::RegistrationOps) directly, they don't need to be in a trait.
>> 
>> This way a new Zpool Registration is created with:
>> 
>> driver::Registration<zpool::Adapter>::new()
>> 
>> This also allows you to take advantage of the module_driver!() macro to provide
>> your own module_zpool_driver!() macro.
>
> There was once a problem with that but I don’t remember what the problem was :) I’ll try that one more time, thanks.

I'm pretty sure this should work out well (just like it does for driver
registrations such as PCI, platform, auxiliary, I2C, etc.). However, if you run
into issues, please let me know, I'm happy to help out resolving them.

Re: [PATCH] rust: zpool: add abstraction for zpool drivers

[Elle Rhumsaa]

On Wed, Aug 20, 2025 at 11:15:43AM +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.
> 
> Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.se>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/bindings/bindings_helper.h |   1 +
>  rust/helpers/helpers.c          |   1 +
>  rust/helpers/zpool.c            |   6 +
>  rust/kernel/alloc.rs            |   5 +
>  rust/kernel/lib.rs              |   2 +
>  rust/kernel/zpool.rs            | 269 ++++++++++++++++++++++++++++++++
>  6 files changed, 284 insertions(+)
>  create mode 100644 rust/helpers/zpool.c
>  create mode 100644 rust/kernel/zpool.rs
> 
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 84d60635e8a9..f0c4c454882b 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -75,6 +75,7 @@
>  #include <linux/wait.h>
>  #include <linux/workqueue.h>
>  #include <linux/xarray.h>
> +#include <linux/zpool.h>
>  #include <trace/events/rust_sample.h>
>  
>  #if defined(CONFIG_DRM_PANIC_SCREEN_QR_CODE)
> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> index 7cf7fe95e41d..e1a7556cc700 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -51,3 +51,4 @@
>  #include "wait.c"
>  #include "workqueue.c"
>  #include "xarray.c"
> +#include "zpool.c"
> diff --git a/rust/helpers/zpool.c b/rust/helpers/zpool.c
> new file mode 100644
> index 000000000000..71ba173f917a
> --- /dev/null
> +++ b/rust/helpers/zpool.c
> @@ -0,0 +1,6 @@
> +#include <linux/zpool.h>
> +
> +void rust_helper_zpool_register_driver(struct zpool_driver *driver)
> +{
> +	zpool_register_driver(driver);
> +}
> diff --git a/rust/kernel/alloc.rs b/rust/kernel/alloc.rs
> index b39c279236f5..0fec5337908c 100644
> --- a/rust/kernel/alloc.rs
> +++ b/rust/kernel/alloc.rs
> @@ -41,6 +41,11 @@
>  pub struct Flags(u32);
>  
>  impl Flags {
> +    /// Create from the raw representation
> +    pub fn new(f: u32) -> Self {
> +        Self(f)
> +    }
> +
>      /// Get the raw representation of this flag.
>      pub(crate) fn as_raw(self) -> u32 {
>          self.0
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index ed53169e795c..165d52feeea4 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -129,6 +129,8 @@
>  pub mod uaccess;
>  pub mod workqueue;
>  pub mod xarray;
> +#[cfg(CONFIG_ZPOOL)]
> +pub mod zpool;
>  
>  #[doc(hidden)]
>  pub use bindings;
> diff --git a/rust/kernel/zpool.rs b/rust/kernel/zpool.rs
> new file mode 100644
> index 000000000000..91926c2e99e8
> --- /dev/null
> +++ b/rust/kernel/zpool.rs
> @@ -0,0 +1,269 @@
> +use crate::{
> +    bindings,
> +    error::Result,
> +    kernel::alloc::Flags,
> +    str::CStr,
> +    types::{ForeignOwnable, Opaque},
> +};
> +use core::ffi::{c_int, c_uchar, c_void};
> +use core::ptr::null_mut;
> +use kernel::alloc::NumaNode;
> +use kernel::ThisModule;
> +
> +/// zpool API
> +pub trait Zpool {
> +    /// Opaque Rust representation of `struct zpool`.
> +    type Pool: ForeignOwnable;
> +
> +    /// Create a pool.
> +    fn create(name: *const c_uchar, gfp: Flags) -> Result<Self::Pool>;
> +
> +    /// Destroy the pool.
> +    fn destroy(pool: Self::Pool);
> +
> +    /// Allocate an object of size `size` using GFP flags `gfp` from the pool `pool`, wuth the
> +    /// preferred NUMA node `nid`. If the allocation is successful, an opaque handle is returned.
> +    fn malloc(
> +        pool: <Self::Pool as ForeignOwnable>::BorrowedMut<'_>,
> +        size: usize,
> +        gfp: Flags,
> +        nid: NumaNode,
> +    ) -> Result<usize>;
> +
> +    /// Free a previously allocated from the `pool` object, represented by `handle`.
> +    fn free(pool: <Self::Pool as ForeignOwnable>::Borrowed<'_>, handle: usize);
> +
> +    /// Make all the necessary preparations for the caller to be able to read from the object
> +    /// represented by `handle` and return a valid pointer to the `handle` memory to be read.
> +    fn read_begin(pool: <Self::Pool as ForeignOwnable>::Borrowed<'_>, handle: usize)
> +        -> *mut c_void;
> +
> +    /// Finish reading from a previously allocated `handle`. `handle_mem` must be the pointer
> +    /// previously returned by `read_begin`.
> +    fn read_end(
> +        pool: <Self::Pool as ForeignOwnable>::Borrowed<'_>,
> +        handle: usize,
> +        handle_mem: *mut c_void,
> +    );
> +
> +    /// Write to the object represented by a previously allocated `handle`. `handle_mem` points
> +    /// to the memory to copy data from, and `mem_len` defines the length of the data block to
> +    /// be copied.
> +    fn write(
> +        pool: <Self::Pool as ForeignOwnable>::Borrowed<'_>,
> +        handle: usize,
> +        handle_mem: *mut c_void,
> +        mem_len: usize,
> +    );
> +
> +    /// Get the number of pages used by the `pool`.
> +    fn total_pages(pool: <Self::Pool as ForeignOwnable>::Borrowed<'_>) -> u64;
> +}
> +
> +/// Zpool driver registration trait.
> +pub trait Registration {
> +    /// Register a zpool driver.
> +    fn register(&self, name: &'static CStr, module: &'static ThisModule) -> Result;
> +
> +    /// Pool creation callback.
> +    extern "C" fn _create(name: *const c_uchar, gfp: u32) -> *mut c_void;
> +
> +    /// Pool destruction callback.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The caller must ensure that `pool` is a valid pointer to `struct zpool`.
> +    unsafe extern "C" fn _destroy(pool: *mut c_void);
> +
> +    /// Callback for object allocation in the pool.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The caller must ensure that `pool` is a valid pointer to `struct zpool` and that `handle`
> +    /// is a valid pointer to usize.
> +    unsafe extern "C" fn _malloc(
> +        pool: *mut c_void,
> +        size: usize,
> +        gfp: u32,
> +        handle: *mut usize,
> +        nid: c_int,
> +    ) -> c_int;
> +
> +    /// Callback for object release.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The caller must ensure that `pool` is a valid pointer to `struct zpool`.
> +    unsafe extern "C" fn _free(pool: *mut c_void, handle: usize);
> +
> +    /// Callback to prepare the object for reading.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The caller must ensure that `pool` is a valid pointer to `struct zpool`.
> +    unsafe extern "C" fn _obj_read_begin(
> +        pool: *mut c_void,
> +        handle: usize,
> +        local_copy: *mut c_void,
> +    ) -> *mut c_void;
> +
> +    /// Callback to signal the end of reading from an object.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The caller must ensure that `pool` is a valid pointer to `struct zpool`.
> +    unsafe extern "C" fn _obj_read_end(pool: *mut c_void, handle: usize, handle_mem: *mut c_void);
> +
> +    /// Callback for writing to an object.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The caller must ensure that `pool` is a valid pointer to `struct zpool`.
> +    unsafe extern "C" fn _obj_write(
> +        pool: *mut c_void,
> +        handle: usize,
> +        handle_mem: *mut c_void,
> +        mem_len: usize,
> +    );
> +
> +    /// Callback to return the number of pages in the pool.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The caller must ensure that `pool` is a valid pointer to `struct zpool`.
> +    unsafe extern "C" fn _total_pages(pool: *mut c_void) -> u64;
> +}
> +
> +/// Zpool driver structure.
> +pub struct ZpoolDriver<T: Zpool> {
> +    inner: Opaque<bindings::zpool_driver>,
> +
> +    /// Zpool callback functions that a zpool driver must provide
> +    pub callbacks: T,
> +}
> +
> +impl<T: Zpool> Clone for ZpoolDriver<T> {
> +    fn clone(&self) -> Self {
> +        todo!()
> +    }
> +}
> +
> +// SAFETY: zpool driver must ensure that ZpoolDriver's `callbacks` are thread safe
> +unsafe impl<T: Zpool> Sync for ZpoolDriver<T> {}
> +
> +impl<T: Zpool> ZpoolDriver<T> {
> +    /// create an instance of a zpool driver
> +    pub const fn new(t: T) -> Self {
> +        Self {
> +            inner: Opaque::uninit(),
> +            callbacks: t,
> +        }
> +    }
> +}
> +
> +impl<T: Zpool> Registration for ZpoolDriver<T> {
> +    extern "C" fn _create(name: *const c_uchar, gfp: u32) -> *mut c_void {
> +        let pool = T::create(name, Flags::new(gfp));
> +        match pool {
> +            Err(_) => null_mut(),
> +            Ok(p) => T::Pool::into_foreign(p),
> +        }
> +    }
> +    unsafe extern "C" fn _destroy(pool: *mut c_void) {
> +        // SAFETY: The pointer originates from an `into_foreign` call.
> +        T::destroy(unsafe { T::Pool::from_foreign(pool) })
> +    }
> +    unsafe extern "C" fn _malloc(
> +        pool: *mut c_void,
> +        size: usize,
> +        gfp: u32,
> +        handle: *mut usize,
> +        nid: c_int,
> +    ) -> c_int {
> +        // SAFETY: The pointer originates from an `into_foreign` call. If `pool` is passed to
> +        // `from_foreign`, then that happens in `_destroy` which will not be called during this
> +        // method.
> +        let pool = unsafe { T::Pool::borrow_mut(pool) };
> +        let real_nid = match nid {
> +            bindings::NUMA_NO_NODE => Ok(NumaNode::NO_NODE),
> +            _ => NumaNode::new(nid),
> +        };
> +        if real_nid.is_err() {
> +            return -(bindings::EINVAL as i32);
> +        }
> +
> +        let result = T::malloc(pool, size, Flags::new(gfp), real_nid.unwrap());
> +        match result {
> +            Err(_) => -(bindings::ENOMEM as i32),
> +            Ok(h) => {
> +                // SAFETY: handle is guaranteed to be a valid pointer by zpool
> +                unsafe { *handle = h };
> +                0
> +            }
> +        }
> +    }
> +    unsafe extern "C" fn _free(pool: *mut c_void, handle: usize) {
> +        // SAFETY: The pointer originates from an `into_foreign` call. If `pool` is passed to
> +        // `from_foreign`, then that happens in `_destroy` which will not be called during this
> +        // method.
> +        let pool = unsafe { T::Pool::borrow(pool) };
> +        T::free(pool, handle)
> +    }
> +    unsafe extern "C" fn _obj_read_begin(
> +        pool: *mut c_void,
> +        handle: usize,
> +        _local_copy: *mut c_void,
> +    ) -> *mut c_void {
> +        // SAFETY: The pointer originates from an `into_foreign` call. If `pool` is passed to
> +        // `from_foreign`, then that happens in `_destroy` which will not be called during this
> +        // method.
> +        let pool = unsafe { T::Pool::borrow(pool) };
> +        T::read_begin(pool, handle)
> +    }
> +    unsafe extern "C" fn _obj_read_end(pool: *mut c_void, handle: usize, handle_mem: *mut c_void) {
> +        // SAFETY: The pointer originates from an `into_foreign` call. If `pool` is passed to
> +        // `from_foreign`, then that happens in `_destroy` which will not be called during this
> +        // method.
> +        let pool = unsafe { T::Pool::borrow(pool) };
> +        T::read_end(pool, handle, handle_mem)
> +    }
> +    unsafe extern "C" fn _obj_write(
> +        pool: *mut c_void,
> +        handle: usize,
> +        handle_mem: *mut c_void,
> +        mem_len: usize,
> +    ) {
> +        // SAFETY: The pointer originates from an `into_foreign` call. If `pool` is passed to
> +        // `from_foreign`, then that happens in `_destroy` which will not be called during this
> +        // method.
> +        let pool = unsafe { T::Pool::borrow(pool) };
> +        T::write(pool, handle, handle_mem, mem_len);
> +    }
> +    unsafe extern "C" fn _total_pages(pool: *mut c_void) -> u64 {
> +        // SAFETY: The pointer originates from an `into_foreign` call. If `pool` is passed to
> +        // `from_foreign`, then that happens in `_destroy` which will not be called during this
> +        // method.
> +        let pool = unsafe { T::Pool::borrow(pool) };
> +        T::total_pages(pool)
> +    }
> +
> +    fn register(&self, name: &'static CStr, module: &'static ThisModule) -> Result {
> +        // SAFETY: `ZpoolDriver::new()` ensures that `self.inner` is a valid pointer
> +        unsafe {
> +            (*(self.inner.get())).create = Some(Self::_create);
> +            (*(self.inner.get())).destroy = Some(Self::_destroy);
> +            (*(self.inner.get())).malloc = Some(Self::_malloc);
> +            (*(self.inner.get())).free = Some(Self::_free);
> +            (*(self.inner.get())).obj_read_begin = Some(Self::_obj_read_begin);
> +            (*(self.inner.get())).obj_read_end = Some(Self::_obj_read_end);
> +            (*(self.inner.get())).obj_write = Some(Self::_obj_write);
> +            (*(self.inner.get())).total_pages = Some(Self::_total_pages);
> +
> +            (*(self.inner.get())).owner = module.0;
> +            (*(self.inner.get())).type_ = name.as_char_ptr().cast_mut();
> +
> +            bindings::zpool_register_driver(self.inner.get());
> +        }
> +        Ok(())
> +    }
> +}
> -- 
> 2.39.2

Reviewed-by: Elle Rhumsaa <elle@weathered-steel.dev>