Re: [PATCH v13 1/4] rust: types: Add Ownable/Owned types

["Gary Guo" <gary@garyguo.net>]

On Mon Feb 2, 2026 at 9:37 AM GMT, Andreas Hindborg wrote:
> Gary Guo <gary@garyguo.net> writes:
>
>> On Mon, 17 Nov 2025 10:07:40 +0000
>> Oliver Mangold <oliver.mangold@pm.me> wrote:
>>
>>> From: Asahi Lina <lina+kernel@asahilina.net>
>>> 
>>> 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/20250202-rust-page-v1-1-e3170d7fe55e@asahilina.net/
>>> Signed-off-by: Asahi Lina <lina+kernel@asahilina.net>
>>> Co-developed-by: Oliver Mangold <oliver.mangold@pm.me>
>>> Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
>>> Co-developed-by: Andreas Hindborg <a.hindborg@kernel.org>
>>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>>> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
>>> ---
>>>  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.
>>
>> The example given in the documentation below shows a valid way of
>> defining a type that's handled on the Rust side, so I think this
>> message is somewhat inaccurate.
>>
>> Perhaps something like
>>
>> 	Types that specify their own way of performing allocation and
>> 	destruction. Typically, this trait is implemented on types from
>> 	the C side.
>
> Thanks, I'll use this.
>
>>
>> ?
>>
>>> +///
>>> +/// 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 {
>>> +/// }
>>> +///
>>> +/// 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`.
>>
>> I can't parse this sentence. Is "out" supposed to be a different word?
>
> I think it should be "our".
>
>>
>>> +/// unsafe impl Ownable for Foo {
>>> +///     unsafe fn release(this: NonNull<Self>) {
>>> +///         // The `Foo` will be dropped when `KBox` goes out of scope.
>>
>> I would just write `drop(unsafe { ... })` to make drop explicit instead
>> of commenting about the implicit drop.
>
> Agree, that is easier to read.
>
>>
>>> +///         // 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> {
>>> +    /// 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.
>>
>> Is this correct? If `Self<T>` is dropped then `T::release` is called so
>> the pointer should also not be accessed further?
>
> I can't follow you point here. Are you saying that the requirement is
> wrong because `T::release` will access the object by reference? If so,
> that is part of `Owned<_>::drop`, which is explicitly mentioned in the
> comment (until .. dropped).

I meant that the `Self<T>` is dropped, the object is destroyed so it should also
not be accessed further. Perhaps just remove the "(at least ...)" part from
comment.

>
>>
>>> +    /// - 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.
>>
>> The concept "interior mutability" doesn't really exist on the C side.
>> Also, use of interior mutability (by UnsafeCell) would be incorrect if
>> the type is implemented in the rust side (as this requires a
>> UnsafePinned).
>>
>> Interior mutability means things can be mutated behind a shared
>> reference -- however in this case, we have a mutable reference (either
>> `Pin<&mut Self>` or `&mut Self`)!
>>
>> Perhaps together with the next line, they could be just phrased like
>> this?
>>
>> - The underlying object must not be accessed (read or mutated) through
>>   any pointer other than the created `Owned<T>`.
>>   Opt-out is still possbile similar to a mutable reference (e.g. by
>>   using p`Opaque`]). 
>>
>> I think we should just tell the user "this is just a unique reference
>> similar to &mut". They should be able to deduce that all the `!Unpin`
>> that opts out from uniqueness of mutable reference applies here too.
>
> I agree. I would suggest updating the struct documentation:
>
>     @@ -108,7 +108,7 @@ pub unsafe trait Ownable {
>         unsafe fn release(this: NonNull<Self>);
>     }
>
>     -/// An owned reference to an owned `T`.
>     +/// An mutable reference to an owned `T`.
>     ///
>     /// The [`Ownable`] is automatically freed or released when an instance of [`Owned`] is
>     /// dropped.
>
> And then the safety requirement as
>
>  An `Owned<T>` is a mutable reference to the underlying object. As such,
>  the object must not be accessed (read or mutated) through any pointer
>  other than the created `Owned<T>`. Opt-out is still possbile similar to
>  a mutable reference (e.g. by using [`Opaque`]).

Sounds good to me.

>
>
>>> +    /// - 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.
>>
>> - If `T` implements [`Unpin`], the structure must not be mutated for
>>   the entire lifetime of `Owned<T>`.
>
> Would it be OK to just write "If `T: Unpin`, the ..."?
>
> Again, opt out is possible, right?
>

When the "mutable reference" framing above I think you can just drop this part.

>>
>>> +    pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
>>
>> This needs a (rather trivial) INVARIANT comment.
>
> OK.
>
>>
>>> +        Self {
>>> +            ptr,
>>> +        }
>>> +    }
>>> +
>>> +    /// Consumes the [`Owned`], returning a raw pointer.
>>> +    ///
>>> +    /// This function does not actually relinquish ownership of the object. After calling this
>>
>> Perhaps "relinquish" isn't the best word here? In my mental model
>> this function is pretty much relinquishing ownership as `Owned<T>` no
>> longer exists. It just doesn't release the object.
>
> How about this:
>
>
>     /// Consumes the [`Owned`], returning a raw pointer.
>     ///
>     /// This function does not drop the underlying `T`. When this function returns, ownership of the
>     /// underlying `T` is with the caller.

SGTM.

>
>
> Thanks for the comments!

Best,
Gary

>
>
> Best regards,
> Andreas Hindborg