Message-ID: <20251201155135.2b9c4084.gary@garyguo.net>
Date: Mon, 1 Dec 2025 15:51:35 +0000
From: Gary Guo <gary@...yguo.net>
To: Oliver Mangold <oliver.mangold@...me>
Cc: Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>,
 Boqun Feng <boqun.feng@...il.com>, Björn Roy Baron
 <bjorn3_gh@...tonmail.com>, Andreas Hindborg <a.hindborg@...nel.org>, Alice
 Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>, Benno Lossin
 <lossin@...nel.org>, Danilo Krummrich <dakr@...nel.org>, Greg Kroah-Hartman
 <gregkh@...uxfoundation.org>, Dave Ertman <david.m.ertman@...el.com>, Ira
 Weiny <ira.weiny@...el.com>, Leon Romanovsky <leon@...nel.org>, "Rafael J.
 Wysocki" <rafael@...nel.org>, Maarten Lankhorst
 <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
 Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>,
 Simona Vetter <simona@...ll.ch>, Alexander Viro <viro@...iv.linux.org.uk>,
 Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>, Lorenzo
 Stoakes <lorenzo.stoakes@...cle.com>, "Liam R. Howlett"
 <Liam.Howlett@...cle.com>, Viresh Kumar <vireshk@...nel.org>, Nishanth
 Menon <nm@...com>, Stephen Boyd <sboyd@...nel.org>, Bjorn Helgaas
 <bhelgaas@...gle.com>, Krzysztof Wilczyński
 <kwilczynski@...nel.org>, Paul Moore <paul@...l-moore.com>, Serge Hallyn
 <sergeh@...nel.org>, Asahi Lina <lina+kernel@...hilina.net>,
 rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-block@...r.kernel.org, dri-devel@...ts.freedesktop.org,
 linux-fsdevel@...r.kernel.org, linux-mm@...ck.org,
 linux-pm@...r.kernel.org, linux-pci@...r.kernel.org,
 linux-security-module@...r.kernel.org
Subject: Re: [PATCH v13 1/4] rust: types: Add Ownable/Owned types

On Mon, 17 Nov 2025 10:07:40 +0000
Oliver Mangold <oliver.mangold@...me> wrote:

> From: Asahi Lina <lina+kernel@...hilina.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@...hilina.net>
> Co-developed-by: Oliver Mangold <oliver.mangold@...me>
> Signed-off-by: Oliver Mangold <oliver.mangold@...me>
> Co-developed-by: Andreas Hindborg <a.hindborg@...nel.org>
> Signed-off-by: Andreas Hindborg <a.hindborg@...nel.org>
> Reviewed-by: Boqun Feng <boqun.feng@...il.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.

?

> +///
> +/// 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?

> +/// 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.

> +///         // 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?

> +    /// - 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.


> +    /// - 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>`.

> +    pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {

This needs a (rather trivial) INVARIANT comment.

> +        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.

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

Best,
Gary

Powered by blists - more mailing lists