Generic `Allocator` support for Rust

Add a kernel specific Allocator trait, that in contrast to the one in Rust's core library doesn't require unstable features and supports GFP flags.

Subsequent patches add the following trait implementors: Kmalloc, Vmalloc and KVmalloc.

Reviewed-by Alice Ryhl <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

---
 rust/kernel/alloc.rs | 81 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

rust/kernel/alloc.rs +8 -0

index 1966bd407017..194745498a75 100644
@@ -11,6 +11,7 @@
 /// Indicates an allocation error.
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
 pub struct AllocError;
+use core::{alloc::Layout, ptr::NonNull};
 
 /// Flags to be used when allocating memory.
 ///

@@ -86,3 +87,83 @@ pub mod flags {
     /// small allocations.
     pub const GFP_NOWAIT: Flags = Flags(bindings::GFP_NOWAIT);
 }
+
+/// The kernel's [`Allocator`] trait.
+///
+/// An implementation of [`Allocator`] can allocate, re-allocate and free memory buffer described
+/// via [`Layout`].
+///
+/// [`Allocator`] is designed to be implemented as a ZST; [`Allocator`] functions do not operate on

rust/kernel/alloc.rs +4 -0

+///
+/// Any pointer to a memory buffer which is currently allocated must be valid to be passed to any
+/// other [`Allocator`] function of the same type.
+///

rust/kernel/alloc.rs +5 -0

+    ///
+    /// If the requested size is larger than the size of the existing allocation, a successful call
+    /// to `realloc` guarantees that the new or grown buffer has at least `Layout::size` bytes, but
+    /// may also be larger.
+    ///

rust/kernel/alloc.rs +21 -1

+    unsafe fn realloc(
+        ptr: Option<NonNull<u8>>,
+        layout: Layout,
+        flags: Flags,
+    ) -> Result<NonNull<[u8]>, AllocError>;
 
+
+    /// Free an existing memory allocation.
+    ///
+    /// # Safety
+    ///
+    /// `ptr` must point to an existing and valid memory allocation created by this `Allocator` and
+    /// must not be a dangling pointer.
+    ///
+    /// The memory allocation at `ptr` must never again be read from or written to.
+    unsafe fn free(ptr: NonNull<u8>) {
+        // SAFETY: The caller guarantees that `ptr` points at a valid allocation created by this
+        // allocator. We are passing a `Layout` with the smallest possible alignment, so it is
+        // smaller than or equal to the alignment previously used with this allocation.
+        let _ = unsafe { Self::realloc(Some(ptr), Layout::new::<()>(), Flags(0)) };
+    }
+}
--

2.45.2

Quoted 43 lines · Danilo Krummrich

Add a kernel specific Allocator trait, that in contrast to the one in Rust's core library doesn't require unstable features and supports GFP flags.

Subsequent patches add the following trait implementors: Kmalloc, Vmalloc and KVmalloc.

Reviewed-by Alice Ryhl <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

---
 rust/kernel/alloc.rs | 81 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

rust/kernel/alloc.rs +15 -0

index 1966bd407017..194745498a75 100644
@@ -11,6 +11,7 @@
 /// Indicates an allocation error.
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
 pub struct AllocError;
+use core::{alloc::Layout, ptr::NonNull};
 
 /// Flags to be used when allocating memory.
 ///

@@ -86,3 +87,83 @@ pub mod flags {
     /// small allocations.
     pub const GFP_NOWAIT: Flags = Flags(bindings::GFP_NOWAIT);
 }
+
+/// The kernel's [`Allocator`] trait.
+///
+/// An implementation of [`Allocator`] can allocate, re-allocate and free memory buffer described
+/// via [`Layout`].
+///
+/// [`Allocator`] is designed to be implemented as a ZST; [`Allocator`] functions do not operate on
+/// an object instance.
+///
+/// In order to be able to support `#[derive(SmartPointer)]` later on, we need to avoid a design
+/// that requires an `Allocator` to be instantiated, hence its functions must not contain any kind
+/// of `self` parameter.
+///
+/// # Safety

rust/kernel/alloc.rs pub mod flags { · line 106

Quoted 3 lines

rust/kernel/alloc.rs +3 -0

+///
+/// Memory returned from an allocator must point to a valid memory buffer and remain valid until
+/// it is explicitly freed.

I wouldn't say that an allocator "returns memory", and in general I don't think the structure of the safety comment here is nice, how about the following: we put "Implementers must ensure that all trait functions abide by the guarantees documented in the # Guarantees sections."...

Quoted 5 lines

rust/kernel/alloc.rs +4 -0

+///
+/// Any pointer to a memory buffer which is currently allocated must be valid to be passed to any
+/// other [`Allocator`] function of the same type.
+///

rust/kernel/alloc.rs pub mod flags { · line 120

Quoted 10 lines

rust/kernel/alloc.rs +10 -0

+/// If `realloc` is called with:
+///   - a size of zero, the given memory allocation, if any, must be freed
+///   - `None`, a new memory allocation must be created
+pub unsafe trait Allocator {
+    /// Allocate memory based on `layout` and `flags`.
+    ///
+    /// On success, returns a buffer represented as `NonNull<[u8]>` that satisfies the layout
+    /// constraints (i.e. minimum size and alignment as specified by `layout`).
+    ///
+    /// This function is equivalent to `realloc` when called with `None`.

... Then we can add this here:

    /// # Guarantees
    ///
    /// When the return value is `Ok(ptr)`, then `ptr` is
    /// - valid for writes (and reads after the memory has been initialized) for `layout.size()` bytes,
    ///   until it is passed to [`Allocator::free`] or [`Allocator::realloc`],
    /// - aligned to `layout.align()`,
    /// - is valid for reads, if `flags.contains(flags::__GFP_ZERO)`,

Do we need to handle other flags?

Also IIRC the memory given to us by C is considered initialized by Rust (though it has a non-deterministic value), so we might have an unconditional "valid for reads". Am I correct?

rust/kernel/alloc.rs pub mod flags { · line 128

Quoted 10 lines

rust/kernel/alloc.rs +8 -0

+    fn alloc(layout: Layout, flags: Flags) -> Result<NonNull<[u8]>, AllocError> {
+        // SAFETY: Passing `None` to `realloc` is valid by it's safety requirements and asks for a
+        // new memory allocation.
+        unsafe { Self::realloc(None, layout, flags) }
+    }
+
+    /// Re-allocate an existing memory allocation to satisfy the requested `layout`. If the
+    /// requested size is zero, `realloc` behaves equivalent to `free`.

I don't think we want to include the second sentence in the short description of this function, please add an empty line in between.

Quoted 5 lines

rust/kernel/alloc.rs +5 -0

+    ///
+    /// If the requested size is larger than the size of the existing allocation, a successful call
+    /// to `realloc` guarantees that the new or grown buffer has at least `Layout::size` bytes, but
+    /// may also be larger.
+    ///

rust/kernel/alloc.rs pub mod flags { · line 143

Quoted 10 lines

rust/kernel/alloc.rs +10 -0

+    /// If the requested size is smaller than the size of the existing allocation, `realloc` may or
+    /// may not shrink the buffer; this is implementation specific to the allocator.
+    ///
+    /// On allocation failure, the existing buffer, if any, remains valid.
+    ///
+    /// The buffer is represented as `NonNull<[u8]>`.
+    ///
+    /// # Safety
+    ///
+    /// If `ptr = Some(p)`, then `p` must point to an existing and valid memory allocation created

I don't like the single = (I might have written it in haste myself), how about == or we use if-let syntax?

rust/kernel/alloc.rs pub mod flags { · line 148

Quoted 5 lines

rust/kernel/alloc.rs +5 -0

+    /// by this allocator. The alignment encoded in `layout` must be smaller than or equal to the
+    /// alignment requested in the previous `alloc` or `realloc` call of the same allocation.
+    ///
+    /// Additionally, `ptr` is allowed to be `None`; in this case a new memory allocation is
+    /// created.

This Safety section does not talk about the case layout.size() == 0, but it should have the same requirement as free().

Also add a # Guarantees section here:

    /// # Guarantees
    ///
    /// This function has the same guarantees as [`Allocator::alloc`]. When `ptr == Some(p)`, then it
    /// additionally has the following:
    /// - when `Ok(ret_ptr)` is the return value, then
    ///   `ret_ptr[0..min(layout.size(), old_size)] == p[0..min(layout.size(), old_size)]`, where
    ///   `old_size` is the size of the allocation that `p` points at.

    /// - when the return value is `Err(AllocError)`, then `p` is still valid.

---
Cheers,
Benno

Quoted 25 lines

rust/kernel/alloc.rs +21 -1

+    unsafe fn realloc(
+        ptr: Option<NonNull<u8>>,
+        layout: Layout,
+        flags: Flags,
+    ) -> Result<NonNull<[u8]>, AllocError>;
 
+
+    /// Free an existing memory allocation.
+    ///
+    /// # Safety
+    ///
+    /// `ptr` must point to an existing and valid memory allocation created by this `Allocator` and
+    /// must not be a dangling pointer.
+    ///
+    /// The memory allocation at `ptr` must never again be read from or written to.
+    unsafe fn free(ptr: NonNull<u8>) {
+        // SAFETY: The caller guarantees that `ptr` points at a valid allocation created by this
+        // allocator. We are passing a `Layout` with the smallest possible alignment, so it is
+        // smaller than or equal to the alignment previously used with this allocation.
+        let _ = unsafe { Self::realloc(Some(ptr), Layout::new::<()>(), Flags(0)) };
+    }
+}
--

2.45.2

Quoted 10 lines · Benno Lossin

rust/kernel/alloc.rs pub mod flags { · line 106

Quoted 4 lines · Danilo Krummrich

rust/kernel/alloc.rs +4 -0

+/// # Safety
+///
+/// Memory returned from an allocator must point to a valid memory buffer and remain valid until
+/// it is explicitly freed.

I wouldn't say that an allocator "returns memory", and in general I don't think the structure of the safety comment here is nice, how about the following: we put "Implementers must ensure that all trait functions abide by the guarantees documented in the # Guarantees sections."...

Sounds reasonable to me. Additionally, I'd still keep the part below, that says that any pointer to a memory allocation must bbe valid to be passed to any other [Allocator] function of the same type.

Quoted 7 lines

rust/kernel/alloc.rs pub mod flags { · line 113

Quoted 7 lines

rust/kernel/alloc.rs +7 -0

+///
+/// Any pointer to a memory buffer which is currently allocated must be valid to be passed to any
+/// other [`Allocator`] function of the same type.
+///
+/// If `realloc` is called with:
+///   - a size of zero, the given memory allocation, if any, must be freed
+///   - `None`, a new memory allocation must be created

Quoted 18 lines

rust/kernel/alloc.rs pub mod flags { · line 120

Quoted 7 lines

rust/kernel/alloc.rs +7 -0

+pub unsafe trait Allocator {
+    /// Allocate memory based on `layout` and `flags`.
+    ///
+    /// On success, returns a buffer represented as `NonNull<[u8]>` that satisfies the layout
+    /// constraints (i.e. minimum size and alignment as specified by `layout`).
+    ///
+    /// This function is equivalent to `realloc` when called with `None`.

... Then we can add this here:

    /// # Guarantees
    ///
    /// When the return value is `Ok(ptr)`, then `ptr` is
    /// - valid for writes (and reads after the memory has been initialized) for `layout.size()` bytes,
    ///   until it is passed to [`Allocator::free`] or [`Allocator::realloc`],
    /// - aligned to `layout.align()`,
    /// - is valid for reads, if `flags.contains(flags::__GFP_ZERO)`,

Quoted 1 line

Do we need to handle other flags?

The whole flags thing is a bit difficult to represent here properly.

Theoretically, we'd need to add that it guarantees that the memory is zeroed for __GFP_ZERO, non-blocking for GFP_NOWAIT, etc. But, I think we shouldn't re-iterate all different behavior for the different flags.

Another inconvenience is that not all page flags are honored or make sense for all allocators. This is especially inconvenient for KVmalloc where we can't even say if we end up in vrealloc() or krealloc(). kvmalloc() even contains a couple of flag fixups for this reason [2].

I think we should just point to [1], which should document everything already.

[1] https://elixir.bootlin.com/linux/v6.10.4/source/include/linux/gfp_types.h
[2] https://elixir.bootlin.com/linux/v6.10.4/source/mm/util.c#L612

Quoted 3 lines

Also IIRC the memory given to us by C is considered initialized by Rust (though it has a non-deterministic value), so we might have an unconditional "valid for reads". Am I correct?

Yes, but as you say, unless allocated with __GFP_ZERO, it contains non-deterministic data. It may even contain old data from previous allocations.

Quoted 11 lines

rust/kernel/alloc.rs pub mod flags { · line 148

Quoted 5 lines

rust/kernel/alloc.rs +5 -0

+    /// by this allocator. The alignment encoded in `layout` must be smaller than or equal to the
+    /// alignment requested in the previous `alloc` or `realloc` call of the same allocation.
+    ///
+    /// Additionally, `ptr` is allowed to be `None`; in this case a new memory allocation is
+    /// created.

This Safety section does not talk about the case layout.size() == 0, but it should have the same requirement as free().

Also add a # Guarantees section here:

Quoted 7 lines

    /// # Guarantees
    ///
    /// This function has the same guarantees as [`Allocator::alloc`]. When `ptr == Some(p)`, then it
    /// additionally has the following:
    /// - when `Ok(ret_ptr)` is the return value, then
    ///   `ret_ptr[0..min(layout.size(), old_size)] == p[0..min(layout.size(), old_size)]`, where
    ///   `old_size` is the size of the allocation that `p` points at.

We could also say "The contents of the memory pointed to by p are preserved up to the lesser of the new and old size." But I'm fine with both.

Implement Allocator for Kmalloc, the kernel's default allocator, typically used for objects smaller than page size.

All memory allocations made with Kmalloc end up in krealloc().

It serves as allocator for the subsequently introduced types KBox and KVec.

Signed-off-by Danilo Krummrich <[email protected]>

---
 rust/helpers.c                 |  3 +-
 rust/kernel/alloc.rs           |  2 +-
 rust/kernel/alloc/allocator.rs | 63 +++++++++++++++++++++++++++++++++-
 3 files changed, 64 insertions(+), 4 deletions(-)

rust/helpers.c +0 -0

index 92d3c03ae1bd..9f7275493365 100644

Quoted 24 lines · Danilo Krummrich

Implement Allocator for Kmalloc, the kernel's default allocator, typically used for objects smaller than page size.

All memory allocations made with Kmalloc end up in krealloc().

It serves as allocator for the subsequently introduced types KBox and KVec.

Signed-off-by Danilo Krummrich <[email protected]>

---
 rust/helpers.c                 |  3 +-
 rust/kernel/alloc.rs           |  2 +-
 rust/kernel/alloc/allocator.rs | 63 +++++++++++++++++++++++++++++++++-
 3 files changed, 64 insertions(+), 4 deletions(-)

rust/helpers.c +0 -0

index 92d3c03ae1bd..9f7275493365 100644
@@ -193,8 +193,7 @@ void rust_helper_init_work_with_key(struct work_struct *work, work_func_t func,
 }
 EXPORT_SYMBOL_GPL(rust_helper_init_work_with_key);
 

rust/helpers.c void rust_helper_init_work_with_key(struct work_struct *work, work_func_t func, · line 199

Quoted 6 lines

rust/helpers.c +1 -2

-void * __must_check __realloc_size(2)
-rust_helper_krealloc(const void *objp, size_t new_size, gfp_t flags)
+void *rust_helper_krealloc(const void *objp, size_t new_size, gfp_t flags)
 {
        return krealloc(objp, new_size, flags);
 }

Why are the various annotations on this helper being removed?

This deserves an explanation in the commit message.

Alice

Quoted 25 lines · Alice Ryhl

Quoted 24 lines · Danilo Krummrich

Implement Allocator for Kmalloc, the kernel's default allocator, typically used for objects smaller than page size.

All memory allocations made with Kmalloc end up in krealloc().

It serves as allocator for the subsequently introduced types KBox and KVec.

Signed-off-by Danilo Krummrich <[email protected]>

---
 rust/helpers.c                 |  3 +-
 rust/kernel/alloc.rs           |  2 +-
 rust/kernel/alloc/allocator.rs | 63 +++++++++++++++++++++++++++++++++-
 3 files changed, 64 insertions(+), 4 deletions(-)

rust/helpers.c +0 -0

index 92d3c03ae1bd..9f7275493365 100644
@@ -193,8 +193,7 @@ void rust_helper_init_work_with_key(struct work_struct *work, work_func_t func,
 }
 EXPORT_SYMBOL_GPL(rust_helper_init_work_with_key);
 

Quoted 8 lines

rust/helpers.c void rust_helper_init_work_with_key(struct work_struct *work, work_func_t func, · line 199

Quoted 6 lines

rust/helpers.c +1 -2

-void * __must_check __realloc_size(2)
-rust_helper_krealloc(const void *objp, size_t new_size, gfp_t flags)
+void *rust_helper_krealloc(const void *objp, size_t new_size, gfp_t flags)
 {
        return krealloc(objp, new_size, flags);
 }

Why are the various annotations on this helper being removed?

rust_helper_krealloc() is only called from Rust, hence neither __must_check nor __realloc_size() should have any effect.

I also do not apply them in subsequent commits for the vrealloc() and kvrealloc() helpers for this reason and removed them here for consistency.

Quoted 1 line

This deserves an explanation in the commit message.

I can also add a separate commit for that.

Quoted 39 lines · Danilo Krummrich

Quoted 25 lines · Alice Ryhl

Quoted 24 lines · Danilo Krummrich

Implement Allocator for Kmalloc, the kernel's default allocator, typically used for objects smaller than page size.

All memory allocations made with Kmalloc end up in krealloc().

It serves as allocator for the subsequently introduced types KBox and KVec.

Signed-off-by Danilo Krummrich <[email protected]>

---
 rust/helpers.c                 |  3 +-
 rust/kernel/alloc.rs           |  2 +-
 rust/kernel/alloc/allocator.rs | 63 +++++++++++++++++++++++++++++++++-
 3 files changed, 64 insertions(+), 4 deletions(-)

rust/helpers.c +0 -0

index 92d3c03ae1bd..9f7275493365 100644
@@ -193,8 +193,7 @@ void rust_helper_init_work_with_key(struct work_struct *work, work_func_t func,
 }
 EXPORT_SYMBOL_GPL(rust_helper_init_work_with_key);
 

Quoted 8 lines

rust/helpers.c void rust_helper_init_work_with_key(struct work_struct *work, work_func_t func, · line 199

Quoted 6 lines

rust/helpers.c +1 -2

-void * __must_check __realloc_size(2)
-rust_helper_krealloc(const void *objp, size_t new_size, gfp_t flags)
+void *rust_helper_krealloc(const void *objp, size_t new_size, gfp_t flags)
 {
        return krealloc(objp, new_size, flags);
 }

Why are the various annotations on this helper being removed?

rust_helper_krealloc() is only called from Rust, hence neither __must_check nor __realloc_size() should have any effect.

Quoted 6 lines

I also do not apply them in subsequent commits for the vrealloc() and kvrealloc() helpers for this reason and removed them here for consistency.

Quoted 1 line

This deserves an explanation in the commit message.

I can also add a separate commit for that.

I think your change would be more obviously correct if you keep them.

Alice

Quoted 47 lines · Alice Ryhl

Quoted 45 lines · Danilo Krummrich

Quoted 25 lines · Alice Ryhl

Quoted 24 lines · Danilo Krummrich

Implement Allocator for Kmalloc, the kernel's default allocator, typically used for objects smaller than page size.

All memory allocations made with Kmalloc end up in krealloc().

It serves as allocator for the subsequently introduced types KBox and KVec.

Signed-off-by Danilo Krummrich <[email protected]>

---
 rust/helpers.c                 |  3 +-
 rust/kernel/alloc.rs           |  2 +-
 rust/kernel/alloc/allocator.rs | 63 +++++++++++++++++++++++++++++++++-
 3 files changed, 64 insertions(+), 4 deletions(-)

rust/helpers.c +0 -0

index 92d3c03ae1bd..9f7275493365 100644
@@ -193,8 +193,7 @@ void rust_helper_init_work_with_key(struct work_struct *work, work_func_t func,
 }
 EXPORT_SYMBOL_GPL(rust_helper_init_work_with_key);
 

Quoted 8 lines

rust/helpers.c void rust_helper_init_work_with_key(struct work_struct *work, work_func_t func, · line 199

Quoted 6 lines

rust/helpers.c +1 -2

-void * __must_check __realloc_size(2)
-rust_helper_krealloc(const void *objp, size_t new_size, gfp_t flags)
+void *rust_helper_krealloc(const void *objp, size_t new_size, gfp_t flags)
 {
        return krealloc(objp, new_size, flags);
 }

Why are the various annotations on this helper being removed?

rust_helper_krealloc() is only called from Rust, hence neither __must_check nor __realloc_size() should have any effect.

I also do not apply them in subsequent commits for the vrealloc() and kvrealloc() helpers for this reason and removed them here for consistency.

Quoted 1 line

This deserves an explanation in the commit message.

I can also add a separate commit for that.

Quoted 1 line

I think your change would be more obviously correct if you keep them.

As in generally, or just for this patch?

Generally, I don't think we should indicate compiler checks that actually are never done.

For this patch, yes, it's probably better to separate it.

Quoted 53 lines · Danilo Krummrich

Quoted 40 lines · Alice Ryhl

Quoted 39 lines · Danilo Krummrich

Quoted 25 lines · Alice Ryhl

Quoted 24 lines · Danilo Krummrich

Implement Allocator for Kmalloc, the kernel's default allocator, typically used for objects smaller than page size.

All memory allocations made with Kmalloc end up in krealloc().

It serves as allocator for the subsequently introduced types KBox and KVec.

Signed-off-by Danilo Krummrich <[email protected]>

---
 rust/helpers.c                 |  3 +-
 rust/kernel/alloc.rs           |  2 +-
 rust/kernel/alloc/allocator.rs | 63 +++++++++++++++++++++++++++++++++-
 3 files changed, 64 insertions(+), 4 deletions(-)

rust/helpers.c +0 -0

index 92d3c03ae1bd..9f7275493365 100644
@@ -193,8 +193,7 @@ void rust_helper_init_work_with_key(struct work_struct *work, work_func_t func,
 }
 EXPORT_SYMBOL_GPL(rust_helper_init_work_with_key);
 

Quoted 8 lines

rust/helpers.c void rust_helper_init_work_with_key(struct work_struct *work, work_func_t func, · line 199

Quoted 6 lines

rust/helpers.c +1 -2

-void * __must_check __realloc_size(2)
-rust_helper_krealloc(const void *objp, size_t new_size, gfp_t flags)
+void *rust_helper_krealloc(const void *objp, size_t new_size, gfp_t flags)
 {
        return krealloc(objp, new_size, flags);
 }

Why are the various annotations on this helper being removed?

rust_helper_krealloc() is only called from Rust, hence neither __must_check nor __realloc_size() should have any effect.

Quoted 8 lines

Quoted 6 lines

I also do not apply them in subsequent commits for the vrealloc() and kvrealloc() helpers for this reason and removed them here for consistency.

Quoted 1 line

This deserves an explanation in the commit message.

I can also add a separate commit for that.

I think your change would be more obviously correct if you keep them.

As in generally, or just for this patch?

Quoted 4 lines

Generally, I don't think we should indicate compiler checks that actually are never done.

For this patch, yes, it's probably better to separate it.

In general. If you keep it, then I don't have to think about whether it affects bindgen's output. That's the main reason.

Alice

If we keep them, we'd consequently also need to add them for vrealloc() and kvrealloc(). But again, they don't do anything for us, and hence are more misleading than helpful IMO.

Quoted 4 lines · Danilo Krummrich

If we keep them, we'd consequently also need to add them for vrealloc() and kvrealloc(). But again, they don't do anything for us, and hence are more misleading than helpful IMO.

In general, they could do something (e.g. noreturn), perhaps in the future.

Apart from being potentially misleading, do we gain something by removing them?

I guess simplicity in the file, but it is also simpler
to keep them aligned to the C side (which I guess is Alice's point), and avoids having to keep track of what could have a present or future impact in bindgen.

Cheers,
Miguel

Quoted 7 lines · Miguel Ojeda

Quoted 4 lines · Danilo Krummrich

If we keep them, we'd consequently also need to add them for vrealloc() and kvrealloc(). But again, they don't do anything for us, and hence are more misleading than helpful IMO.

In general, they could do something (e.g. noreturn), perhaps in the future.

Indeed, and I think once they're honored we should add them again.

It's just that I think as long as compiler attributes aren't honored, we should not have them in the first place to avoid confusion about whether they do or do not have any effect.

Quoted 7 lines · Danilo Krummrich

Quoted 6 lines · Miguel Ojeda

Quoted 4 lines · Danilo Krummrich

If we keep them, we'd consequently also need to add them for vrealloc() and kvrealloc(). But again, they don't do anything for us, and hence are more misleading than helpful IMO.

Quoted 3 lines

Quoted 1 line

In general, they could do something (e.g. noreturn), perhaps in the future.

Indeed, and I think once they're honored we should add them again.

That sounds like it will be a lot of work, going through every function and checking if it has the given attribute. Especially when the attributes are enabled one by one. I think we should keep them (and of course introduce them on new functions).

---
Cheers,
Benno

Quoted 3 lines

It's just that I think as long as compiler attributes aren't honored, we should not have them in the first place to avoid confusion about whether they do or do not have any effect.

Indeed, and I think once they're honored we should add them again.

It's just that I think as long as compiler attributes aren't honored, we should not have them in the first place to avoid confusion about whether they do or do not have any effect.

It's not so much that I want to remove them for krealloc(), it's that I don't want to intentionally add them for the vrealloc() and kvrealloc() helpers, knowing that they don't do anything (yet).

I checked about __must_check, because it would be nice if it is used by bindgen, and it turns out it already does, but behind --enable-function-attribute-detection (apparently for performance reasons):

Quoted 5 lines · Miguel Ojeda

I checked about __must_check, because it would be nice if it is used by bindgen, and it turns out it already does, but behind --enable-function-attribute-detection (apparently for performance reasons):

I just tried, and the flag seems to work, gives us a bunch of #[must_use]s which are nice, and apparently no other change (in usual x86_64 config at least).

I don't notice any significant performance difference in our case, so I will send a quick patch to see if others find an issue with it.

Cheers,
Miguel

rust/kernel/alloc/allocator.rs +9 -0

+    unsafe fn call(
+        &self,
+        ptr: Option<NonNull<u8>>,
+        layout: Layout,
+        flags: Flags,
+    ) -> Result<NonNull<[u8]>, AllocError> {
+        let size = aligned_size(layout);
+        let ptr = match ptr {
+            Some(ptr) => ptr.as_ptr(),

rust/kernel/alloc/allocator.rs +8 -0

+            NonNull::dangling()
+        } else {
+            NonNull::new(raw_ptr).ok_or(AllocError)?
+        };
+
+        Ok(NonNull::slice_from_raw_parts(ptr, size))
+    }
+}

rust/kernel/alloc/allocator.rs pub(crate) unsafe fn krealloc_aligned(ptr: *mut u8, new_layout: Layout, flags: F · line 60

Quoted 3 lines · Danilo Krummrich

rust/kernel/alloc/allocator.rs +3 -0

+    /// # Safety
+    ///
+    /// This method has the same safety requirements as `Allocator::realloc`.

Please make this a link.

Quoted 14 lines

rust/kernel/alloc/allocator.rs +13 -0

+    unsafe fn call(
+        &self,
+        ptr: Option<NonNull<u8>>,
+        layout: Layout,
+        flags: Flags,
+    ) -> Result<NonNull<[u8]>, AllocError> {
+        let size = aligned_size(layout);
+        let ptr = match ptr {
+            Some(ptr) => ptr.as_ptr(),
+            None => ptr::null(),
+        };
+
+        // SAFETY: `ptr` is either NULL or valid by the safety requirements of this function.

rust/kernel/alloc/allocator.rs pub(crate) unsafe fn krealloc_aligned(ptr: *mut u8, new_layout: Layout, flags: F · line 79

Quoted 6 lines

rust/kernel/alloc/allocator.rs +6 -0

+        let raw_ptr = unsafe {
+            // If `size == 0` and `ptr != NULL` the memory behind the pointer is freed.
+            self.0(ptr.cast(), size, flags.0).cast()
+        };
+
+        let ptr = if size == 0 {

Why do you do this check *after* calling self.0?

---
Cheers,
Benno

Quoted 8 lines

rust/kernel/alloc/allocator.rs +8 -0

+            NonNull::dangling()
+        } else {
+            NonNull::new(raw_ptr).ok_or(AllocError)?
+        };
+
+        Ok(NonNull::slice_from_raw_parts(ptr, size))
+    }
+}

Quoted 14 lines · Benno Lossin

Quoted 13 lines · Danilo Krummrich

rust/kernel/alloc/allocator.rs +13 -0

+    unsafe fn call(
+        &self,
+        ptr: Option<NonNull<u8>>,
+        layout: Layout,
+        flags: Flags,
+    ) -> Result<NonNull<[u8]>, AllocError> {
+        let size = aligned_size(layout);
+        let ptr = match ptr {
+            Some(ptr) => ptr.as_ptr(),
+            None => ptr::null(),
+        };
+
+        // SAFETY: `ptr` is either NULL or valid by the safety requirements of this function.

Quoted 8 lines

rust/kernel/alloc/allocator.rs pub(crate) unsafe fn krealloc_aligned(ptr: *mut u8, new_layout: Layout, flags: F · line 79

Quoted 6 lines

rust/kernel/alloc/allocator.rs +6 -0

+        let raw_ptr = unsafe {
+            // If `size == 0` and `ptr != NULL` the memory behind the pointer is freed.
+            self.0(ptr.cast(), size, flags.0).cast()
+        };
+
+        let ptr = if size == 0 {

Why do you do this check *after* calling self.0?

Because I need raw_ptr in the else case below.

Quoted 24 lines · Danilo Krummrich

Quoted 22 lines · Benno Lossin

Quoted 9 lines · Danilo Krummrich

rust/kernel/alloc/allocator.rs +9 -0

+    unsafe fn call(
+        &self,
+        ptr: Option<NonNull<u8>>,
+        layout: Layout,
+        flags: Flags,
+    ) -> Result<NonNull<[u8]>, AllocError> {
+        let size = aligned_size(layout);
+        let ptr = match ptr {
+            Some(ptr) => ptr.as_ptr(),

rust/kernel/alloc/allocator.rs pub(crate) unsafe fn krealloc_aligned(ptr: *mut u8, new_layout: Layout, flags: F · line 79

Quoted 10 lines

rust/kernel/alloc/allocator.rs +10 -0

+            None => ptr::null(),
+        };
+
+        // SAFETY: `ptr` is either NULL or valid by the safety requirements of this function.
+        let raw_ptr = unsafe {
+            // If `size == 0` and `ptr != NULL` the memory behind the pointer is freed.
+            self.0(ptr.cast(), size, flags.0).cast()
+        };
+
+        let ptr = if size == 0 {

Why do you do this check *after* calling self.0?

Quoted 1 line

Because I need raw_ptr in the else case below.

But you can just return early above? I would prefer the check be done before self.0 is called.

---
Cheers,
Benno

Quoted 25 lines · Benno Lossin

Quoted 24 lines · Danilo Krummrich

Quoted 22 lines · Benno Lossin

Quoted 9 lines · Danilo Krummrich

rust/kernel/alloc/allocator.rs +9 -0

+    unsafe fn call(
+        &self,
+        ptr: Option<NonNull<u8>>,
+        layout: Layout,
+        flags: Flags,
+    ) -> Result<NonNull<[u8]>, AllocError> {
+        let size = aligned_size(layout);
+        let ptr = match ptr {
+            Some(ptr) => ptr.as_ptr(),

rust/kernel/alloc/allocator.rs pub(crate) unsafe fn krealloc_aligned(ptr: *mut u8, new_layout: Layout, flags: F · line 79

Quoted 10 lines

rust/kernel/alloc/allocator.rs +10 -0

+            None => ptr::null(),
+        };
+
+        // SAFETY: `ptr` is either NULL or valid by the safety requirements of this function.
+        let raw_ptr = unsafe {
+            // If `size == 0` and `ptr != NULL` the memory behind the pointer is freed.
+            self.0(ptr.cast(), size, flags.0).cast()
+        };
+
+        let ptr = if size == 0 {

Why do you do this check *after* calling self.0?

Quoted 4 lines

Quoted 1 line

Because I need raw_ptr in the else case below.

But you can just return early above? I would prefer the check be done before self.0 is called.

No, we can't return early, we need self.0 to be called, because if size == 0 we free the given allocation, if any.

+unsafe impl Allocator for Kmalloc {

rust/kernel/alloc/allocator.rs +10 -1

+    unsafe fn realloc(
+        ptr: Option<NonNull<u8>>,
+        layout: Layout,
+        flags: Flags,
+    ) -> Result<NonNull<[u8]>, AllocError> {
+        // SAFETY: `ReallocFunc::call` has the same safety requirements as `Allocator::realloc`.
+        unsafe { ReallocFunc::KREALLOC.call(ptr, layout, flags) }
+    }
+}
+
 unsafe impl GlobalAlloc for Kmalloc {
     unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
         // SAFETY: `ptr::null_mut()` is null and `layout` has a non-zero size by the function safety
--

2.45.2

Quoted 1 line · Danilo Krummrich

+unsafe impl Allocator for Kmalloc {

There is a missing SAFETY comment here (and also for Vmalloc, probably also for VKmalloc then).

---
Cheers,
Benno

Quoted 16 lines

rust/kernel/alloc/allocator.rs +10 -1

+    unsafe fn realloc(
+        ptr: Option<NonNull<u8>>,
+        layout: Layout,
+        flags: Flags,
+    ) -> Result<NonNull<[u8]>, AllocError> {
+        // SAFETY: `ReallocFunc::call` has the same safety requirements as `Allocator::realloc`.
+        unsafe { ReallocFunc::KREALLOC.call(ptr, layout, flags) }
+    }
+}
+
 unsafe impl GlobalAlloc for Kmalloc {
     unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
         // SAFETY: `ptr::null_mut()` is null and `layout` has a non-zero size by the function safety
--

2.45.2

Quoted 5 lines · Benno Lossin

Quoted 1 line · Danilo Krummrich

+unsafe impl Allocator for Kmalloc {

There is a missing SAFETY comment here (and also for Vmalloc, probably also for VKmalloc then).

Any suggestion on what to write here?

I'd probably come up with something like:

--
Memory returned from Kmalloc remains valid until explicitly freed.

It is valid to pass any pointer to an allocated memory buffer obtained with any function of Kmalloc to any other function of Kmalloc.

If Kmalloc::realloc is called with a size of zero, the given memory allocation, if any, is freed.

If Kmalloc::realloc is called with None it behaves like Kmalloc::alloc, i.e. a new memory allocation is created.
--

and repeat that for Vmalloc and KVmalloc.

I'm not sure how useful that is though.

Allocators, such as Kmalloc, will be used by e.g. Box and Vec in subsequent patches, and hence this dependency propagates throughout the whole kernel.

Add the allocator_test module that provides an empty implementation for all Allocators in the kernel, such that we don't break the rusttest make target in subsequent patches.

Reviewed-by Alice Ryhl <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

---
 rust/kernel/alloc.rs                |  9 +++++++--
 rust/kernel/alloc/allocator_test.rs | 19 +++++++++++++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)
 create mode 100644 rust/kernel/alloc/allocator_test.rs

Implement Allocator for Vmalloc, the kernel's virtually contiguous allocator, typically used for larger objects, (much) larger than page size.

All memory allocations made with Vmalloc end up in vrealloc().

Reviewed-by Alice Ryhl <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

---
 rust/helpers.c                      |  7 +++++++
 rust/kernel/alloc/allocator.rs      | 28 ++++++++++++++++++++++++++++
 rust/kernel/alloc/allocator_test.rs |  1 +
 3 files changed, 36 insertions(+)

rust/helpers.c +7 -0

index 9f7275493365..7406943f887d 100644
@@ -33,6 +33,7 @@
 #include <linux/sched/signal.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
+#include <linux/vmalloc.h>
 #include <linux/wait.h>
 #include <linux/workqueue.h>
 

@@ -199,6 +200,12 @@ void *rust_helper_krealloc(const void *objp, size_t new_size, gfp_t flags)
 }
 EXPORT_SYMBOL_GPL(rust_helper_krealloc);
 
+void *rust_helper_vrealloc(const void *p, size_t size, gfp_t flags)
+{
+	return vrealloc(p, size, flags);
+}
+EXPORT_SYMBOL_GPL(rust_helper_vrealloc);
+
 /*
  * `bindgen` binds the C `size_t` type as the Rust `usize` type, so we can
  * use it in contexts where Rust expects a `usize` like slice (array) indices.

rust/kernel/alloc/allocator.rs +10 -0

index b46883d87715..fdda22c6983f 100644
@@ -9,6 +9,7 @@
 
 use crate::alloc::{AllocError, Allocator};
 use crate::bindings;
+use crate::pr_warn;
 
 /// The contiguous kernel allocator.
 ///

@@ -16,6 +17,12 @@
 /// `bindings::krealloc`.
 pub struct Kmalloc;
 
+/// The virtually contiguous kernel allocator.
+///
+/// The vmalloc allocator allocates pages from the page level allocator and maps them into the
+/// contiguous kernel virtual space.
+pub struct Vmalloc;
+
 /// Returns a proper size to alloc a new object aligned to `new_layout`'s alignment.
 fn aligned_size(new_layout: Layout) -> usize {
     // Customized layouts from `Layout::from_size_align()` can have size < align, so pad first.

@@ -55,6 +62,9 @@ impl ReallocFunc {
     // INVARIANT: `krealloc` satisfies the type invariants.
     const KREALLOC: Self = Self(bindings::krealloc);
 
+    // INVARIANT: `vrealloc` satisfies the type invariants.
+    const VREALLOC: Self = Self(bindings::vrealloc);
+
     /// # Safety
     ///
     /// This method has the same safety requirements as `Allocator::realloc`.

rust/kernel/alloc/allocator.rs +8 -0

+        }
+
+        // SAFETY: If not `None`, `ptr` is guaranteed to point to valid memory, which was previously
+        // allocated with this `Allocator`.
+        unsafe { ReallocFunc::VREALLOC.call(ptr, layout, flags) }
+    }
+}
+
 #[global_allocator]
 static ALLOCATOR: Kmalloc = Kmalloc;
 

rust/kernel/alloc/allocator_test.rs +1 -1

index 4785efc474a7..e7bf2982f68f 100644
@@ -7,6 +7,7 @@
 use core::ptr::NonNull;
 
 pub struct Kmalloc;
+pub type Vmalloc = Kmalloc;
 
 unsafe impl Allocator for Kmalloc {
     unsafe fn realloc(
--

2.45.2

Quoted 78 lines · Danilo Krummrich

Implement Allocator for Vmalloc, the kernel's virtually contiguous allocator, typically used for larger objects, (much) larger than page size.

All memory allocations made with Vmalloc end up in vrealloc().

Reviewed-by Alice Ryhl <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

---
 rust/helpers.c                      |  7 +++++++
 rust/kernel/alloc/allocator.rs      | 28 ++++++++++++++++++++++++++++
 rust/kernel/alloc/allocator_test.rs |  1 +
 3 files changed, 36 insertions(+)

rust/helpers.c +7 -0

index 9f7275493365..7406943f887d 100644
@@ -33,6 +33,7 @@
 #include <linux/sched/signal.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
+#include <linux/vmalloc.h>
 #include <linux/wait.h>
 #include <linux/workqueue.h>
 

@@ -199,6 +200,12 @@ void *rust_helper_krealloc(const void *objp, size_t new_size, gfp_t flags)
 }
 EXPORT_SYMBOL_GPL(rust_helper_krealloc);
 
+void *rust_helper_vrealloc(const void *p, size_t size, gfp_t flags)
+{
+	return vrealloc(p, size, flags);
+}
+EXPORT_SYMBOL_GPL(rust_helper_vrealloc);
+
 /*
  * `bindgen` binds the C `size_t` type as the Rust `usize` type, so we can
  * use it in contexts where Rust expects a `usize` like slice (array) indices.

rust/kernel/alloc/allocator.rs +10 -0

index b46883d87715..fdda22c6983f 100644
@@ -9,6 +9,7 @@
 
 use crate::alloc::{AllocError, Allocator};
 use crate::bindings;
+use crate::pr_warn;
 
 /// The contiguous kernel allocator.
 ///

@@ -16,6 +17,12 @@
 /// `bindings::krealloc`.
 pub struct Kmalloc;
 
+/// The virtually contiguous kernel allocator.
+///
+/// The vmalloc allocator allocates pages from the page level allocator and maps them into the
+/// contiguous kernel virtual space.
+pub struct Vmalloc;
+
 /// Returns a proper size to alloc a new object aligned to `new_layout`'s alignment.
 fn aligned_size(new_layout: Layout) -> usize {
     // Customized layouts from `Layout::from_size_align()` can have size < align, so pad first.

@@ -55,6 +62,9 @@ impl ReallocFunc {
     // INVARIANT: `krealloc` satisfies the type invariants.
     const KREALLOC: Self = Self(bindings::krealloc);
 
+    // INVARIANT: `vrealloc` satisfies the type invariants.
+    const VREALLOC: Self = Self(bindings::vrealloc);
+
     /// # Safety
     ///
     /// This method has the same safety requirements as `Allocator::realloc`.

@@ -132,6 +142,24 @@ unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 {
     }
 }
 

rust/kernel/alloc/allocator.rs unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 { · line 145

Quoted 1 line

rust/kernel/alloc/allocator.rs +1 -0

+unsafe impl Allocator for Vmalloc {

Missing SAFETY comment.

rust/kernel/alloc/allocator.rs unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 { · line 146

Quoted 1 line

rust/kernel/alloc/allocator.rs +1 -0

+    unsafe fn realloc(

Does this need #[inline]?

rust/kernel/alloc/allocator.rs unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 { · line 154

Quoted 9 lines

rust/kernel/alloc/allocator.rs +8 -0

+        ptr: Option<NonNull<u8>>,
+        layout: Layout,
+        flags: Flags,
+    ) -> Result<NonNull<[u8]>, AllocError> {
+        // TODO: Support alignments larger than PAGE_SIZE.
+        if layout.align() > bindings::PAGE_SIZE {
+            pr_warn!("Vmalloc does not support alignments larger than PAGE_SIZE yet.\n");
+            return Err(AllocError);

I think here we should first try to use build_error!, most often the alignment will be specified statically, so it should get optimized away.

How difficult will it be to support this? (it is a weird requirement, but I dislike just returning an error...)

---
Cheers,
Benno

Quoted 26 lines

rust/kernel/alloc/allocator.rs +8 -0

+        }
+
+        // SAFETY: If not `None`, `ptr` is guaranteed to point to valid memory, which was previously
+        // allocated with this `Allocator`.
+        unsafe { ReallocFunc::VREALLOC.call(ptr, layout, flags) }
+    }
+}
+
 #[global_allocator]
 static ALLOCATOR: Kmalloc = Kmalloc;
 

rust/kernel/alloc/allocator_test.rs +1 -1

index 4785efc474a7..e7bf2982f68f 100644
@@ -7,6 +7,7 @@
 use core::ptr::NonNull;
 
 pub struct Kmalloc;
+pub type Vmalloc = Kmalloc;
 
 unsafe impl Allocator for Kmalloc {
     unsafe fn realloc(
--

2.45.2

Quoted 11 lines

Quoted 10 lines

rust/kernel/alloc/allocator.rs +8 -0

+        ptr: Option<NonNull<u8>>,
+        layout: Layout,
+        flags: Flags,
+    ) -> Result<NonNull<[u8]>, AllocError> {
+        // TODO: Support alignments larger than PAGE_SIZE.
+        if layout.align() > bindings::PAGE_SIZE {
+            pr_warn!("Vmalloc does not support alignments larger than PAGE_SIZE yet.\n");
+            return Err(AllocError);

Quoted 2 lines

I think here we should first try to use build_error!, most often the alignment will be specified statically, so it should get optimized away.

Sure, we can try that first.

Quoted 4 lines

How difficult will it be to support this? (it is a weird requirement, but I dislike just returning an error...)

It's not difficult to support at all. But it requires a C API taking an alignment argument (same for KVmalloc).

Coming up with a vrealloc_aligned() is rather trivial. kvrealloc_aligned() would be a bit weird though, because the alignment argument could only be really honored if we run into the vrealloc() case. For the krealloc() case it'd still depend on the bucket size that is selected for the requested size.

Adding the C API, I'm also pretty sure someone's gonna ask what we need an alignment larger than PAGE_SIZE for and if we have a real use case for that. I'm not entirely sure we have a reasonable answer for that.

I got some hacked up patches for that, but I'd rather polish and send them once we actually need it.

Quoted 14 lines · Danilo Krummrich

Quoted 13 lines

rust/kernel/alloc/allocator.rs unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 { · line 154

Quoted 10 lines

rust/kernel/alloc/allocator.rs +8 -0

+        ptr: Option<NonNull<u8>>,
+        layout: Layout,
+        flags: Flags,
+    ) -> Result<NonNull<[u8]>, AllocError> {
+        // TODO: Support alignments larger than PAGE_SIZE.
+        if layout.align() > bindings::PAGE_SIZE {
+            pr_warn!("Vmalloc does not support alignments larger than PAGE_SIZE yet.\n");
+            return Err(AllocError);

I think here we should first try to use build_error!, most often the alignment will be specified statically, so it should get optimized away.

Quoted 1 line

Sure, we can try that first.

I think I spoke too soon here. I don't think build_error! or build_assert! can work here, it would also fail the build when the compiler doesn't know the value of the alignment, wouldn't it? I remember that I wasn't overly happy about failing this on runtime either when I first thought about this case, but I also couldn't think of something better.

In the end it's rather unlikely to ever hit this case, and probably even more unlikely to hit it for a sane reason.

Quoted 5 lines

Quoted 2 lines

How difficult will it be to support this? (it is a weird requirement, but I dislike just returning an error...)

It's not difficult to support at all. But it requires a C API taking an alignment argument (same for KVmalloc).

Quoted 4 lines

Coming up with a vrealloc_aligned() is rather trivial. kvrealloc_aligned() would be a bit weird though, because the alignment argument could only be really honored if we run into the vrealloc() case. For the krealloc() case it'd still depend on the bucket size that is selected for the requested size.

Quoted 3 lines

Adding the C API, I'm also pretty sure someone's gonna ask what we need an alignment larger than PAGE_SIZE for and if we have a real use case for that. I'm not entirely sure we have a reasonable answer for that.

Quoted 2 lines

I got some hacked up patches for that, but I'd rather polish and send them once we actually need it.

Quoted 15 lines · Danilo Krummrich

Quoted 14 lines · Danilo Krummrich

Quoted 13 lines

rust/kernel/alloc/allocator.rs unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 { · line 154

Quoted 10 lines

rust/kernel/alloc/allocator.rs +8 -0

+        ptr: Option<NonNull<u8>>,
+        layout: Layout,
+        flags: Flags,
+    ) -> Result<NonNull<[u8]>, AllocError> {
+        // TODO: Support alignments larger than PAGE_SIZE.
+        if layout.align() > bindings::PAGE_SIZE {
+            pr_warn!("Vmalloc does not support alignments larger than PAGE_SIZE yet.\n");
+            return Err(AllocError);

I think here we should first try to use build_error!, most often the alignment will be specified statically, so it should get optimized away.

Quoted 7 lines

Quoted 1 line

Sure, we can try that first.

I think I spoke too soon here. I don't think build_error! or build_assert! can work here, it would also fail the build when the compiler doesn't know the value of the alignment, wouldn't it? I remember that I wasn't overly happy about failing this on runtime either when I first thought about this case, but I also couldn't think of something better.

Yes, it might fail even though the alignment at runtime will be fine. But that's why I suggested trying build_error!(or build_assert!) first, if nobody hits the case where the compiler cannot figure it out, then we can keep it. If there are instances, where it fails, but the alignment would be fine at runtime, then we can change it to the above. (I would add such a comment above the assert).

Quoted 3 lines

In the end it's rather unlikely to ever hit this case, and probably even more unlikely to hit it for a sane reason.

Yeah, but I still prefer the build to fail, rather than emitting a warn message that can be overlooked at runtime.

Quoted 5 lines

Quoted 5 lines

Quoted 2 lines

How difficult will it be to support this? (it is a weird requirement, but I dislike just returning an error...)

It's not difficult to support at all. But it requires a C API taking an alignment argument (same for KVmalloc).

I see, that's good to know.

Quoted 4 lines

Quoted 4 lines

Coming up with a vrealloc_aligned() is rather trivial. kvrealloc_aligned() would be a bit weird though, because the alignment argument could only be really honored if we run into the vrealloc() case. For the krealloc() case it'd still depend on the bucket size that is selected for the requested size.

Yeah... Maybe some more logic on the Rust side can help with that.

Quoted 4 lines

Quoted 4 lines

Adding the C API, I'm also pretty sure someone's gonna ask what we need an alignment larger than PAGE_SIZE for and if we have a real use case for that. I'm not entirely sure we have a reasonable answer for that.

We could argue that we can remove an "ugly hack" (when we don't have the build assert, if we do have that, I don't mind not supporting it), but I agree that finding a user will be difficult.

Quoted 3 lines

Quoted 3 lines

I got some hacked up patches for that, but I'd rather polish and send them once we actually need it.

Sure, just wanted to check why you don't want to do it this series.

---
Cheers,
Benno

Quoted 24 lines · Benno Lossin

Quoted 22 lines · Danilo Krummrich

Quoted 11 lines · Danilo Krummrich

Quoted 11 lines

Quoted 10 lines

rust/kernel/alloc/allocator.rs +8 -0

+        ptr: Option<NonNull<u8>>,
+        layout: Layout,
+        flags: Flags,
+    ) -> Result<NonNull<[u8]>, AllocError> {
+        // TODO: Support alignments larger than PAGE_SIZE.
+        if layout.align() > bindings::PAGE_SIZE {
+            pr_warn!("Vmalloc does not support alignments larger than PAGE_SIZE yet.\n");
+            return Err(AllocError);

Quoted 4 lines

Quoted 2 lines

I think here we should first try to use build_error!, most often the alignment will be specified statically, so it should get optimized away.

Sure, we can try that first.

I think I spoke too soon here. I don't think build_error! or build_assert! can work here, it would also fail the build when the compiler doesn't know the value of the alignment, wouldn't it? I remember that I wasn't overly happy about failing this on runtime either when I first thought about this case, but I also couldn't think of something better.

Quoted 6 lines

Yes, it might fail even though the alignment at runtime will be fine. But that's why I suggested trying build_error!(or build_assert!) first, if nobody hits the case where the compiler cannot figure it out, then we can keep it. If there are instances, where it fails, but the alignment would be fine at runtime, then we can change it to the above. (I would add such a comment above the assert).

Unfortunately, it already does fail with just the test cases.

Anyway, even if it would have been fine, I don't think it would have been nice for a future user to run into a build error even though the alignment is perfectlly within bounds.

Quoted 19 lines

Quoted 2 lines

In the end it's rather unlikely to ever hit this case, and probably even more unlikely to hit it for a sane reason.

Yeah, but I still prefer the build to fail, rather than emitting a warn message that can be overlooked at runtime.

Quoted 5 lines

Quoted 5 lines

Quoted 2 lines

How difficult will it be to support this? (it is a weird requirement, but I dislike just returning an error...)

It's not difficult to support at all. But it requires a C API taking an alignment argument (same for KVmalloc).

I see, that's good to know.

Quoted 4 lines

Quoted 4 lines

Coming up with a vrealloc_aligned() is rather trivial. kvrealloc_aligned() would be a bit weird though, because the alignment argument could only be really honored if we run into the vrealloc() case. For the krealloc() case it'd still depend on the bucket size that is selected for the requested size.

Quoted 1 line

Yeah... Maybe some more logic on the Rust side can help with that.

Only if we reimplement KVmalloc in Rust, However, there are quite some special cases in __kvmalloc_node_noprof(), i.e. fixup page flags, sanity check the size on kmalloc failure, fail on certain page flags, etc.

I don't really want to duplicate this code, unless we absolutely have to.

Quoted 7 lines

Quoted 3 lines

Quoted 3 lines

Adding the C API, I'm also pretty sure someone's gonna ask what we need an alignment larger than PAGE_SIZE for and if we have a real use case for that. I'm not entirely sure we have a reasonable answer for that.

We could argue that we can remove an "ugly hack" (when we don't have the build assert, if we do have that, I don't mind not supporting it), but I agree that finding a user will be difficult.

I'd argue it's not really a hack to fail on something that's not supported (yet). Allocations can (almost) always fail, this is just another case.

Quoted 25 lines · Danilo Krummrich

Quoted 24 lines · Benno Lossin

Quoted 22 lines · Danilo Krummrich

Quoted 11 lines · Danilo Krummrich

Quoted 11 lines

Quoted 10 lines

(quoted diff) +8 -0

+        ptr: Option<NonNull<u8>>,
+        layout: Layout,
+        flags: Flags,
+    ) -> Result<NonNull<[u8]>, AllocError> {
+        // TODO: Support alignments larger than PAGE_SIZE.
+        if layout.align() > bindings::PAGE_SIZE {
+            pr_warn!("Vmalloc does not support alignments larger than PAGE_SIZE yet.\n");
+            return Err(AllocError);

Quoted 4 lines

Quoted 2 lines

I think here we should first try to use build_error!, most often the alignment will be specified statically, so it should get optimized away.

Sure, we can try that first.

I think I spoke too soon here. I don't think build_error! or build_assert! can work here, it would also fail the build when the compiler doesn't know the value of the alignment, wouldn't it? I remember that I wasn't overly happy about failing this on runtime either when I first thought about this case, but I also couldn't think of something better.

Quoted 8 lines

Quoted 6 lines

Yes, it might fail even though the alignment at runtime will be fine. But that's why I suggested trying build_error!(or build_assert!) first, if nobody hits the case where the compiler cannot figure it out, then we can keep it. If there are instances, where it fails, but the alignment would be fine at runtime, then we can change it to the above. (I would add such a comment above the assert).

Unfortunately, it already does fail with just the test cases.

Aw that's sad.

Quoted 3 lines

Anyway, even if it would have been fine, I don't think it would have been nice for a future user to run into a build error even though the alignment is perfectlly within bounds.

I think it would have been better compared to failing with a warning at runtime.

Quoted 22 lines

Quoted 21 lines

Quoted 3 lines

In the end it's rather unlikely to ever hit this case, and probably even more unlikely to hit it for a sane reason.

Yeah, but I still prefer the build to fail, rather than emitting a warn message that can be overlooked at runtime.

Quoted 5 lines

Quoted 5 lines

Quoted 2 lines

How difficult will it be to support this? (it is a weird requirement, but I dislike just returning an error...)

It's not difficult to support at all. But it requires a C API taking an alignment argument (same for KVmalloc).

I see, that's good to know.

Quoted 4 lines

Quoted 4 lines

Coming up with a vrealloc_aligned() is rather trivial. kvrealloc_aligned() would be a bit weird though, because the alignment argument could only be really honored if we run into the vrealloc() case. For the krealloc() case it'd still depend on the bucket size that is selected for the requested size.

Yeah... Maybe some more logic on the Rust side can help with that.

Quoted 5 lines

Only if we reimplement KVmalloc in Rust, However, there are quite some special cases in __kvmalloc_node_noprof(), i.e. fixup page flags, sanity check the size on kmalloc failure, fail on certain page flags, etc.

I don't really want to duplicate this code, unless we absolutely have to.

I am under the (probably wrong) impression that kvmalloc has some size check and selects vmalloc or kmalloc depending on that.

I think that we
could check the size and if it is going to allocate via kmalloc, then we adjust the size for alignment as usual

and if it is going to select
vmalloc, then we can just pass the alignment (if the vmalloc alignment patch is done first).

Quoted 11 lines

Quoted 8 lines

Quoted 4 lines

Quoted 4 lines

Adding the C API, I'm also pretty sure someone's gonna ask what we need an alignment larger than PAGE_SIZE for and if we have a real use case for that. I'm not entirely sure we have a reasonable answer for that.

We could argue that we can remove an "ugly hack" (when we don't have the build assert, if we do have that, I don't mind not supporting it), but I agree that finding a user will be difficult.

I'd argue it's not really a hack to fail on something that's not supported (yet). Allocations can (almost) always fail, this is just another case.

I guess since this is a deterministic failure, it's better than other failures. But I would still say this is hacky.

---
Cheers,
Benno

Quoted 19 lines · Benno Lossin

Quoted 18 lines · Danilo Krummrich

Quoted 16 lines · Benno Lossin

Quoted 6 lines · Danilo Krummrich

Quoted 5 lines · Danilo Krummrich

Quoted 2 lines

How difficult will it be to support this? (it is a weird requirement, but I dislike just returning an error...)

It's not difficult to support at all. But it requires a C API taking an alignment argument (same for KVmalloc).

I see, that's good to know.

Quoted 4 lines

Quoted 4 lines

Coming up with a vrealloc_aligned() is rather trivial. kvrealloc_aligned() would be a bit weird though, because the alignment argument could only be really honored if we run into the vrealloc() case. For the krealloc() case it'd still depend on the bucket size that is selected for the requested size.

Yeah... Maybe some more logic on the Rust side can help with that.

Quoted 8 lines

Quoted 5 lines

Only if we reimplement KVmalloc in Rust, However, there are quite some special cases in __kvmalloc_node_noprof(), i.e. fixup page flags, sanity check the size on kmalloc failure, fail on certain page flags, etc.

I don't really want to duplicate this code, unless we absolutely have to.

I am under the (probably wrong) impression that kvmalloc has some size check and selects vmalloc or kmalloc depending on that.

Basically, yes. But as mentioned above, there are quite some corner cases [1].

Quoted 3 lines

I think that we
could check the size and if it is going to allocate via kmalloc, then we adjust the size for alignment as usual

We don't need this adjustment any longer, see commit ad59baa31695 ("slab, rust:
extend kmalloc() alignment guarantees to remove Rust padding").

Quoted 3 lines

and if it is going to select
vmalloc, then we can just pass the alignment (if the vmalloc alignment patch is done first).

Yeah, but as mentioned, I'd prefer to do this in C, such that we don't need to open code everything the C code already does.

[1] https://elixir.bootlin.com/linux/v6.11-rc3/source/mm/util.c#L628

Some test cases in subsequent patches provoke allocation failures. Add __GFP_NOWARN to enable test cases to silence unpleasant warnings.

Signed-off-by Danilo Krummrich <[email protected]>

 rust/bindings/bindings_helper.h | 1 +
 rust/kernel/alloc.rs            | 5 +++++
 2 files changed, 6 insertions(+)

rust/bindings/bindings_helper.h +1 -0

index b940a5777330..7f781256fda9 100644
@@ -31,4 +31,5 @@ const gfp_t RUST_CONST_HELPER_GFP_KERNEL_ACCOUNT = GFP_KERNEL_ACCOUNT;
 const gfp_t RUST_CONST_HELPER_GFP_NOWAIT = GFP_NOWAIT;
 const gfp_t RUST_CONST_HELPER___GFP_ZERO = __GFP_ZERO;
 const gfp_t RUST_CONST_HELPER___GFP_HIGHMEM = ___GFP_HIGHMEM;
+const gfp_t RUST_CONST_HELPER___GFP_NOWARN = ___GFP_NOWARN;
 const blk_features_t RUST_CONST_HELPER_BLK_FEAT_ROTATIONAL = BLK_FEAT_ROTATIONAL;

rust/kernel/alloc.rs +5 -1

index f0c80ab78602..295107777a12 100644
@@ -91,6 +91,11 @@ pub mod flags {
     /// use any filesystem callback.  It is very likely to fail to allocate memory, even for very
     /// small allocations.
     pub const GFP_NOWAIT: Flags = Flags(bindings::GFP_NOWAIT);
+
+    /// Suppresses allocation failure reports.
+    ///
+    /// This is normally or'd with other flags.
+    pub const __GFP_NOWARN: Flags = Flags(bindings::__GFP_NOWARN);
 }
 
 /// The kernel's [`Allocator`] trait.
--

2.45.2

Some test cases in subsequent patches provoke allocation failures. Add __GFP_NOWARN to enable test cases to silence unpleasant warnings.

Signed-off-by Danilo Krummrich <[email protected]>

---
 rust/bindings/bindings_helper.h | 1 +
 rust/kernel/alloc.rs            | 5 +++++
 2 files changed, 6 insertions(+)

Box provides the simplest way to allocate memory for a generic type with one of the kernel's allocators, e.g. Kmalloc, Vmalloc or KVmalloc.

In contrast to Rust's Box type, the kernel Box type considers the kernel's GFP flags for all appropriate functions, always reports allocation failures through Result<_, AllocError> and remains independent from unstable features.

Signed-off-by Danilo Krummrich <[email protected]>

---
 rust/kernel/alloc.rs      |   6 +
 rust/kernel/alloc/kbox.rs | 423 ++++++++++++++++++++++++++++++++++++++
 rust/kernel/prelude.rs    |   2 +-
 3 files changed, 430 insertions(+), 1 deletion(-)
 create mode 100644 rust/kernel/alloc/kbox.rs

rust/kernel/alloc.rs +6 -0

index 295107777a12..ed46b69204d0 100644
@@ -5,6 +5,7 @@
 #[cfg(not(any(test, testlib)))]
 pub mod allocator;
 pub mod box_ext;
+pub mod kbox;
 pub mod vec_ext;
 
 #[cfg(any(test, testlib))]

@@ -13,6 +14,11 @@
 #[cfg(any(test, testlib))]
 pub use self::allocator_test as allocator;
 
+pub use self::kbox::Box;
+pub use self::kbox::KBox;
+pub use self::kbox::KVBox;
+pub use self::kbox::VBox;
+
 /// Indicates an allocation error.
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
 pub struct AllocError;

rust/kernel/alloc/kbox.rs new file +27 -0

new file mode 100644
index 000000000000..67bdfc0712d2
@@ -0,0 +1,423 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Implementation of [`Box`].
+
+use super::{AllocError, Allocator, Flags};
+use core::fmt;
+use core::marker::PhantomData;
+use core::mem::ManuallyDrop;
+use core::mem::MaybeUninit;
+use core::ops::{Deref, DerefMut};
+use core::pin::Pin;
+use core::ptr::NonNull;
+use core::result::Result;
+
+use crate::init::{InPlaceInit, Init, PinInit};
+use crate::types::ForeignOwnable;
+
+/// The kernel's [`Box`] type - a heap allocation for a single value of type `T`.
+///
+/// This is the kernel's version of the Rust stdlib's `Box`. There are a couple of differences,
+/// for example no `noalias` attribute is emitted and partially moving out of a `Box` is not
+/// supported.
+///
+/// `Box` works with any of the kernel's allocators, e.g. [`super::allocator::Kmalloc`],
+/// [`super::allocator::Vmalloc`] or [`super::allocator::KVmalloc`]. There are aliases for `Box`
+/// with these allocators ([`KBox`], [`VBox`], [`KVBox`]).
+///

rust/kernel/alloc/kbox.rs new file +15 -0

+/// ```
+///
+/// ```
+/// # use kernel::bindings;
+///
+/// const SIZE: usize = bindings::KMALLOC_MAX_SIZE as usize + 1;
+/// struct Huge([u8; SIZE]);
+///
+/// assert!(KBox::<Huge>::new_uninit(GFP_KERNEL | __GFP_NOWARN).is_err());
+/// ```
+///
+/// ```
+/// # use kernel::bindings;
+///
+/// const SIZE: usize = bindings::KMALLOC_MAX_SIZE as usize + 1;

Quoted 82 lines · Danilo Krummrich

Box provides the simplest way to allocate memory for a generic type with one of the kernel's allocators, e.g. Kmalloc, Vmalloc or KVmalloc.

In contrast to Rust's Box type, the kernel Box type considers the kernel's GFP flags for all appropriate functions, always reports allocation failures through Result<_, AllocError> and remains independent from unstable features.

Signed-off-by Danilo Krummrich <[email protected]>

---
 rust/kernel/alloc.rs      |   6 +
 rust/kernel/alloc/kbox.rs | 423 ++++++++++++++++++++++++++++++++++++++
 rust/kernel/prelude.rs    |   2 +-
 3 files changed, 430 insertions(+), 1 deletion(-)
 create mode 100644 rust/kernel/alloc/kbox.rs

rust/kernel/alloc.rs +6 -0

index 295107777a12..ed46b69204d0 100644
@@ -5,6 +5,7 @@
 #[cfg(not(any(test, testlib)))]
 pub mod allocator;
 pub mod box_ext;
+pub mod kbox;
 pub mod vec_ext;
 
 #[cfg(any(test, testlib))]

@@ -13,6 +14,11 @@
 #[cfg(any(test, testlib))]
 pub use self::allocator_test as allocator;
 
+pub use self::kbox::Box;
+pub use self::kbox::KBox;
+pub use self::kbox::KVBox;
+pub use self::kbox::VBox;
+
 /// Indicates an allocation error.
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
 pub struct AllocError;

rust/kernel/alloc/kbox.rs new file +34 -0

new file mode 100644
index 000000000000..67bdfc0712d2
@@ -0,0 +1,423 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Implementation of [`Box`].
+
+use super::{AllocError, Allocator, Flags};
+use core::fmt;
+use core::marker::PhantomData;
+use core::mem::ManuallyDrop;
+use core::mem::MaybeUninit;
+use core::ops::{Deref, DerefMut};
+use core::pin::Pin;
+use core::ptr::NonNull;
+use core::result::Result;
+
+use crate::init::{InPlaceInit, Init, PinInit};
+use crate::types::ForeignOwnable;
+
+/// The kernel's [`Box`] type - a heap allocation for a single value of type `T`.
+///
+/// This is the kernel's version of the Rust stdlib's `Box`. There are a couple of differences,
+/// for example no `noalias` attribute is emitted and partially moving out of a `Box` is not
+/// supported.
+///
+/// `Box` works with any of the kernel's allocators, e.g. [`super::allocator::Kmalloc`],
+/// [`super::allocator::Vmalloc`] or [`super::allocator::KVmalloc`]. There are aliases for `Box`
+/// with these allocators ([`KBox`], [`VBox`], [`KVBox`]).
+///
+/// When dropping a [`Box`], the value is also dropped and the heap memory is automatically freed.
+///
+/// # Examples
+///
+/// ```
+/// let b = KBox::<u64>::new(24_u64, GFP_KERNEL)?;
+///

rust/kernel/alloc/kbox.rs line 37

Quoted 3 lines

rust/kernel/alloc/kbox.rs new file +3 -0

+/// assert_eq!(*b, 24_u64);
+///
+/// # Ok::<(), Error>(())

This is a minor nit, but when hiding lines in examples you should avoid having the rendered docs have empty lines at the beginning/end. There are also several examples of this below with the kernel::bindings import.

Quoted 16 lines

rust/kernel/alloc/kbox.rs new file +15 -0

+/// ```
+///
+/// ```
+/// # use kernel::bindings;
+///
+/// const SIZE: usize = bindings::KMALLOC_MAX_SIZE as usize + 1;
+/// struct Huge([u8; SIZE]);
+///
+/// assert!(KBox::<Huge>::new_uninit(GFP_KERNEL | __GFP_NOWARN).is_err());
+/// ```
+///
+/// ```
+/// # use kernel::bindings;
+///
+/// const SIZE: usize = bindings::KMALLOC_MAX_SIZE as usize + 1;

rust/kernel/alloc/kbox.rs line 62

Quoted 10 lines

rust/kernel/alloc/kbox.rs new file +10 -0

+/// struct Huge([u8; SIZE]);
+///
+/// assert!(KVBox::<Huge>::new_uninit(GFP_KERNEL).is_ok());
+/// ```
+///
+/// # Invariants
+///
+/// The [`Box`]' pointer always properly aligned and either points to memory allocated with `A` or,
+/// for zero-sized types, is a dangling pointer.
+pub struct Box<T: ?Sized, A: Allocator>(NonNull<T>, PhantomData<A>);

I was about to say this needs a PhantomData<T> too, but I guess it isn't necessary anymore.
https://doc.rust-lang.org/nomicon/phantom-data.html#generic-parameters-and-drop-checking

rust/kernel/alloc/kbox.rs line 103

Quoted 1 line

rust/kernel/alloc/kbox.rs new file +1 -0

+// SAFETY: `Box` is `Send` if `T` is `Send` because the data referenced by `self.0` is unaliased.

Instead of "unaliased" I would probably just say "because the Box owns a T".

rust/kernel/alloc/kbox.rs line 115

Quoted 7 lines

rust/kernel/alloc/kbox.rs new file +6 -0

+
+// SAFETY: `Box` is `Sync` if `T` is `Sync` because the data referenced by `self.0` is unaliased.
+unsafe impl<T, A> Sync for Box<T, A>
+where
+    T: Send + ?Sized,
+    A: Allocator,

This needs to say T: Sync instead of T: Send. That matches the std Box.

rust/kernel/alloc/kbox.rs line 129

Quoted 13 lines

rust/kernel/alloc/kbox.rs new file +12 -0

+
+impl<T, A> Box<T, A>
+where
+    T: ?Sized,
+    A: Allocator,
+{
+    /// Creates a new `Box<T, A>` from a raw pointer.
+    ///
+    /// # Safety
+    ///
+    /// `raw` must point to valid memory, previously be allocated with `A`, and provide at least
+    /// the size of type `T`. For ZSTs `raw` must be a dangling pointer.

Hmm. I don't love this wording. How about this?

For non-ZSTs, raw must point at a live allocation allocated with A that is sufficiently aligned for and holds a valid T. The caller passes ownership of the allocation to the Box.

For ZSTs, the pointer must be non-null and aligned.

rust/kernel/alloc/kbox.rs line 280

Quoted 12 lines

rust/kernel/alloc/kbox.rs new file +11 -0

+impl<T, A> From<Box<T, A>> for Pin<Box<T, A>>
+where
+    T: ?Sized,
+    A: Allocator,
+{
+    /// Converts a `Box<T, A>` into a `Pin<Box<T, A>>`. If `T` does not implement [`Unpin`], then
+    /// `*b` will be pinned in memory and can't be moved.
+    ///
+    /// See [`Box::into_pin`] for more details.
+    fn from(b: Box<T, A>) -> Self {
+        Box::into_pin(b)

I still think it makes more sense to match std and only provide From and not an into_pin, but it's not a blocker.

Alice

Quoted 15 lines · Alice Ryhl

rust/kernel/alloc/kbox.rs line 280

Quoted 11 lines · Danilo Krummrich

rust/kernel/alloc/kbox.rs new file +11 -0

+impl<T, A> From<Box<T, A>> for Pin<Box<T, A>>
+where
+    T: ?Sized,
+    A: Allocator,
+{
+    /// Converts a `Box<T, A>` into a `Pin<Box<T, A>>`. If `T` does not implement [`Unpin`], then
+    /// `*b` will be pinned in memory and can't be moved.
+    ///
+    /// See [`Box::into_pin`] for more details.
+    fn from(b: Box<T, A>) -> Self {
+        Box::into_pin(b)

I still think it makes more sense to match std and only provide From and not an into_pin, but it's not a blocker.

Yeah, I just kept it since I'm not (yet) entirely sure what to think of the From and Into stuff in some cases.

I don't really like that, depending on the context, it may hide relevant details.

In the kernel, no matter how well documented an API is, I think it's rather common to look at the code for some implementation details before using it.

Sometimes it might not be super trivial for the "occasional" reader to figure out what's the type of some variable. Calling into_pin vs. just into immediately tells the reader that things need to be pinned from there on.

However, I had no specific example in my mind and I'm also not overly concerned to remove into_pin, but I want to at least share the reason why I kept it in the first place.

rust/kernel/alloc/kbox.rs new file +28 -0

+///
+/// `Box` works with any of the kernel's allocators, e.g. [`super::allocator::Kmalloc`],
+/// [`super::allocator::Vmalloc`] or [`super::allocator::KVmalloc`]. There are aliases for `Box`
+/// with these allocators ([`KBox`], [`VBox`], [`KVBox`]).
+///
+/// When dropping a [`Box`], the value is also dropped and the heap memory is automatically freed.
+///
+/// # Examples
+///
+/// ```
+/// let b = KBox::<u64>::new(24_u64, GFP_KERNEL)?;
+///
+/// assert_eq!(*b, 24_u64);
+///
+/// # Ok::<(), Error>(())
+/// ```
+///
+/// ```
+/// # use kernel::bindings;
+///
+/// const SIZE: usize = bindings::KMALLOC_MAX_SIZE as usize + 1;
+/// struct Huge([u8; SIZE]);
+///
+/// assert!(KBox::<Huge>::new_uninit(GFP_KERNEL | __GFP_NOWARN).is_err());
+/// ```
+///
+/// ```
+/// # use kernel::bindings;

+/// for zero-sized types, is a dangling pointer.

+pub struct Box<T: ?Sized, A: Allocator>(NonNull<T>, PhantomData<A>);

rust/kernel/alloc/kbox.rs new file +67 -0

+///
+/// # Examples
+///
+/// ```
+/// let b = KVBox::new(24_u64, GFP_KERNEL)?;
+///
+/// assert_eq!(*b, 24_u64);
+///
+/// # Ok::<(), Error>(())
+/// ```
+pub type KVBox<T> = Box<T, super::allocator::KVmalloc>;
+
+// SAFETY: `Box` is `Send` if `T` is `Send` because the data referenced by `self.0` is unaliased.
+unsafe impl<T, A> Send for Box<T, A>
+where
+    T: Send + ?Sized,
+    A: Allocator,
+{
+}
+
+// SAFETY: `Box` is `Sync` if `T` is `Sync` because the data referenced by `self.0` is unaliased.
+unsafe impl<T, A> Sync for Box<T, A>
+where
+    T: Send + ?Sized,
+    A: Allocator,
+{
+}
+
+impl<T, A> Box<T, A>
+where
+    T: ?Sized,
+    A: Allocator,
+{
+    /// Creates a new `Box<T, A>` from a raw pointer.
+    ///
+    /// # Safety
+    ///
+    /// `raw` must point to valid memory, previously be allocated with `A`, and provide at least
+    /// the size of type `T`. For ZSTs `raw` must be a dangling pointer.
+    #[inline]
+    pub const unsafe fn from_raw(raw: *mut T) -> Self {
+        // INVARIANT: Validity of `raw` is guaranteed by the safety preconditions of this function.
+        // SAFETY: By the safety preconditions of this function, `raw` is not a NULL pointer.
+        Self(unsafe { NonNull::new_unchecked(raw) }, PhantomData::<A>)
+    }
+
+    /// Consumes the `Box<T, A>` and returns a raw pointer.
+    ///
+    /// This will not run the destructor of `T` and for non-ZSTs the allocation will stay alive
+    /// indefinitely. Use [`Box::from_raw`] to recover the [`Box`], drop the value and free the
+    /// allocation, if any.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// let x = KBox::new(24, GFP_KERNEL)?;
+    /// let ptr = KBox::into_raw(x);
+    /// let x = unsafe { KBox::from_raw(ptr) };
+    ///
+    /// assert_eq!(*x, 24);
+    ///
+    /// # Ok::<(), Error>(())
+    /// ```
+    #[inline]
+    pub fn into_raw(b: Self) -> *mut T {
+        let b = ManuallyDrop::new(b);
+

rust/kernel/alloc/kbox.rs new file +7 -0

+        // SAFETY: The value wrapped inside a `Pin<Box<T, A>>` cannot be moved or replaced as long
+        // as `T` does not implement `Unpin`.
+        unsafe { Pin::new_unchecked(b) }
+    }
+}
+
+impl<T, A> Box<MaybeUninit<T>, A>