Hi Daniel, I'll send the next iteration of this series.
Daniel Almeida <[email protected]> writes: > Hi Oliver, > >> On 17 Nov 2025, at 07:07, Oliver Mangold <[email protected]> wrote: >> >> From: Asahi Lina <[email protected]> >> >> By analogy to `AlwaysRefCounted` and `ARef`, an `Ownable` type is a >> (typically C FFI) type that *may* be owned by Rust, but need not be. Unlike >> `AlwaysRefCounted`, this mechanism expects the reference to be unique >> within Rust, and does not allow cloning. >> >> Conceptually, this is similar to a `KBox<T>`, except that it delegates >> resource management to the `T` instead of using a generic allocator. >> >> [ om: >> - Split code into separate file and `pub use` it from types.rs. >> - Make from_raw() and into_raw() public. >> - Remove OwnableMut, and make DerefMut dependent on Unpin instead. >> - Usage example/doctest for Ownable/Owned. >> - Fixes to documentation and commit message. >> ] >> >> Link: >> https://lore.kernel.org/all/[email protected]/ >> Signed-off-by: Asahi Lina <[email protected]> >> Co-developed-by: Oliver Mangold <[email protected]> >> Signed-off-by: Oliver Mangold <[email protected]> >> Co-developed-by: Andreas Hindborg <[email protected]> >> Signed-off-by: Andreas Hindborg <[email protected]> >> Reviewed-by: Boqun Feng <[email protected]> >> --- >> rust/kernel/lib.rs | 1 + >> rust/kernel/owned.rs | 195 >> +++++++++++++++++++++++++++++++++++++++++++++++ >> rust/kernel/sync/aref.rs | 5 ++ >> rust/kernel/types.rs | 2 + >> 4 files changed, 203 insertions(+) >> >> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs >> index 3dd7bebe7888..e0ee04330dd0 100644 >> --- a/rust/kernel/lib.rs >> +++ b/rust/kernel/lib.rs >> @@ -112,6 +112,7 @@ >> pub mod of; >> #[cfg(CONFIG_PM_OPP)] >> pub mod opp; >> +pub mod owned; >> pub mod page; >> #[cfg(CONFIG_PCI)] >> pub mod pci; >> diff --git a/rust/kernel/owned.rs b/rust/kernel/owned.rs >> new file mode 100644 >> index 000000000000..a2cdd2cb8a10 >> --- /dev/null >> +++ b/rust/kernel/owned.rs >> @@ -0,0 +1,195 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> + >> +//! Unique owned pointer types for objects with custom drop logic. >> +//! >> +//! These pointer types are useful for C-allocated objects which by >> API-contract >> +//! are owned by Rust, but need to be freed through the C API. >> + >> +use core::{ >> + mem::ManuallyDrop, >> + ops::{Deref, DerefMut}, >> + pin::Pin, >> + ptr::NonNull, >> +}; >> + >> +/// Type allocated and destroyed on the C side, but owned by Rust. >> +/// >> +/// Implementing this trait allows types to be referenced via the >> [`Owned<Self>`] pointer type. This >> +/// is useful when it is desirable to tie the lifetime of the reference to >> an owned object, rather >> +/// than pass around a bare reference. [`Ownable`] types can define custom >> drop logic that is >> +/// executed when the owned reference [`Owned<Self>`] pointing to the >> object is dropped. >> +/// >> +/// Note: The underlying object is not required to provide internal >> reference counting, because it >> +/// represents a unique, owned reference. If reference counting (on the >> Rust side) is required, >> +/// [`AlwaysRefCounted`](crate::types::AlwaysRefCounted) should be >> implemented. >> +/// >> +/// # Safety >> +/// >> +/// Implementers must ensure that the [`release()`](Self::release) function >> frees the underlying >> +/// object in the correct way for a valid, owned object of this type. >> +/// >> +/// # Examples >> +/// >> +/// A minimal example implementation of [`Ownable`] and its usage with >> [`Owned`] looks like this: >> +/// >> +/// ``` >> +/// # #![expect(clippy::disallowed_names)] >> +/// # use core::cell::Cell; >> +/// # use core::ptr::NonNull; >> +/// # use kernel::sync::global_lock; >> +/// # use kernel::alloc::{flags, kbox::KBox, AllocError}; >> +/// # use kernel::types::{Owned, Ownable}; >> +/// >> +/// // Let's count the allocations to see if freeing works. >> +/// kernel::sync::global_lock! { >> +/// // SAFETY: we call `init()` right below, before doing anything else. >> +/// unsafe(uninit) static FOO_ALLOC_COUNT: Mutex<usize> = 0; >> +/// } >> +/// // SAFETY: We call `init()` only once, here. >> +/// unsafe { FOO_ALLOC_COUNT.init() }; >> +/// >> +/// struct Foo { >> +/// } > > nit: this can be simply: > > struct Foo; Got it. > >> +/// >> +/// impl Foo { >> +/// fn new() -> Result<Owned<Self>, AllocError> { >> +/// // We are just using a `KBox` here to handle the actual >> allocation, as our `Foo` is >> +/// // not actually a C-allocated object. >> +/// let result = KBox::new( >> +/// Foo {}, >> +/// flags::GFP_KERNEL, >> +/// )?; >> +/// let result = NonNull::new(KBox::into_raw(result)) >> +/// .expect("Raw pointer to newly allocation KBox is null, this >> should never happen."); >> +/// // Count new allocation >> +/// *FOO_ALLOC_COUNT.lock() += 1; >> +/// // SAFETY: We just allocated the `Self`, thus it is valid and >> there cannot be any other >> +/// // Rust references. Calling `into_raw()` makes us responsible >> for ownership and we won't >> +/// // use the raw pointer anymore. Thus we can transfer ownership >> to the `Owned`. >> +/// Ok(unsafe { Owned::from_raw(result) }) >> +/// } >> +/// } >> +/// >> +/// // SAFETY: What out `release()` function does is safe of any valid >> `Self`. >> +/// unsafe impl Ownable for Foo { >> +/// unsafe fn release(this: NonNull<Self>) { >> +/// // The `Foo` will be dropped when `KBox` goes out of scope. >> +/// // SAFETY: The [`KBox<Self>`] is still alive. We can pass >> ownership to the [`KBox`], as >> +/// // by requirement on calling this function, the `Self` will no >> longer be used by the >> +/// // caller. >> +/// unsafe { KBox::from_raw(this.as_ptr()) }; >> +/// // Count released allocation >> +/// *FOO_ALLOC_COUNT.lock() -= 1; >> +/// } >> +/// } >> +/// >> +/// { >> +/// let foo = Foo::new().expect("Failed to allocate a Foo. This >> shouldn't happen"); >> +/// assert!(*FOO_ALLOC_COUNT.lock() == 1); >> +/// } >> +/// // `foo` is out of scope now, so we expect no live allocations. >> +/// assert!(*FOO_ALLOC_COUNT.lock() == 0); >> +/// ``` >> +pub unsafe trait Ownable { >> + /// Releases the object. >> + /// >> + /// # Safety >> + /// >> + /// Callers must ensure that: >> + /// - `this` points to a valid `Self`. >> + /// - `*this` is no longer used after this call. >> + unsafe fn release(this: NonNull<Self>); >> +} >> + >> +/// An owned reference to an owned `T`. >> +/// >> +/// The [`Ownable`] is automatically freed or released when an instance of >> [`Owned`] is >> +/// dropped. >> +/// >> +/// # Invariants >> +/// >> +/// - The [`Owned<T>`] has exclusive access to the instance of `T`. >> +/// - The instance of `T` will stay alive at least as long as the >> [`Owned<T>`] is alive. >> +pub struct Owned<T: Ownable> { >> + ptr: NonNull<T>, >> +} >> + >> +// SAFETY: It is safe to send an [`Owned<T>`] to another thread when the >> underlying `T` is [`Send`], >> +// because of the ownership invariant. Sending an [`Owned<T>`] is >> equivalent to sending the `T`. >> +unsafe impl<T: Ownable + Send> Send for Owned<T> {} >> + >> +// SAFETY: It is safe to send [`&Owned<T>`] to another thread when the >> underlying `T` is [`Sync`], >> +// because of the ownership invariant. Sending an [`&Owned<T>`] is >> equivalent to sending the `&T`. >> +unsafe impl<T: Ownable + Sync> Sync for Owned<T> {} >> + >> +impl<T: Ownable> Owned<T> { > > Can you make sure that impl Owned<T> follows the struct declaration? > > IOW: please move the Send and Sync impls to be after the impl above. I don't really see the point, but moving them is no problem, so let's do that. > >> + /// Creates a new instance of [`Owned`]. >> + /// >> + /// It takes over ownership of the underlying object. >> + /// >> + /// # Safety >> + /// >> + /// Callers must ensure that: >> + /// - `ptr` points to a valid instance of `T`. >> + /// - Ownership of the underlying `T` can be transferred to the >> `Self<T>` (i.e. operations >> + /// which require ownership will be safe). >> + /// - No other Rust references to the underlying object exist. This >> implies that the underlying >> + /// object is not accessed through `ptr` anymore after the function >> call (at least until the >> + /// the `Self<T>` is dropped. > > It looks like this can be written more succinctly as: > > "This implies that the underlying object is not accessed through `ptr` > anymore until `Self<T>` is dropped." I'll rephrase with that text. > >> + /// - The C code follows the usual shared reference requirements. That >> is, the kernel will never >> + /// mutate or free the underlying object (excluding interior >> mutability that follows the usual >> + /// rules) while Rust owns it. >> + /// - In case `T` implements [`Unpin`] the previous requirement is >> extended from shared to >> + /// mutable reference requirements. That is, the kernel will not >> mutate or free the underlying >> + /// object and is okay with it being modified by Rust code. >> + pub unsafe fn from_raw(ptr: NonNull<T>) -> Self { >> + Self { >> + ptr, >> + } >> + } >> + >> + /// Consumes the [`Owned`], returning a raw pointer. >> + /// >> + /// This function does not actually relinquish ownership of the object. >> After calling this >> + /// function, the caller is responsible for ownership previously managed >> + /// by the [`Owned`]. >> + pub fn into_raw(me: Self) -> NonNull<T> { >> + ManuallyDrop::new(me).ptr >> + } >> + >> + /// Get a pinned mutable reference to the data owned by this `Owned<T>`. >> + pub fn get_pin_mut(&mut self) -> Pin<&mut T> { >> + // SAFETY: The type invariants guarantee that the object is valid, >> and that we can safely >> + // return a mutable reference to it. >> + let unpinned = unsafe { self.ptr.as_mut() }; >> + >> + // SAFETY: We never hand out unpinned mutable references to the >> data in >> + // `Self`, unless the contained type is `Unpin`. >> + unsafe { Pin::new_unchecked(unpinned) } >> + } >> +} >> + >> +impl<T: Ownable> Deref for Owned<T> { >> + type Target = T; >> + >> + fn deref(&self) -> &Self::Target { >> + // SAFETY: The type invariants guarantee that the object is valid. >> + unsafe { self.ptr.as_ref() } >> + } >> +} >> + >> +impl<T: Ownable + Unpin> DerefMut for Owned<T> { >> + fn deref_mut(&mut self) -> &mut Self::Target { >> + // SAFETY: The type invariants guarantee that the object is valid, >> and that we can safely >> + // return a mutable reference to it. >> + unsafe { self.ptr.as_mut() } >> + } >> +} >> + >> +impl<T: Ownable> Drop for Owned<T> { >> + fn drop(&mut self) { >> + // SAFETY: The type invariants guarantee that the `Owned` owns the >> object we're about to >> + // release. >> + unsafe { T::release(self.ptr) }; >> + } >> +} >> diff --git a/rust/kernel/sync/aref.rs b/rust/kernel/sync/aref.rs >> index 0d24a0432015..e175aefe8615 100644 >> --- a/rust/kernel/sync/aref.rs >> +++ b/rust/kernel/sync/aref.rs >> @@ -29,6 +29,11 @@ >> /// Rust code, the recommendation is to use [`Arc`](crate::sync::Arc) to >> create reference-counted >> /// instances of a type. >> /// >> +/// Note: Implementing this trait allows types to be wrapped in an >> [`ARef<Self>`]. It requires an >> +/// internal reference count and provides only shared references. If unique >> references are required >> +/// [`Ownable`](crate::types::Ownable) should be implemented which allows >> types to be wrapped in an >> +/// [`Owned<Self>`](crate::types::Owned). >> +/// >> /// # Safety >> /// >> /// Implementers must ensure that increments to the reference count keep the >> object alive in memory >> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs >> index dc0a02f5c3cf..7bc07c38cd6c 100644 >> --- a/rust/kernel/types.rs >> +++ b/rust/kernel/types.rs >> @@ -11,6 +11,8 @@ >> }; >> use pin_init::{PinInit, Wrapper, Zeroable}; >> >> +pub use crate::owned::{Ownable, Owned}; >> + >> pub use crate::sync::aref::{ARef, AlwaysRefCounted}; >> >> /// Used to transfer ownership to and from foreign (non-Rust) languages. >> >> -- >> 2.51.2 >> >> >> > > With the changes above, > > Reviewed-by: Daniel Almeida <[email protected]> Thanks!
