linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Gary Guo <gary@garyguo.net>
To: Danilo Krummrich <dakr@kernel.org>
Cc: ojeda@kernel.org, alex.gaynor@gmail.com, wedsonaf@gmail.com,
	boqun.feng@gmail.com, bjorn3_gh@protonmail.com,
	benno.lossin@proton.me, a.hindborg@samsung.com,
	aliceryhl@google.com, akpm@linux-foundation.org,
	daniel.almeida@collabora.com, faith.ekstrand@collabora.com,
	boris.brezillon@collabora.com, lina@asahilina.net,
	mcanal@igalia.com, zhiw@nvidia.com, cjia@nvidia.com,
	jhubbard@nvidia.com, airlied@redhat.com, ajanulgu@redhat.com,
	lyude@redhat.com, linux-kernel@vger.kernel.org,
	rust-for-linux@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH v7 09/26] rust: alloc: implement kernel `Box`
Date: Sat, 28 Sep 2024 19:54:36 +0100	[thread overview]
Message-ID: <20240928195436.7f9c1674.gary@garyguo.net> (raw)
In-Reply-To: <20240911225449.152928-10-dakr@kernel.org>

On Thu, 12 Sep 2024 00:52:45 +0200
Danilo Krummrich <dakr@kernel.org> wrote:

> `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 <dakr@kernel.org>
> ---
>  rust/kernel/alloc.rs      |   6 +
>  rust/kernel/alloc/kbox.rs | 465 ++++++++++++++++++++++++++++++++++++++
>  rust/kernel/prelude.rs    |   2 +-
>  3 files changed, 472 insertions(+), 1 deletion(-)
>  create mode 100644 rust/kernel/alloc/kbox.rs
> 
> diff --git a/rust/kernel/alloc.rs b/rust/kernel/alloc.rs
> index 7a405d6f3034..b1f28334b27a 100644
> --- a/rust/kernel/alloc.rs
> +++ b/rust/kernel/alloc.rs
> @@ -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;
> diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs
> new file mode 100644
> index 000000000000..6188494f040d
> --- /dev/null
> +++ b/rust/kernel/alloc/kbox.rs
> @@ -0,0 +1,465 @@
> +// 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`.
> +///
> +/// 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 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());
> +/// ```
> +///
> +/// ```
> +/// # 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());
> +/// ```
> +///
> +/// # Invariants
> +///
> +/// `self.0` is always properly aligned and either points to memory allocated with `A` or, for
> +/// zero-sized types, is a 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.
> +///
> +/// # 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`.
> +    ///
> +    /// For ZSTs, `raw` must be a dangling, well aligned 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>)

nit: the turbofish here is not necessary.

	Self(unsafe { NonNull::new_unchecked(raw) }, PhantomData)

> +    }
> +
> +    /// 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> {

I think this can take `self` and shadow the underlying
`MaybeUninit::assume_init` -- the rationale being that the
`MaybeUninit::assume_init` takes `self` so it cannot actually be used
for `Box<MaybeUninit<T>>`. Rust standard library also takes the
approach (with the unstable box assume_init takes `self`).

Note the argument of `Box` not having `self` methods don't apply here,
as it doesn't deref to an arbitrary type, but rather always
`MaybeUninit`.

> +        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.cast()) }
> +    }
> +
> +    /// 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 `b`'s value.
> +        unsafe { Self::assume_init(b) }
> +    }

same here.

> +}
> +
> +impl<T, A> Box<T, A>
> +where
> +    A: Allocator,
> +{
> +    fn is_zst() -> bool {
> +        core::mem::size_of::<T>() == 0
> +    }
> +
> +    /// Creates a new `Box<T, A>` and initializes its contents with `x`.
> +    ///
> +    /// New memory is allocated with `A`. The allocation may fail, in which case an error is
> +    /// returned. For ZSTs no memory is allocated.
> +    pub fn new(x: T, flags: Flags) -> Result<Self, AllocError> {
> +        let b = Self::new_uninit(flags)?;
> +        Ok(Box::write(b, x))
> +    }


  parent reply	other threads:[~2024-09-28 18:54 UTC|newest]

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-11 22:52 [PATCH v7 00/26] Generic `Allocator` support for Rust Danilo Krummrich
2024-09-11 22:52 ` [PATCH v7 01/26] rust: alloc: add `Allocator` trait Danilo Krummrich
2024-09-15 15:28   ` Gary Guo
2024-09-15 17:02     ` Danilo Krummrich
2024-09-15 19:22       ` Gary Guo
2024-09-15 20:08         ` Gary Guo
2024-09-15 21:39           ` Danilo Krummrich
2024-09-15 21:37         ` Danilo Krummrich
2024-09-21 15:32   ` [RFC PATCH] rust: alloc: pass `old_layout` to `Allocator` Danilo Krummrich
     [not found]     ` <CAH5fLgievsJOe7QET+Wh2c0upygv-nhSnOuTN8K_QkruLwOPgA@mail.gmail.com>
2024-09-23 15:20       ` Benno Lossin
2024-09-23 16:13       ` Gary Guo
2024-09-24 13:31         ` Danilo Krummrich
2024-09-24 13:34           ` Danilo Krummrich
2024-09-24 19:58           ` Gary Guo
2024-09-11 22:52 ` [PATCH v7 02/26] rust: alloc: separate `aligned_size` from `krealloc_aligned` Danilo Krummrich
2024-09-11 22:52 ` [PATCH v7 03/26] rust: alloc: rename `KernelAllocator` to `Kmalloc` Danilo Krummrich
2024-09-11 22:52 ` [PATCH v7 04/26] rust: alloc: implement `Allocator` for `Kmalloc` Danilo Krummrich
2024-09-26 13:00   ` Benno Lossin
2024-09-26 13:24     ` Danilo Krummrich
2024-09-26 14:00       ` Benno Lossin
2024-09-11 22:52 ` [PATCH v7 05/26] rust: alloc: add module `allocator_test` Danilo Krummrich
2024-09-11 22:52 ` [PATCH v7 06/26] rust: alloc: implement `Vmalloc` allocator Danilo Krummrich
2024-09-26 13:06   ` Benno Lossin
2024-09-11 22:52 ` [PATCH v7 07/26] rust: alloc: implement `KVmalloc` allocator Danilo Krummrich
2024-09-26 13:07   ` Benno Lossin
2024-09-11 22:52 ` [PATCH v7 08/26] rust: alloc: add __GFP_NOWARN to `Flags` Danilo Krummrich
2024-09-28 18:55   ` Gary Guo
2024-09-11 22:52 ` [PATCH v7 09/26] rust: alloc: implement kernel `Box` Danilo Krummrich
2024-09-26 13:23   ` Benno Lossin
2024-09-28 18:54   ` Gary Guo [this message]
2024-09-11 22:52 ` [PATCH v7 10/26] rust: treewide: switch to our kernel `Box` type Danilo Krummrich
2024-09-28 18:59   ` Gary Guo
2024-09-29 14:52     ` Danilo Krummrich
2024-09-11 22:52 ` [PATCH v7 11/26] rust: alloc: remove extension of std's `Box` Danilo Krummrich
2024-09-28 19:00   ` Gary Guo
2024-09-11 22:52 ` [PATCH v7 12/26] rust: alloc: add `Box` to prelude Danilo Krummrich
2024-09-28 19:00   ` Gary Guo
2024-09-11 22:52 ` [PATCH v7 13/26] rust: alloc: implement kernel `Vec` type Danilo Krummrich
2024-09-26 13:47   ` Benno Lossin
2024-09-28 12:43     ` Danilo Krummrich
2024-09-28 13:20       ` Benno Lossin
2024-09-28 19:14   ` Gary Guo
2024-09-11 22:52 ` [PATCH v7 14/26] rust: alloc: implement `IntoIterator` for `Vec` Danilo Krummrich
2024-09-26 13:53   ` Benno Lossin
2024-09-28 19:20   ` Gary Guo
2024-09-11 22:52 ` [PATCH v7 15/26] rust: alloc: implement `collect` for `IntoIter` Danilo Krummrich
2024-09-26 13:57   ` Benno Lossin
2024-09-28 19:27   ` Gary Guo
2024-09-29 15:12     ` Danilo Krummrich
2024-09-11 22:52 ` [PATCH v7 16/26] rust: treewide: switch to the kernel `Vec` type Danilo Krummrich
2024-09-28 19:28   ` Gary Guo
2024-09-11 22:52 ` [PATCH v7 17/26] rust: alloc: remove `VecExt` extension Danilo Krummrich
2024-09-28 19:29   ` Gary Guo
2024-09-11 22:52 ` [PATCH v7 18/26] rust: alloc: add `Vec` to prelude Danilo Krummrich
2024-09-28 19:29   ` Gary Guo
2024-09-11 22:52 ` [PATCH v7 19/26] rust: error: use `core::alloc::LayoutError` Danilo Krummrich
2024-09-28 19:30   ` Gary Guo
2024-09-11 22:52 ` [PATCH v7 20/26] rust: error: check for config `test` in `Error::name` Danilo Krummrich
2024-09-28 19:30   ` Gary Guo
2024-09-11 22:52 ` [PATCH v7 21/26] rust: alloc: implement `contains` for `Flags` Danilo Krummrich
2024-09-28 19:31   ` Gary Guo
2024-09-11 22:52 ` [PATCH v7 22/26] rust: alloc: implement `Cmalloc` in module allocator_test Danilo Krummrich
2024-09-28 19:35   ` Gary Guo
2024-09-29 15:14     ` Danilo Krummrich
2024-09-11 22:52 ` [PATCH v7 23/26] rust: str: test: replace `alloc::format` Danilo Krummrich
2024-09-28 19:37   ` Gary Guo
2024-09-11 22:53 ` [PATCH v7 24/26] rust: alloc: update module comment of alloc.rs Danilo Krummrich
2024-09-28 19:38   ` Gary Guo
2024-09-11 22:53 ` [PATCH v7 25/26] kbuild: rust: remove the `alloc` crate and `GlobalAlloc` Danilo Krummrich
2024-09-28 19:43   ` Gary Guo
2024-09-29 15:17     ` Danilo Krummrich
2024-10-01 13:27       ` Danilo Krummrich
     [not found]     ` <CANiq72nuZ41eDXkybGBbGRMgJzUOe1rRZioS-amJfH7UV-9cMQ@mail.gmail.com>
2024-10-03 21:53       ` Danilo Krummrich
2024-09-11 22:53 ` [PATCH v7 26/26] MAINTAINERS: add entry for the Rust `alloc` module Danilo Krummrich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240928195436.7f9c1674.gary@garyguo.net \
    --to=gary@garyguo.net \
    --cc=a.hindborg@samsung.com \
    --cc=airlied@redhat.com \
    --cc=ajanulgu@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=benno.lossin@proton.me \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=boris.brezillon@collabora.com \
    --cc=cjia@nvidia.com \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=faith.ekstrand@collabora.com \
    --cc=jhubbard@nvidia.com \
    --cc=lina@asahilina.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lyude@redhat.com \
    --cc=mcanal@igalia.com \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=wedsonaf@gmail.com \
    --cc=zhiw@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox