rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Oliver Mangold <oliver.mangold@pm.me>
To: Andreas Hindborg <a.hindborg@kernel.org>
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>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Benno Lossin" <lossin@kernel.org>,
	"Asahi Lina" <lina+kernel@asahilina.net>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v11 1/4] rust: types: Add Ownable/Owned types
Date: Mon, 18 Aug 2025 13:04:02 +0000	[thread overview]
Message-ID: <aKMkvHAfDozzDjkB@mango> (raw)
In-Reply-To: <87o6scdchf.fsf@t14s.mail-host-address-is-not-set>

On 250818 1446, Andreas Hindborg wrote:
> "Oliver Mangold" <oliver.mangold@pm.me> writes:
> 
> > 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.
> >
> > Link: https://lore.kernel.org/all/20250202-rust-page-v1-1-e3170d7fe55e@asahilina.net/
> > Signed-off-by: Asahi Lina <lina@asahilina.net>
> > [ om:
> >   - split code into separate file and `pub use` it from types.rs
> >   - make from_raw() and into_raw() public
> >   - fixes to documentation and commit message
> > ]
> > Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
> > Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
> > ---
> >  rust/kernel/types.rs         |   7 +++
> >  rust/kernel/types/ownable.rs | 134 +++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 141 insertions(+)
> >
> > diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> > index 22985b6f69820d6df8ff3aae0bf815fad36a9d92..c12ff4d2a3f2d79b760c34c0b84a51b507d0cfb1 100644
> > --- a/rust/kernel/types.rs
> > +++ b/rust/kernel/types.rs
> > @@ -11,6 +11,9 @@
> >  };
> >  use pin_init::{PinInit, Zeroable};
> >
> > +pub mod ownable;
> > +pub use ownable::{Ownable, OwnableMut, Owned};
> > +
> >  /// Used to transfer ownership to and from foreign (non-Rust) languages.
> >  ///
> >  /// Ownership is transferred from Rust to a foreign language by calling [`Self::into_foreign`] and
> > @@ -425,6 +428,10 @@ pub const fn raw_get(this: *const Self) -> *mut T {
> >  /// 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`] should be implemented which allows types to be wrapped in an [`Owned<Self>`].
> > +///
> >  /// # Safety
> >  ///
> >  /// Implementers must ensure that increments to the reference count keep the object alive in memory
> > diff --git a/rust/kernel/types/ownable.rs b/rust/kernel/types/ownable.rs
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..f4065a0d627a62d3ecb15edabf306e9b812556e1
> > --- /dev/null
> > +++ b/rust/kernel/types/ownable.rs
> > @@ -0,0 +1,134 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! Owned reference types.
> > +
> > +use core::{
> > +    marker::PhantomData,
> > +    mem::ManuallyDrop,
> > +    ops::{Deref, DerefMut},
> > +    ptr::NonNull,
> > +};
> > +
> > +/// Types that may be owned by Rust code or borrowed, but have a lifetime managed by C code.
> > +///
> > +/// It allows such types to define their own custom destructor function to be called when a
> > +/// Rust-owned reference is dropped.
> > +///
> > +/// This is usually implemented by wrappers to existing structures on the C side of the code.
> > +///
> > +/// Note: Implementing this trait allows types to be wrapped in an [`Owned<Self>`]. This does not
> > +/// provide reference counting but represents a unique, owned reference. If reference counting is
> > +/// required [`AlwaysRefCounted`](crate::types::AlwaysRefCounted) should be implemented which allows
> > +/// types to be wrapped in an [`ARef<Self>`](crate::types::ARef).
> > +///
> > +/// # Safety
> > +///
> > +/// Implementers must ensure that:
> > +/// - The [`release()`](Ownable::release) method leaves the underlying object in a state which the
> > +///   kernel expects after ownership has been relinquished (i.e. no dangling references in the
> > +///   kernel is case it frees the object, etc.).
> > +pub unsafe trait Ownable {
> > +    /// Releases the object (frees it or returns it to foreign ownership).
> > +    ///
> > +    /// # Safety
> > +    ///
> > +    /// Callers must ensure that:
> > +    /// - `this` points to a valid `Self`.
> > +    /// - The object is no longer referenced after this call.
> > +    unsafe fn release(this: NonNull<Self>);
> > +}
> > +
> > +/// Type where [`Owned<Self>`] derefs to `&mut Self`.
> > +///
> > +/// # Safety
> > +///
> > +/// Implementers must ensure that access to a `&mut T` is safe, implying that:
> > +/// - It is safe to call [`core::mem::swap`] on the [`Ownable`]. This excludes pinned types
> > +///   (i.e. most kernel types).
> > +/// - The kernel will never access the underlying object (excluding internal mutability that follows
> > +///   the usual rules) while Rust owns it.
> > +pub unsafe trait OwnableMut: Ownable {}
> > +
> > +/// An owned reference to an ownable kernel object.
> > +///
> > +/// The object is automatically freed or released when an instance of [`Owned`] is
> > +/// dropped.
> > +///
> > +/// # Invariants
> > +///
> > +/// The pointer stored in `ptr` can be considered owned by the [`Owned`] instance.
> > +pub struct Owned<T: Ownable> {
> > +    ptr: NonNull<T>,
> > +    _p: PhantomData<T>,
> > +}
> > +
> > +// SAFETY: It is safe to send `Owned<T>` to another thread when the underlying `T` is `Send` because
> > +// it effectively means sending a `&mut T` (which is safe because `T` is `Send`).
> > +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 it effectively means sharing `&T` (which is safe because `T` is `Sync`).
> > +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:
> > +    /// - Ownership of the underlying object can be transferred to the `Owned<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 `Owned<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 [`OwnableMut`] 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 {
> > +        // INVARIANT: The safety requirements guarantee that the new instance now owns the
> > +        // reference.
> > +        Self {
> > +            ptr,
> > +            _p: PhantomData,
> > +        }
> > +    }
> > +
> > +    /// 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
> > +    }
> > +}
> > +
> > +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: OwnableMut> 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() }
> > +    }
> > +}
> 
> I think someone mentioned this before, but handing out mutable
> references can be a problem if `T: !Unpin`. For instance, we don't want
> to hand out `&mut Page` in case of `Owned<Page>`.
>

That was the reason, why `OwnableMut` was introduced in the first place.
It's clear, I guess, that as-is it cannot be implemented on many classes.

> Could we do something like this:
> 
>   diff --git a/rust/kernel/types/ownable.rs b/rust/kernel/types/ownable.rs
>   index 29729dc10cb4..52d21e41c26b 100644
>   --- a/rust/kernel/types/ownable.rs
>   +++ b/rust/kernel/types/ownable.rs
>   @@ -3,6 +3,7 @@
>   //! Owned reference types.
> 
>   use crate::{
>   +    prelude::*,
>       sync::aref::{ARef, RefCounted},
>       types::ForeignOwnable,
>   };
>   @@ -112,6 +113,17 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
>       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> {
>   @@ -123,7 +135,7 @@ fn deref(&self) -> &Self::Target {
>       }
>   }
> 
>   -impl<T: OwnableMut> DerefMut for Owned<T> {
>   +impl<T: OwnableMut + 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.
> 

Good question, I have been thinking about it, too. But it might
be, that it isn't needed at all. As I understand, usually Rust wrappers
are around non-movable C structs. Do we actually have a useful application
for OwnableMut?

Best,

Oliver


  reply	other threads:[~2025-08-18 13:04 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <OYpTDi4YYXiWvLG3nO_8_WKsgOl9KOpun9l3a34m0jza6nmEWDCLTldSwCfZ2PRRprjXqGmrgSL2JN8rPOQH8Q==@protonmail.internalid>
2025-06-18 12:27 ` [PATCH v11 0/4] New trait OwnableRefCounted for ARef<->Owned conversion Oliver Mangold
2025-06-18 12:27   ` [PATCH v11 1/4] rust: types: Add Ownable/Owned types Oliver Mangold
2025-07-02 11:03     ` Benno Lossin
2025-07-07  6:58       ` Oliver Mangold
2025-07-07  9:23         ` Benno Lossin
2025-07-08  9:56           ` Oliver Mangold
2025-07-08 10:16             ` Miguel Ojeda
2025-07-08 13:06               ` Benno Lossin
2025-07-08 18:30                 ` Andreas Hindborg
2025-07-08 19:18                   ` Benno Lossin
2025-07-09  8:53                     ` Andreas Hindborg
2025-07-09  9:11                       ` Benno Lossin
2025-07-08 13:22               ` Andreas Hindborg
2025-07-08 14:53                 ` Benno Lossin
2025-07-08 15:00             ` Benno Lossin
2025-07-07 12:26         ` Miguel Ojeda
2025-09-25  8:31       ` Oliver Mangold
2025-08-18 12:46     ` Andreas Hindborg
2025-08-18 13:04       ` Oliver Mangold [this message]
2025-08-18 22:27         ` Benno Lossin
2025-08-19  6:04           ` Oliver Mangold
2025-08-19  8:26             ` Benno Lossin
2025-08-19  8:45               ` Oliver Mangold
2025-08-19  9:00                 ` Andreas Hindborg
2025-08-19 17:15                   ` Benno Lossin
2025-08-20 10:48                     ` Andreas Hindborg
2025-08-19  8:53               ` Andreas Hindborg
2025-08-19 17:13                 ` Benno Lossin
2025-08-19 18:28                   ` Andreas Hindborg
2025-08-20  6:02                   ` Oliver Mangold
2025-08-20  7:41                     ` Benno Lossin
2025-08-20  7:43                       ` Oliver Mangold
2025-08-20 10:51                         ` Andreas Hindborg
2025-06-18 12:27   ` [PATCH v11 2/4] rust: Split `AlwaysRefCounted` into two traits Oliver Mangold
2025-06-19  3:15     ` kernel test robot
2025-07-02 11:23     ` Benno Lossin
2025-07-07  7:42       ` Oliver Mangold
2025-07-07  9:27         ` Benno Lossin
2025-06-18 12:27   ` [PATCH v11 3/4] rust: Add missing SAFETY documentation for `ARef` example Oliver Mangold
2025-06-18 12:27   ` [PATCH v11 4/4] rust: Add `OwnableRefCounted` Oliver Mangold
2025-07-02 13:24     ` Benno Lossin
2025-07-07  8:07       ` Oliver Mangold
2025-07-07  9:33         ` Benno Lossin
2025-07-07 11:12           ` Andreas Hindborg
2025-07-07 11:47             ` Benno Lossin
2025-07-07 13:21               ` Andreas Hindborg
2025-07-07 15:39                 ` Benno Lossin
2025-07-08 13:15                   ` Andreas Hindborg
2025-07-08 14:50                     ` Benno Lossin
2025-07-08 15:35                       ` Andreas Hindborg
2025-07-08  9:36           ` Oliver Mangold
2025-07-08 13:42             ` Benno Lossin
2025-08-05 17:23   ` [PATCH v11 0/4] New trait OwnableRefCounted for ARef<->Owned conversion Danilo Krummrich
2025-08-06  5:56     ` Oliver Mangold
2025-08-15 10:12   ` Andreas Hindborg
2025-08-18  5:59     ` Oliver Mangold

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=aKMkvHAfDozzDjkB@mango \
    --to=oliver.mangold@pm.me \
    --cc=a.hindborg@kernel.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=gary@garyguo.net \
    --cc=lina+kernel@asahilina.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    /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;
as well as URLs for NNTP newsgroup(s).