Re: [PATCH] rust: slab: add basic slab module

Benno Lossin
2025-09-25T08:01:51Z

On Wed Sep 24, 2025 at 9:36 PM CEST, Elijah Wright wrote:
> this patch adds a basic slab module for kmem_cache, primarily wrapping
> kmem_cache_create, kmem_cache_alloc, kmem_cache_free, and kmem_cache_destroy.

IIRC, Alice had an idea for how to use our allocator API to create
idiomatic kmem_cache abstractions. So we might want to explore that too?

> Signed-off-by: Elijah Wright <git@elijahs.space>
> ---
> rust/helpers/slab.c | 10 ++++++
> rust/kernel/lib.rs | 1 +
> rust/kernel/slab.rs | 85 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 96 insertions(+)
> create mode 100644 rust/kernel/slab.rs
>
> diff --git a/rust/helpers/slab.c b/rust/helpers/slab.c
> index a842bfbddcba..799de7bc1405 100644
> --- a/rust/helpers/slab.c
> +++ b/rust/helpers/slab.c
> @@ -13,3 +13,13 @@ rust_helper_kvrealloc(const void *p, size_t size, gfp_t flags)
> {
> return kvrealloc(p, size, flags);
> }
> +
> +struct kmem_cache * rust_helper_kmem_cache_create(const char *name, unsigned int size, unsigned int align, gfp_t flags, void (*ctor)(void *))
> +{
> + return kmem_cache_create(name, size, align, flags, NULL);
> +}
> +
> +void * rust_helper_kmem_cache_alloc(struct kmem_cache *cachep, gfp_t flags)
> +{
> + return kmem_cache_alloc(cachep, flags);
> +}
> \ No newline at end of file
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index fef97f2a5098..bd76eadbe297 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -116,6 +116,7 @@
> pub mod security;
> pub mod seq_file;
> pub mod sizes;
> +pub mod slab;
> mod static_assert;
> #[doc(hidden)]
> pub mod std_vendor;
> diff --git a/rust/kernel/slab.rs b/rust/kernel/slab.rs
> new file mode 100644
> index 000000000000..8b418f9db7cb
> --- /dev/null
> +++ b/rust/kernel/slab.rs
> @@ -0,0 +1,85 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Slab bindings.
> +//!
> +//! C header: [`include/linux/slab.h`](srctree/include/linux/slab.h)
> +
> +use core::{marker::PhantomData, mem, ptr::NonNull};
> +
> +use crate::{
> + alloc::Flags,
> + bindings,
> + error::{code::ENOMEM, Result},
> + str::CStr,
> +};
> +
> +/// A wrapper for kmem_cache that allocates objects of type `T`.
> +#[repr(transparent)]
> +pub struct Slab<T> {
> + cache: NonNull<bindings::kmem_cache>,

You need an invariant for this pointer to point at a valid
kmem_cache for values of layout `T`.

> + _p: PhantomData<T>,
> +}
> +
> +impl<T> Slab<T> {
> + /// Creates a cache for objects of type `T`.
> + pub fn try_new(name: &CStr, flags: Flags) -> Result<Self> {
> + let size = mem::size_of::<T>();
> + let align = mem::align_of::<T>();
> + debug_assert!(size <= usize::MAX);
> + debug_assert!(align <= usize::MAX);

These are always true? Did you mean to use `u32::MAX`?

> +
> + // SAFETY: `flags` is a valid impl, `name` is a valid C string, and
> + // other arguments are plain values.

But you also care about them being correct for the invariant I mentioned
above.

> + let cache = unsafe {
> + bindings::kmem_cache_create(
> + name.as_char_ptr(),
> + size as u32,
> + align as u32,

What do you do if they don't fit in u32? This will just produce wrong
code? I think you should return `EINVAL`. (maybe use `TryFrom` & then
convert the error)

> + flags.as_raw(),
> + None,
> + )
> + };
> +
> + NonNull::new(cache)
> + .map(|c| Slab {
> + cache: c,
> + _p: PhantomData,
> + })
> + .ok_or(ENOMEM)
> + }
> +
> + /// Allocates one object from the cache with the given gfp flags.
> + #[inline]
> + pub fn alloc(&self, flags: Flags) -> Result<NonNull<T>> {
> + // SAFETY: `self.cache` is a valid pointer obtained from
> + // `kmem_cache_create` and still alive because `self` is borrowed.
> + let ptr = unsafe { bindings::kmem_cache_alloc(self.cache.as_ptr(), flags.as_raw()) };
> + NonNull::new(ptr.cast()).ok_or(ENOMEM)
> + }

I don't like that this API makes the user handle `NonNull` directly...

> +
> + /// Frees an object previously returned by `alloc()`.
> + ///
> + /// # Safety
> + /// The caller must guarantee that `obj` was allocated from this cache and
> + /// is no longer accessed afterwards.
> + #[inline]
> + pub unsafe fn free(&self, obj: NonNull<T>) {
> + // SAFETY: By the safety contract the pointer is valid and unique at
> + // this point.
> + unsafe { bindings::kmem_cache_free(self.cache.as_ptr(), obj.cast().as_ptr()) };
> + }

This similarly looks bad, we should have some kind of `KmemCacheBox` or
use the approach that Alice had in mind.

---
Cheers,
Benno

> +
> + /// Returns the raw mutable pointer to the cache
> + #[inline]
> + pub fn as_ptr(&self) -> *mut bindings::kmem_cache {
> + self.cache.as_ptr()
> + }
> +}
> +
> +impl<T> Drop for Slab<T> {
> + fn drop(&mut self) {
> + // SAFETY: `self.cache` is valid and we are the final owner because
> + // of ownership rules.
> + unsafe { bindings::kmem_cache_destroy(self.cache.as_ptr()) };
> + }
> +}