From 08d3f54928796557fc832467ad54f04908fc14e4 Mon Sep 17 00:00:00 2001 From: Wedson Almeida Filho Date: Wed, 27 Mar 2024 22:35:59 -0300 Subject: rust: alloc: introduce the `BoxExt` trait Make fallible versions of `new` and `new_uninit` methods available in `Box` even though it doesn't implement them because we build `alloc` with the `no_global_oom_handling` config. They also have an extra `flags` parameter that allows callers to pass flags to the allocator. Signed-off-by: Wedson Almeida Filho Reviewed-by: Boqun Feng Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20240328013603.206764-7-wedsonaf@gmail.com [ Used `Box::write()` to avoid one `unsafe` block as suggested by Boqun. ] Signed-off-by: Miguel Ojeda --- rust/kernel/sync/arc.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'rust/kernel/sync/arc.rs') diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs index 7d4c4bf58388..1252a1b630ed 100644 --- a/rust/kernel/sync/arc.rs +++ b/rust/kernel/sync/arc.rs @@ -16,6 +16,7 @@ //! [`Arc`]: https://doc.rust-lang.org/std/sync/struct.Arc.html use crate::{ + alloc::{box_ext::BoxExt, flags::*}, bindings, error::{self, Error}, init::{self, InPlaceInit, Init, PinInit}, @@ -170,7 +171,7 @@ impl Arc { data: contents, }; - let inner = Box::try_new(value)?; + let inner = as BoxExt<_>>::new(value, GFP_KERNEL)?; // SAFETY: We just created `inner` with a reference count of 1, which is owned by the new // `Arc` object. -- cgit From cc41670e06383c08f3afdd7a19b782d03ae4d63a Mon Sep 17 00:00:00 2001 From: Wedson Almeida Filho Date: Wed, 27 Mar 2024 22:36:01 -0300 Subject: rust: sync: update `Arc` and `UniqueArc` to take allocation flags We also remove the `try_` prefix to align with how `Box` and `Vec` are providing methods now. `init` is temporarily updated with uses of GFP_KERNEL. These will be updated in a subsequent patch to take flags as well. Reviewed-by: Benno Lossin Signed-off-by: Wedson Almeida Filho Link: https://lore.kernel.org/r/20240328013603.206764-9-wedsonaf@gmail.com Signed-off-by: Miguel Ojeda --- rust/kernel/sync/arc.rs | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) (limited to 'rust/kernel/sync/arc.rs') diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs index 1252a1b630ed..b67bb876ddf7 100644 --- a/rust/kernel/sync/arc.rs +++ b/rust/kernel/sync/arc.rs @@ -16,7 +16,7 @@ //! [`Arc`]: https://doc.rust-lang.org/std/sync/struct.Arc.html use crate::{ - alloc::{box_ext::BoxExt, flags::*}, + alloc::{box_ext::BoxExt, Flags}, bindings, error::{self, Error}, init::{self, InPlaceInit, Init, PinInit}, @@ -58,7 +58,7 @@ mod std_vendor; /// } /// /// // Create a refcounted instance of `Example`. -/// let obj = Arc::try_new(Example { a: 10, b: 20 })?; +/// let obj = Arc::new(Example { a: 10, b: 20 }, GFP_KERNEL)?; /// /// // Get a new pointer to `obj` and increment the refcount. /// let cloned = obj.clone(); @@ -97,7 +97,7 @@ mod std_vendor; /// } /// } /// -/// let obj = Arc::try_new(Example { a: 10, b: 20 })?; +/// let obj = Arc::new(Example { a: 10, b: 20 }, GFP_KERNEL)?; /// obj.use_reference(); /// obj.take_over(); /// # Ok::<(), Error>(()) @@ -120,7 +120,7 @@ mod std_vendor; /// impl MyTrait for Example {} /// /// // `obj` has type `Arc`. -/// let obj: Arc = Arc::try_new(Example)?; +/// let obj: Arc = Arc::new(Example, GFP_KERNEL)?; /// /// // `coerced` has type `Arc`. /// let coerced: Arc = obj; @@ -163,7 +163,7 @@ unsafe impl Sync for Arc {} impl Arc { /// Constructs a new reference counted instance of `T`. - pub fn try_new(contents: T) -> Result { + pub fn new(contents: T, flags: Flags) -> Result { // INVARIANT: The refcount is initialised to a non-zero value. let value = ArcInner { // SAFETY: There are no safety requirements for this FFI call. @@ -171,7 +171,7 @@ impl Arc { data: contents, }; - let inner = as BoxExt<_>>::new(value, GFP_KERNEL)?; + let inner = as BoxExt<_>>::new(value, flags)?; // SAFETY: We just created `inner` with a reference count of 1, which is owned by the new // `Arc` object. @@ -388,7 +388,7 @@ impl From>> for Arc { /// e.into() /// } /// -/// let obj = Arc::try_new(Example)?; +/// let obj = Arc::new(Example, GFP_KERNEL)?; /// let cloned = do_something(obj.as_arc_borrow()); /// /// // Assert that both `obj` and `cloned` point to the same underlying object. @@ -412,7 +412,7 @@ impl From>> for Arc { /// } /// } /// -/// let obj = Arc::try_new(Example { a: 10, b: 20 })?; +/// let obj = Arc::new(Example { a: 10, b: 20 }, GFP_KERNEL)?; /// obj.as_arc_borrow().use_reference(); /// # Ok::<(), Error>(()) /// ``` @@ -500,7 +500,7 @@ impl Deref for ArcBorrow<'_, T> { /// } /// /// fn test() -> Result> { -/// let mut x = UniqueArc::try_new(Example { a: 10, b: 20 })?; +/// let mut x = UniqueArc::new(Example { a: 10, b: 20 }, GFP_KERNEL)?; /// x.a += 1; /// x.b += 1; /// Ok(x.into()) @@ -523,7 +523,7 @@ impl Deref for ArcBorrow<'_, T> { /// } /// /// fn test() -> Result> { -/// let x = UniqueArc::try_new_uninit()?; +/// let x = UniqueArc::new_uninit(GFP_KERNEL)?; /// Ok(x.write(Example { a: 10, b: 20 }).into()) /// } /// @@ -543,7 +543,7 @@ impl Deref for ArcBorrow<'_, T> { /// } /// /// fn test() -> Result> { -/// let mut pinned = Pin::from(UniqueArc::try_new(Example { a: 10, b: 20 })?); +/// let mut pinned = Pin::from(UniqueArc::new(Example { a: 10, b: 20 }, GFP_KERNEL)?); /// // We can modify `pinned` because it is `Unpin`. /// pinned.as_mut().a += 1; /// Ok(pinned.into()) @@ -557,15 +557,15 @@ pub struct UniqueArc { impl UniqueArc { /// Tries to allocate a new [`UniqueArc`] instance. - pub fn try_new(value: T) -> Result { + pub fn new(value: T, flags: Flags) -> Result { Ok(Self { // INVARIANT: The newly-created object has a refcount of 1. - inner: Arc::try_new(value)?, + inner: Arc::new(value, flags)?, }) } /// Tries to allocate a new [`UniqueArc`] instance whose contents are not initialised yet. - pub fn try_new_uninit() -> Result>, AllocError> { + pub fn new_uninit(_flags: Flags) -> Result>, AllocError> { // INVARIANT: The refcount is initialised to a non-zero value. let inner = Box::try_init::(try_init!(ArcInner { // SAFETY: There are no safety requirements for this FFI call. -- cgit From c34aa00d1d7dd482dc48660ad594cb693334de2d Mon Sep 17 00:00:00 2001 From: Wedson Almeida Filho Date: Wed, 27 Mar 2024 22:36:02 -0300 Subject: rust: init: update `init` module to take allocation flags This is the last component in the conversion for allocators to take allocation flags as parameters. Reviewed-by: Benno Lossin Signed-off-by: Wedson Almeida Filho Link: https://lore.kernel.org/r/20240328013603.206764-10-wedsonaf@gmail.com Signed-off-by: Miguel Ojeda --- rust/kernel/sync/arc.rs | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) (limited to 'rust/kernel/sync/arc.rs') diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs index b67bb876ddf7..0866378f1360 100644 --- a/rust/kernel/sync/arc.rs +++ b/rust/kernel/sync/arc.rs @@ -182,22 +182,22 @@ impl Arc { /// /// If `T: !Unpin` it will not be able to move afterwards. #[inline] - pub fn pin_init(init: impl PinInit) -> error::Result + pub fn pin_init(init: impl PinInit, flags: Flags) -> error::Result where Error: From, { - UniqueArc::pin_init(init).map(|u| u.into()) + UniqueArc::pin_init(init, flags).map(|u| u.into()) } /// Use the given initializer to in-place initialize a `T`. /// /// This is equivalent to [`Arc::pin_init`], since an [`Arc`] is always pinned. #[inline] - pub fn init(init: impl Init) -> error::Result + pub fn init(init: impl Init, flags: Flags) -> error::Result where Error: From, { - UniqueArc::init(init).map(|u| u.into()) + UniqueArc::init(init, flags).map(|u| u.into()) } } @@ -565,13 +565,16 @@ impl UniqueArc { } /// Tries to allocate a new [`UniqueArc`] instance whose contents are not initialised yet. - pub fn new_uninit(_flags: Flags) -> Result>, AllocError> { + pub fn new_uninit(flags: Flags) -> Result>, AllocError> { // INVARIANT: The refcount is initialised to a non-zero value. - let inner = Box::try_init::(try_init!(ArcInner { - // SAFETY: There are no safety requirements for this FFI call. - refcount: Opaque::new(unsafe { bindings::REFCOUNT_INIT(1) }), - data <- init::uninit::(), - }? AllocError))?; + let inner = Box::try_init::( + try_init!(ArcInner { + // SAFETY: There are no safety requirements for this FFI call. + refcount: Opaque::new(unsafe { bindings::REFCOUNT_INIT(1) }), + data <- init::uninit::(), + }? AllocError), + flags, + )?; Ok(UniqueArc { // INVARIANT: The newly-created object has a refcount of 1. // SAFETY: The pointer from the `Box` is valid. -- cgit From 2c1092853f163762ef0aabc551a630ef233e1be3 Mon Sep 17 00:00:00 2001 From: Wedson Almeida Filho Date: Wed, 27 Mar 2024 22:36:03 -0300 Subject: rust: kernel: remove usage of `allocator_api` unstable feature With the adoption of `BoxExt` and `VecExt`, we don't need the functions provided by this feature (namely the methods prefixed with `try_` and different allocator per collection instance). We do need `AllocError`, but we define our own as it is a trivial empty struct. Reviewed-by: Benno Lossin Signed-off-by: Wedson Almeida Filho Link: https://lore.kernel.org/r/20240328013603.206764-11-wedsonaf@gmail.com Signed-off-by: Miguel Ojeda --- rust/kernel/sync/arc.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'rust/kernel/sync/arc.rs') diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs index 0866378f1360..c2a3a2c7cbc5 100644 --- a/rust/kernel/sync/arc.rs +++ b/rust/kernel/sync/arc.rs @@ -16,7 +16,7 @@ //! [`Arc`]: https://doc.rust-lang.org/std/sync/struct.Arc.html use crate::{ - alloc::{box_ext::BoxExt, Flags}, + alloc::{box_ext::BoxExt, AllocError, Flags}, bindings, error::{self, Error}, init::{self, InPlaceInit, Init, PinInit}, @@ -25,7 +25,7 @@ use crate::{ }; use alloc::boxed::Box; use core::{ - alloc::{AllocError, Layout}, + alloc::Layout, fmt, marker::{PhantomData, Unsize}, mem::{ManuallyDrop, MaybeUninit}, -- cgit From 51f6af86de35bfd12a005caf457f2ec834193de8 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Tue, 2 Apr 2024 13:07:42 +0000 Subject: rust: sync: add `ArcBorrow::from_raw` Allows access to a value in an `Arc` that is currently held as a raw pointer due to use of `Arc::into_raw`, without destroying or otherwise consuming that raw pointer. This is a dependency of the linked list that Rust Binder uses. The linked list uses this method when iterating over the linked list [1]. Reviewed-by: Benno Lossin Reviewed-by: Boqun Feng Link: https://lore.kernel.org/r/20240402-linked-list-v1-6-b1c59ba7ae3b@google.com [1] Signed-off-by: Alice Ryhl Link: https://lore.kernel.org/r/20240402-arc-for-list-v4-1-54db6440a9a9@google.com Signed-off-by: Miguel Ojeda --- rust/kernel/sync/arc.rs | 76 +++++++++++++++++++++++++++++++++++++------------ 1 file changed, 58 insertions(+), 18 deletions(-) (limited to 'rust/kernel/sync/arc.rs') diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs index c2a3a2c7cbc5..730d11ec766e 100644 --- a/rust/kernel/sync/arc.rs +++ b/rust/kernel/sync/arc.rs @@ -138,6 +138,39 @@ struct ArcInner { data: T, } +impl ArcInner { + /// Converts a pointer to the contents of an [`Arc`] into a pointer to the [`ArcInner`]. + /// + /// # Safety + /// + /// `ptr` must have been returned by a previous call to [`Arc::into_raw`], and the `Arc` must + /// not yet have been destroyed. + unsafe fn container_of(ptr: *const T) -> NonNull> { + let refcount_layout = Layout::new::(); + // SAFETY: The caller guarantees that the pointer is valid. + let val_layout = Layout::for_value(unsafe { &*ptr }); + // SAFETY: We're computing the layout of a real struct that existed when compiling this + // binary, so its layout is not so large that it can trigger arithmetic overflow. + let val_offset = unsafe { refcount_layout.extend(val_layout).unwrap_unchecked().1 }; + + // Pointer casts leave the metadata unchanged. This is okay because the metadata of `T` and + // `ArcInner` is the same since `ArcInner` is a struct with `T` as its last field. + // + // This is documented at: + // . + let ptr = ptr as *const ArcInner; + + // SAFETY: The pointer is in-bounds of an allocation both before and after offsetting the + // pointer, since it originates from a previous call to `Arc::into_raw` on an `Arc` that is + // still valid. + let ptr = unsafe { ptr.byte_sub(val_offset) }; + + // SAFETY: The pointer can't be null since you can't have an `ArcInner` value at the null + // address. + unsafe { NonNull::new_unchecked(ptr.cast_mut()) } + } +} + // This is to allow [`Arc`] (and variants) to be used as the type of `self`. impl core::ops::Receiver for Arc {} @@ -233,27 +266,13 @@ impl Arc { /// `ptr` must have been returned by a previous call to [`Arc::into_raw`]. Additionally, it /// must not be called more than once for each previous call to [`Arc::into_raw`]. pub unsafe fn from_raw(ptr: *const T) -> Self { - let refcount_layout = Layout::new::(); - // SAFETY: The caller guarantees that the pointer is valid. - let val_layout = Layout::for_value(unsafe { &*ptr }); - // SAFETY: We're computing the layout of a real struct that existed when compiling this - // binary, so its layout is not so large that it can trigger arithmetic overflow. - let val_offset = unsafe { refcount_layout.extend(val_layout).unwrap_unchecked().1 }; - - // Pointer casts leave the metadata unchanged. This is okay because the metadata of `T` and - // `ArcInner` is the same since `ArcInner` is a struct with `T` as its last field. - // - // This is documented at: - // . - let ptr = ptr as *const ArcInner; - - // SAFETY: The pointer is in-bounds of an allocation both before and after offsetting the - // pointer, since it originates from a previous call to `Arc::into_raw` and is still valid. - let ptr = unsafe { ptr.byte_sub(val_offset) }; + // SAFETY: The caller promises that this pointer originates from a call to `into_raw` on an + // `Arc` that is still valid. + let ptr = unsafe { ArcInner::container_of(ptr) }; // SAFETY: By the safety requirements we know that `ptr` came from `Arc::into_raw`, so the // reference count held then will be owned by the new `Arc` object. - unsafe { Self::from_inner(NonNull::new_unchecked(ptr.cast_mut())) } + unsafe { Self::from_inner(ptr) } } /// Returns an [`ArcBorrow`] from the given [`Arc`]. @@ -454,6 +473,27 @@ impl ArcBorrow<'_, T> { _p: PhantomData, } } + + /// Creates an [`ArcBorrow`] to an [`Arc`] that has previously been deconstructed with + /// [`Arc::into_raw`]. + /// + /// # Safety + /// + /// * The provided pointer must originate from a call to [`Arc::into_raw`]. + /// * For the duration of the lifetime annotated on this `ArcBorrow`, the reference count must + /// not hit zero. + /// * For the duration of the lifetime annotated on this `ArcBorrow`, there must not be a + /// [`UniqueArc`] reference to this value. + pub unsafe fn from_raw(ptr: *const T) -> Self { + // SAFETY: The caller promises that this pointer originates from a call to `into_raw` on an + // `Arc` that is still valid. + let ptr = unsafe { ArcInner::container_of(ptr) }; + + // SAFETY: The caller promises that the value remains valid since the reference count must + // not hit zero, and no mutable reference will be created since that would involve a + // `UniqueArc`. + unsafe { Self::new(ptr) } + } } impl From> for Arc { -- cgit From a0a4e17013f68739733028bba89673cdbb9caabd Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Tue, 2 Apr 2024 13:07:43 +0000 Subject: rust: sync: add `Arc::into_unique_or_drop` Decrement the refcount of an `Arc`, but handle the case where it hits zero by taking ownership of the now-unique `Arc`, instead of destroying and deallocating it. This is a dependency of the linked list that Rust Binder uses. The linked list uses this method as part of its `ListArc` abstraction [1]. Boqun Feng has authored the examples. Reviewed-by: Benno Lossin Reviewed-by: Boqun Feng Link: https://lore.kernel.org/r/20240402-linked-list-v1-1-b1c59ba7ae3b@google.com [1] Co-developed-by: Boqun Feng Signed-off-by: Boqun Feng Signed-off-by: Alice Ryhl Link: https://lore.kernel.org/r/20240402-arc-for-list-v4-2-54db6440a9a9@google.com [ Replace `try_new` with `new` in example since we now have the new allocation APIs. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/sync/arc.rs | 62 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) (limited to 'rust/kernel/sync/arc.rs') diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs index 730d11ec766e..a65716ec24a6 100644 --- a/rust/kernel/sync/arc.rs +++ b/rust/kernel/sync/arc.rs @@ -291,6 +291,68 @@ impl Arc { pub fn ptr_eq(this: &Self, other: &Self) -> bool { core::ptr::eq(this.ptr.as_ptr(), other.ptr.as_ptr()) } + + /// Converts this [`Arc`] into a [`UniqueArc`], or destroys it if it is not unique. + /// + /// When this destroys the `Arc`, it does so while properly avoiding races. This means that + /// this method will never call the destructor of the value. + /// + /// # Examples + /// + /// ``` + /// use kernel::sync::{Arc, UniqueArc}; + /// + /// let arc = Arc::new(42, GFP_KERNEL)?; + /// let unique_arc = arc.into_unique_or_drop(); + /// + /// // The above conversion should succeed since refcount of `arc` is 1. + /// assert!(unique_arc.is_some()); + /// + /// assert_eq!(*(unique_arc.unwrap()), 42); + /// + /// # Ok::<(), Error>(()) + /// ``` + /// + /// ``` + /// use kernel::sync::{Arc, UniqueArc}; + /// + /// let arc = Arc::new(42, GFP_KERNEL)?; + /// let another = arc.clone(); + /// + /// let unique_arc = arc.into_unique_or_drop(); + /// + /// // The above conversion should fail since refcount of `arc` is >1. + /// assert!(unique_arc.is_none()); + /// + /// # Ok::<(), Error>(()) + /// ``` + pub fn into_unique_or_drop(self) -> Option>> { + // We will manually manage the refcount in this method, so we disable the destructor. + let me = ManuallyDrop::new(self); + // SAFETY: We own a refcount, so the pointer is still valid. + let refcount = unsafe { me.ptr.as_ref() }.refcount.get(); + + // If the refcount reaches a non-zero value, then we have destroyed this `Arc` and will + // return without further touching the `Arc`. If the refcount reaches zero, then there are + // no other arcs, and we can create a `UniqueArc`. + // + // SAFETY: We own a refcount, so the pointer is not dangling. + let is_zero = unsafe { bindings::refcount_dec_and_test(refcount) }; + if is_zero { + // SAFETY: We have exclusive access to the arc, so we can perform unsynchronized + // accesses to the refcount. + unsafe { core::ptr::write(refcount, bindings::REFCOUNT_INIT(1)) }; + + // INVARIANT: We own the only refcount to this arc, so we may create a `UniqueArc`. We + // must pin the `UniqueArc` because the values was previously in an `Arc`, and they pin + // their values. + Some(Pin::from(UniqueArc { + inner: ManuallyDrop::into_inner(me), + })) + } else { + None + } + } } impl ForeignOwnable for Arc { -- cgit From 00280272a0e5d98055e4d47db38a9b4b5517520e Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Mon, 1 Apr 2024 23:23:02 +0200 Subject: rust: kernel: remove redundant imports Rust's `unused_imports` lint covers both unused and redundant imports. In the upcoming 1.78.0, the lint detects more cases of redundant imports [1], e.g.: error: the item `bindings` is imported redundantly --> rust/kernel/print.rs:38:9 | 38 | use crate::bindings; | ^^^^^^^^^^^^^^^ the item `bindings` is already defined by prelude Most cases are `use crate::bindings`, plus a few other items like `Box`. Thus clean them up. Note that, in the `bindings` case, the message "defined by prelude" above means the extern prelude, i.e. the `--extern` flags we pass. Link: https://github.com/rust-lang/rust/pull/117772 [1] Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20240401212303.537355-3-ojeda@kernel.org Signed-off-by: Miguel Ojeda --- rust/kernel/sync/arc.rs | 1 - 1 file changed, 1 deletion(-) (limited to 'rust/kernel/sync/arc.rs') diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs index a65716ec24a6..3673496c2363 100644 --- a/rust/kernel/sync/arc.rs +++ b/rust/kernel/sync/arc.rs @@ -17,7 +17,6 @@ use crate::{ alloc::{box_ext::BoxExt, AllocError, Flags}, - bindings, error::{self, Error}, init::{self, InPlaceInit, Init, PinInit}, try_init, -- cgit