On 16.08.24 02:10, Danilo Krummrich wrote:
> diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs
> new file mode 100644
> index 000000000000..93b1ab9de6e8
> --- /dev/null
> +++ b/rust/kernel/alloc/kbox.rs
> @@ -0,0 +1,480 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Implementation of [`Box`].
> +
> +#[allow(unused_imports)] // Used in doc comments.
> +use super::allocator::{KVmalloc, Kmalloc, Vmalloc};
> +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, InPlaceWrite, Init, PinInit};
> +use crate::types::ForeignOwnable;
> +
> +/// The kernel's [`Box`] type - a heap allocation for a single value of type `T`.
A single `-` doesn't really render nicely in markdown, instead use a
double or triple dash (`--` or `---`).
> +///
> +/// This is the kernel's version of the Rust stdlib's `Box`. There are several of differences,
> +/// for example no `noalias` attribute is emitted and partially moving out of a `Box` is not
> +/// supported. There are also several API differences, e.g. `Box` always requires an [`Allocator`]
> +/// implementation to be passed as generic, page [`Flags`] when allocating memory and all functions
> +/// that may allocate memory are failable.
Do you mean fallible?
> +///
> +/// `Box` works with any of the kernel's allocators, e.g. [`Kmalloc`], [`Vmalloc`] or [`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());
> +/// ```
It would be nice if you could add something like "KBox can't handle big
allocations:" above this example, so that people aren't confused why
this example expects an error.
> +///
> +/// ```
> +/// # use kernel::bindings;
> +/// const SIZE: usize = bindings::KMALLOC_MAX_SIZE as usize + 1;
> +/// struct Huge([u8; SIZE]);
> +///
> +/// assert!(KVBox::<Huge>::new_uninit(GFP_KERNEL).is_ok());
> +/// ```
Similarly, you could then say above this one "Instead use either `VBox`
or `KVBox`:"
> +///
> +/// # Invariants
> +///
> +/// The [`Box`]' pointer is always properly aligned and either points to memory allocated with `A`
Please use `self.0` instead of "[`Box`]'".
> +/// or, for zero-sized types, is a dangling pointer.
Probably "dangling, well aligned pointer.".
> +#[repr(transparent)]
> +pub struct Box<T: ?Sized, A: Allocator>(NonNull<T>, PhantomData<A>);
> +
> +/// Type alias for `Box` with a [`Kmalloc`] allocator.
Can you make these `Box` references links?
> +///
> +/// # Examples
> +///
> +/// ```
> +/// let b = KBox::new(24_u64, GFP_KERNEL)?;
> +///
> +/// assert_eq!(*b, 24_u64);
> +/// # Ok::<(), Error>(())
> +/// ```
> +pub type KBox<T> = Box<T, super::allocator::Kmalloc>;
> +
> +/// Type alias for `Box` with a [`Vmalloc`] allocator.
> +///
> +/// # Examples
> +///
> +/// ```
> +/// let b = VBox::new(24_u64, GFP_KERNEL)?;
> +///
> +/// assert_eq!(*b, 24_u64);
> +/// # Ok::<(), Error>(())
> +/// ```
> +pub type VBox<T> = Box<T, super::allocator::Vmalloc>;
> +
> +/// Type alias for `Box` with a [`KVmalloc`] allocator.
> +///
> +/// # 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 `Box` owns a `T`.
> +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 `Box` owns a `T`.
> +unsafe impl<T, A> Sync for Box<T, A>
> +where
> + T: Sync + ?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
> + ///
> + /// For non-ZSTs, `raw` must point at an allocation allocated with `A`that is sufficiently
> + /// aligned for and holds a valid `T`. The caller passes ownership of the allocation to the
> + /// `Box`.
You don't say what must happen for ZSTs.
> + #[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);
> +
> + b.0.as_ptr()
> + }
> +
> + /// Consumes and leaks the `Box<T, A>` and returns a mutable reference.
> + ///
> + /// See [Box::into_raw] for more details.
> + #[inline]
> + pub fn leak<'a>(b: Self) -> &'a mut T {
> + // SAFETY: `Box::into_raw` always returns a properly aligned and dereferenceable pointer
> + // which points to an initialized instance of `T`.
> + unsafe { &mut *Box::into_raw(b) }
> + }
> +}
> +
> +impl<T, A> Box<MaybeUninit<T>, A>
> +where
> + A: Allocator,
> +{
> + /// Converts a `Box<MaybeUninit<T>, A>` to a `Box<T, A>`.
> + ///
> + /// It is undefined behavior to call this function while the value inside of `b` is not yet
> + /// fully initialized.
> + ///
> + /// # Safety
> + ///
> + /// Callers must ensure that the value inside of `b` is in an initialized state.
> + pub unsafe fn assume_init(b: Self) -> Box<T, A> {
> + let raw = Self::into_raw(b);
> +
> + // SAFETY: `raw` comes from a previous call to `Box::into_raw`. By the safety requirements
> + // of this function, the value inside the `Box` is in an initialized state. Hence, it is
> + // safe to reconstruct the `Box` as `Box<T, A>`.
> + unsafe { Box::from_raw(raw as *mut T) }
You should be able to use `.cast()` instead.
> + }
> +
> + /// Writes the value and converts to `Box<T, A>`.
> + pub fn write(mut b: Self, value: T) -> Box<T, A> {
> + (*b).write(value);
> + // SAFETY: We've just initialized `boxed`'s value.
The variable is called `b`.
> + unsafe { Self::assume_init(b) }
> + }
> +}
[...]
> +impl<T, A> Drop for Box<T, A>
> +where
> + T: ?Sized,
> + A: Allocator,
> +{
> + fn drop(&mut self) {
> + let size = core::mem::size_of_val::<T>(self);
> +
> + // SAFETY: We need to drop `self.0` in place, before we free the backing memory.
This is the reason you are calling this function, not the justification
why it is OK to do so. (the pointer is valid)
> + unsafe { core::ptr::drop_in_place(self.0.as_ptr()) };
Instead of using the raw pointer directly, you can also just use
`deref_mut`.
> +
> + if size != 0 {
> + // SAFETY: `ptr` was previously allocated with `A`.
There is no variable `ptr`, this is guaranteed by the type invariant of
`Self`.
---
Cheers,
Benno
> + unsafe { A::free(self.0.cast()) };
> + }
> + }
> +}
> diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
> index 4571daec0961..a9210634a8c3 100644
> --- a/rust/kernel/prelude.rs
> +++ b/rust/kernel/prelude.rs
> @@ -14,7 +14,7 @@
> #[doc(no_inline)]
> pub use core::pin::Pin;
>
> -pub use crate::alloc::{box_ext::BoxExt, flags::*, vec_ext::VecExt};
> +pub use crate::alloc::{box_ext::BoxExt, flags::*, vec_ext::VecExt, KBox, KVBox, VBox};
>
> #[doc(no_inline)]
> pub use alloc::{boxed::Box, vec::Vec};
> --
> 2.46.0
>
