From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9F9D1C5474C for ; Sat, 31 Aug 2024 05:39:20 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 08EE68D0015; Sat, 31 Aug 2024 01:39:20 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 03F0A8D0013; Sat, 31 Aug 2024 01:39:19 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E497E8D0015; Sat, 31 Aug 2024 01:39:19 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id B83FE8D0013 for ; Sat, 31 Aug 2024 01:39:19 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 2037D1C41C4 for ; Sat, 31 Aug 2024 05:39:19 +0000 (UTC) X-FDA: 82511437638.05.D295346 Received: from mail-40134.protonmail.ch (mail-40134.protonmail.ch [185.70.40.134]) by imf20.hostedemail.com (Postfix) with ESMTP id C52EA1C000A for ; Sat, 31 Aug 2024 05:39:16 +0000 (UTC) Authentication-Results: imf20.hostedemail.com; dkim=pass header.d=proton.me header.s=protonmail header.b=HE7AO8Vi; spf=pass (imf20.hostedemail.com: domain of benno.lossin@proton.me designates 185.70.40.134 as permitted sender) smtp.mailfrom=benno.lossin@proton.me; dmarc=pass (policy=quarantine) header.from=proton.me ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1725082666; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=H0yhunYtVRJSRKYFJ1oR8mA4txjPQQLzLVyKGXLbgQw=; b=7g2QDUA4MXdZ9N8q0P1livxGZlrjEk1QjHgWjGfWCoGHotV8dY++LTUCxukQ75gPdzjDbI 4x/PW3RJeAIyh6ROX+euTvN7oYKl0yvNePp8Ui4P1tx5Cgsd0JJNI7D7/B6jL0xjvsCxRf i9MHMQp3ZaZrRNNuDUd9gdx5CAY+qOI= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1725082666; a=rsa-sha256; cv=none; b=UMdV9kIb/s6cdonRDT7b4Q9qUXohW3v1HkRqMmh0VNiCBxHe1BkNbbG7xlw53hYFOAx8GR xnvQYVMNlzFSd7TvepYQlRPfok4ges07Hc9k5XJUen3ePJC4QL2AdQqAArK9gUZOWCav3u HxFdP4LPBqT6sEd9eqVcdb/jtJkYlfA= ARC-Authentication-Results: i=1; imf20.hostedemail.com; dkim=pass header.d=proton.me header.s=protonmail header.b=HE7AO8Vi; spf=pass (imf20.hostedemail.com: domain of benno.lossin@proton.me designates 185.70.40.134 as permitted sender) smtp.mailfrom=benno.lossin@proton.me; dmarc=pass (policy=quarantine) header.from=proton.me DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=proton.me; s=protonmail; t=1725082753; x=1725341953; bh=H0yhunYtVRJSRKYFJ1oR8mA4txjPQQLzLVyKGXLbgQw=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector; b=HE7AO8Vi8VdhtuYn2MrDdC+iDn1DisKqFwwCfE+hI4fXCXk0EnLoXihOTHOyl2uHc L7P1tVQX5LB8vRKuH8Adj7Kea9FSGCKR25ZTwnlqMUX304PKVqfesD5WO9YUi1dKNr 6SY0rybcldbSRTLNll9cCEulSWm+IJoSZs55q7EZb5TGz5gaZxRjyPuqvOXTrmIs9T NupZfN0+ByG0JW75Hqlm0nfiBoZiRtv/f9FIfWxyosHFRfU8pVRybpnP86ogwxzgJQ 0FmftV8e9OsgQV2jM5FpieFuJc29D9k+XkZrfbp9LUany8oFuBFaqx+VhS2DtLqTen iAsvEJtRRVRDg== Date: Sat, 31 Aug 2024 05:39:07 +0000 To: Danilo Krummrich , ojeda@kernel.org, alex.gaynor@gmail.com, wedsonaf@gmail.com, boqun.feng@gmail.com, gary@garyguo.net, bjorn3_gh@protonmail.com, a.hindborg@samsung.com, aliceryhl@google.com, akpm@linux-foundation.org From: Benno Lossin Cc: 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 v6 09/26] rust: alloc: implement kernel `Box` Message-ID: <0146cb78-5a35-4d6b-bc75-fed1fc0c270c@proton.me> In-Reply-To: <20240816001216.26575-10-dakr@kernel.org> References: <20240816001216.26575-1-dakr@kernel.org> <20240816001216.26575-10-dakr@kernel.org> Feedback-ID: 71780778:user:proton X-Pm-Message-ID: 698fe482ee03c71dfbffcc1aecbcd36dd1de7e9e MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: C52EA1C000A X-Stat-Signature: ckkxrayp1ckdcsz8ysc1ow34nz7qrtge X-HE-Tag: 1725082756-320669 X-HE-Meta: U2FsdGVkX1+6KqYf5gKLTtDpU5qTPeBlqtJfQpciHFMwgtcMGNyyYDN/hCyPHKGT8B7W4XqxkhTLbecA37aj6V5vqhM6+wFOlCc6lxpfEedG3HmKFCzXB4nDvnER2dHbOnT2FXhD25Ri7RfjihfCC5e6g2CJmmDCHwfqqlEShZJLRV1VHgmsg33j7P4uZGOEqlwBHXDQ6kl25JgT9zNu0SmJG0GnDgqxK9uJmO5rRFGzukbC66rjThRNS7y53VWUv8L22eoz1o907nYTH/v2KI01KqgS3fDdWmj/QfZjEGESAa4Xa+G5vwT7StHQurKIVXJGHqKXye5FbmdcjTlZ1DPVY/wTM7WP0QiKl1RUkuoxI6dWcd0ooAHUQol6OuZdnX2J2gYS7kS3tWIpZ3bGqE1sh9Ehi6+0vVAUz7Y+egC9/vRzcXWSkbwpejaLXyAVuySzln/Q+PRcoW8eb2vkE/ahS+ec/1xro11uUYqwuiymdR/1fibLLeqV8b/kYxXSopydEGHSOc7IVXugoiRP1fS+5hmyA8uf/KURUc6kOVs+D6rc3nHmwTnmtNIeGae/uiM8w6TbhwHtItW/Ddx0ZqJapGVd68xALaPOuphlwdNOOPDeWbZ9GLwupmTpFkYmJOKy9L9rvQqbvd46WsetXBnJk95vVmZJpJQhkZiWR0deTDOCq5Ngfaef9BLumj+IavapxCIOmU011e3ZJNT+hNnDMvDJXUeh9Q0Tu46aWpkRYWrR98csxrhnouFovLOBwak+Le5tBGh7YiTcjpij2zwz8TqRO0XcJUCg2jIpivuH8DJ/1Q7MVHI2ry85d+TGrKi1aJHX6NXSwWdxtA3a5doPSkJYMC1wLPUSWHo/nJ8CwLpnCBYT+SwmgUxgBTRuEuKgpu20AacN37ONknPKScDcNXcY4fuETUnJIzsQ4gg5ZDsFTCOpQQiT/ffLoAt2zp5CbrJRCTE0eB0MiGR f0wP32Og jiURXC/k+dLFuQ1+XBSBXq8xgHX6Ji1jxZlTGWmU54FSOV00o4WtLj5sOoaxBJY2yj4eTJBVdK7VA3WLt88f+ljFZ69At5LZV55GY/FFNfc9laTkIfu2lF/olc+AJszjtS4ezYpzmhuUc5tDPCm4D7WZqz61CfiyuJA84XlY+nNlCN2hfKC4IpxKcnyrl865egUX6wRSNbpSwmZzX2kdQL/sLRtmLQV4EgIYUQPtRvMrYAXNMOGDVwrfv71/e9rjtpnRQc3mZvp6IrsYC2xqtDRthtkoqHbaFI0kGQ7G4pEBdHr2cY1Uc+Kp009/AVZONshTtyMU3ymGQ2hMeUMspJ1pdvH0pl6uazx+VvnbYFCB0F5w= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: 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 s= everal of differences, > +/// for example no `noalias` attribute is emitted and partially moving o= ut 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 allocati= ng 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 memo= ry is automatically freed. > +/// > +/// # Examples > +/// > +/// ``` > +/// let b =3D KBox::::new(24_u64, GFP_KERNEL)?; > +/// > +/// assert_eq!(*b, 24_u64); > +/// # Ok::<(), Error>(()) > +/// ``` > +/// > +/// ``` > +/// # use kernel::bindings; > +/// const SIZE: usize =3D bindings::KMALLOC_MAX_SIZE as usize + 1; > +/// struct Huge([u8; SIZE]); > +/// > +/// assert!(KBox::::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 =3D bindings::KMALLOC_MAX_SIZE as usize + 1; > +/// struct Huge([u8; SIZE]); > +/// > +/// assert!(KVBox::::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(NonNull, PhantomData); > + > +/// Type alias for `Box` with a [`Kmalloc`] allocator. Can you make these `Box` references links? > +/// > +/// # Examples > +/// > +/// ``` > +/// let b =3D KBox::new(24_u64, GFP_KERNEL)?; > +/// > +/// assert_eq!(*b, 24_u64); > +/// # Ok::<(), Error>(()) > +/// ``` > +pub type KBox =3D Box; > + > +/// Type alias for `Box` with a [`Vmalloc`] allocator. > +/// > +/// # Examples > +/// > +/// ``` > +/// let b =3D VBox::new(24_u64, GFP_KERNEL)?; > +/// > +/// assert_eq!(*b, 24_u64); > +/// # Ok::<(), Error>(()) > +/// ``` > +pub type VBox =3D Box; > + > +/// Type alias for `Box` with a [`KVmalloc`] allocator. > +/// > +/// # Examples > +/// > +/// ``` > +/// let b =3D KVBox::new(24_u64, GFP_KERNEL)?; > +/// > +/// assert_eq!(*b, 24_u64); > +/// # Ok::<(), Error>(()) > +/// ``` > +pub type KVBox =3D Box; > + > +// SAFETY: `Box` is `Send` if `T` is `Send` because the `Box` owns a `T`= . > +unsafe impl Send for Box > +where > + T: Send + ?Sized, > + A: Allocator, > +{ > +} > + > +// SAFETY: `Box` is `Sync` if `T` is `Sync` because the `Box` owns a `T`= . > +unsafe impl Sync for Box > +where > + T: Sync + ?Sized, > + A: Allocator, > +{ > +} > + > +impl Box > +where > + T: ?Sized, > + A: Allocator, > +{ > + /// Creates a new `Box` 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 o= f 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 prec= onditions of this function. > + // SAFETY: By the safety preconditions of this function, `raw` i= s not a NULL pointer. > + Self(unsafe { NonNull::new_unchecked(raw) }, PhantomData::) > + } > + > + /// Consumes the `Box` and returns a raw pointer. > + /// > + /// This will not run the destructor of `T` and for non-ZSTs the all= ocation will stay alive > + /// indefinitely. Use [`Box::from_raw`] to recover the [`Box`], drop= the value and free the > + /// allocation, if any. > + /// > + /// # Examples > + /// > + /// ``` > + /// let x =3D KBox::new(24, GFP_KERNEL)?; > + /// let ptr =3D KBox::into_raw(x); > + /// let x =3D unsafe { KBox::from_raw(ptr) }; > + /// > + /// assert_eq!(*x, 24); > + /// # Ok::<(), Error>(()) > + /// ``` > + #[inline] > + pub fn into_raw(b: Self) -> *mut T { > + let b =3D ManuallyDrop::new(b); > + > + b.0.as_ptr() > + } > + > + /// Consumes and leaks the `Box` and returns a mutable referen= ce. > + /// > + /// 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 Box, A> > +where > + A: Allocator, > +{ > + /// Converts a `Box, A>` to a `Box`. > + /// > + /// It is undefined behavior to call this function while the value i= nside of `b` is not yet > + /// fully initialized. > + /// > + /// # Safety > + /// > + /// Callers must ensure that the value inside of `b` is in an initia= lized state. > + pub unsafe fn assume_init(b: Self) -> Box { > + let raw =3D 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 initial= ized state. Hence, it is > + // safe to reconstruct the `Box` as `Box`. > + unsafe { Box::from_raw(raw as *mut T) } You should be able to use `.cast()` instead. > + } > + > + /// Writes the value and converts to `Box`. > + pub fn write(mut b: Self, value: T) -> Box { > + (*b).write(value); > + // SAFETY: We've just initialized `boxed`'s value. The variable is called `b`. > + unsafe { Self::assume_init(b) } > + } > +} [...] > +impl Drop for Box > +where > + T: ?Sized, > + A: Allocator, > +{ > + fn drop(&mut self) { > + let size =3D core::mem::size_of_val::(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 !=3D 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; >=20 > -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}; >=20 > #[doc(no_inline)] > pub use alloc::{boxed::Box, vec::Vec}; > -- > 2.46.0 >=20