Linux PCI subsystem development
 help / color / mirror / Atom feed
From: Daniel Almeida <daniel.almeida@collabora.com>
To: Oliver Mangold <oliver.mangold@pm.me>
Cc: "Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Benno Lossin" <lossin@kernel.org>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Dave Ertman" <david.m.ertman@intel.com>,
	"Ira Weiny" <ira.weiny@intel.com>,
	"Leon Romanovsky" <leon@kernel.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Alexander Viro" <viro@zeniv.linux.org.uk>,
	"Christian Brauner" <brauner@kernel.org>,
	"Jan Kara" <jack@suse.cz>,
	"Lorenzo Stoakes" <lorenzo.stoakes@oracle.com>,
	"Liam R. Howlett" <Liam.Howlett@oracle.com>,
	"Viresh Kumar" <vireshk@kernel.org>, "Nishanth Menon" <nm@ti.com>,
	"Stephen Boyd" <sboyd@kernel.org>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Krzysztof Wilczyński" <kwilczynski@kernel.org>,
	"Paul Moore" <paul@paul-moore.com>,
	"Serge Hallyn" <sergeh@kernel.org>,
	"Asahi Lina" <lina+kernel@asahilina.net>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-block@vger.kernel.org, dri-devel@lists.freedesktop.org,
	linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
	linux-pm@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-security-module@vger.kernel.org
Subject: Re: [PATCH v13 1/4] rust: types: Add Ownable/Owned types
Date: Fri, 28 Nov 2025 12:09:38 -0300	[thread overview]
Message-ID: <C95B13F7-B3F5-4508-A862-EAD22EF56FE2@collabora.com> (raw)
In-Reply-To: <20251117-unique-ref-v13-1-b5b243df1250@pm.me>

Hi Oliver,

> On 17 Nov 2025, at 07:07, 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.
> +///
> +/// 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;

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

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

> +    /// - 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 <daniel.almeida@collabora.com>

  reply	other threads:[~2025-11-28 15:11 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-17 10:07 [PATCH v13 0/4] New trait OwnableRefCounted for ARef<->Owned conversion Oliver Mangold
2025-11-17 10:07 ` [PATCH v13 1/4] rust: types: Add Ownable/Owned types Oliver Mangold
2025-11-28 15:09   ` Daniel Almeida [this message]
2026-02-02  9:14     ` Andreas Hindborg
2025-12-01 15:51   ` Gary Guo
2026-02-02  9:37     ` Andreas Hindborg
2026-02-02 12:29       ` Gary Guo
2026-02-02 13:04         ` Andreas Hindborg
2025-11-17 10:07 ` [PATCH v13 2/4] rust: `AlwaysRefCounted` is renamed to `RefCounted` Oliver Mangold
2025-11-28 17:46   ` Daniel Almeida
2026-02-02  9:46     ` Andreas Hindborg
2025-12-01 16:00   ` Gary Guo
2026-02-02  9:48     ` Andreas Hindborg
2025-11-17 10:08 ` [PATCH v13 3/4] rust: Add missing SAFETY documentation for `ARef` example Oliver Mangold
2025-11-28 17:50   ` Daniel Almeida
2026-02-02  9:52     ` Andreas Hindborg
2025-11-17 10:08 ` [PATCH v13 4/4] rust: Add `OwnableRefCounted` Oliver Mangold
2025-11-28 18:06   ` Daniel Almeida
2025-12-01 10:23     ` Oliver Mangold
2025-12-01 17:09       ` Miguel Ojeda
2026-02-02 10:06     ` Andreas Hindborg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=C95B13F7-B3F5-4508-A862-EAD22EF56FE2@collabora.com \
    --to=daniel.almeida@collabora.com \
    --cc=Liam.Howlett@oracle.com \
    --cc=a.hindborg@kernel.org \
    --cc=airlied@gmail.com \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=bhelgaas@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=brauner@kernel.org \
    --cc=dakr@kernel.org \
    --cc=david.m.ertman@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=ira.weiny@intel.com \
    --cc=jack@suse.cz \
    --cc=kwilczynski@kernel.org \
    --cc=leon@kernel.org \
    --cc=lina+kernel@asahilina.net \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=lossin@kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=nm@ti.com \
    --cc=ojeda@kernel.org \
    --cc=oliver.mangold@pm.me \
    --cc=paul@paul-moore.com \
    --cc=rafael@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=sboyd@kernel.org \
    --cc=sergeh@kernel.org \
    --cc=simona@ffwll.ch \
    --cc=tmgross@umich.edu \
    --cc=tzimmermann@suse.de \
    --cc=vireshk@kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox