linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Alice Ryhl <aliceryhl@google.com>
To: Andreas Hindborg <a.hindborg@kernel.org>
Cc: "Miguel Ojeda" <ojeda@kernel.org>, "Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Trevor Gross" <tmgross@umich.edu>,
	"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>,
	"Paul Moore" <paul@paul-moore.com>,
	"Serge Hallyn" <sergeh@kernel.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"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>,
	"Igor Korotin" <igor.korotin.linux@gmail.com>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"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>,
	"Boqun Feng" <boqun@kernel.org>,
	linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
	linux-block@vger.kernel.org,
	linux-security-module@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,
	"Asahi Lina" <lina+kernel@asahilina.net>,
	"Oliver Mangold" <oliver.mangold@pm.me>
Subject: Re: [PATCH v15 1/9] rust: types: Add Ownable/Owned types
Date: Fri, 20 Feb 2026 10:35:12 +0000	[thread overview]
Message-ID: <aZg44EmMWKK-z5KP@google.com> (raw)
In-Reply-To: <20260220-unique-ref-v15-1-893ed86b06cc@kernel.org>

On Fri, Feb 20, 2026 at 10:51:10AM +0100, Andreas Hindborg 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>
> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
> [ Andreas: Updated documentation, examples, and formatting ]
> Reviewed-by: Gary Guo <gary@garyguo.net>
> Co-developed-by: Andreas Hindborg <a.hindborg@kernel.org>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

> +///         let result = NonNull::new(KBox::into_raw(result))
> +///             .expect("Raw pointer to newly allocation KBox is null, this should never happen.");

KBox should probably have an into_raw_nonnull().

> +///    let foo = Foo::new().expect("Failed to allocate a Foo. This shouldn't happen");
> +///    assert!(*FOO_ALLOC_COUNT.lock() == 1);

Use ? here.

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

Honestly, not using it after this call may be too strong. I can imagine
wanting a value where I have both an ARef<_> and Owned<_> reference to
something similar to the existing Arc<_>/ListArc<_> pattern, and in that
case the value may in fact be accessed after this call if you still have
an ARef<_>.

If you modify Owned<_> invariants and Owned::from_raw() safety
requirements along the lines of what I say below, then this could just
say that the caller must have permission to call this function. The
concrete implementer can specify what that means more directly, but here
all it means is that a prior call to Owned::from_raw() promised to give
you permission to call it.

> +/// A mutable 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>,
> +}

I think some more direct and less fuzzy invariants would be:

- This `Owned<T>` holds permissions to call `T::release()` on the value once.
- Until `T::release()` is called, this `Owned<T>` may perform mutable access on the `T`.
- The `T` value is pinned.

> +    /// Get a pinned mutable reference to the data owned by this `Owned<T>`.
> +    pub fn as_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) }

I'd prefer if "pinned" was a type invariant, rather than make an
argument about what kind of APIs exist.

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

Surely this safety comment should say something about pinning.

Alice


  reply	other threads:[~2026-02-20 10:35 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-20  9:51 [PATCH v15 0/9] rust: add `Ownable` trait and `Owned` type Andreas Hindborg
2026-02-20  9:51 ` [PATCH v15 1/9] rust: types: Add Ownable/Owned types Andreas Hindborg
2026-02-20 10:35   ` Alice Ryhl [this message]
2026-02-20  9:51 ` [PATCH v15 2/9] rust: rename `AlwaysRefCounted` to `RefCounted` Andreas Hindborg
2026-02-20  9:51 ` [PATCH v15 3/9] rust: Add missing SAFETY documentation for `ARef` example Andreas Hindborg
2026-02-20 10:49   ` Alice Ryhl
2026-02-20  9:51 ` [PATCH v15 4/9] rust: aref: update formatting of use statements Andreas Hindborg
2026-02-20  9:51 ` [PATCH v15 5/9] rust: Add `OwnableRefCounted` Andreas Hindborg
2026-02-20  9:51 ` [PATCH v15 6/9] rust: page: update formatting of `use` statements Andreas Hindborg
2026-02-20  9:51 ` [PATCH v15 7/9] rust: page: convert to `Ownable` Andreas Hindborg
2026-02-20  9:51 ` [PATCH v15 8/9] rust: implement `ForeignOwnable` for `Owned` Andreas Hindborg
2026-02-20  9:51 ` [PATCH v15 9/9] rust: page: add `from_raw()` Andreas Hindborg
2026-02-20 10:49   ` Alice Ryhl
2026-02-20 17:33   ` Miguel Ojeda
2026-02-20 17:50     ` Tamir Duberstein

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=aZg44EmMWKK-z5KP@google.com \
    --to=aliceryhl@google.com \
    --cc=Liam.Howlett@oracle.com \
    --cc=a.hindborg@kernel.org \
    --cc=airlied@gmail.com \
    --cc=bhelgaas@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=brauner@kernel.org \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=david.m.ertman@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=igor.korotin.linux@gmail.com \
    --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=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=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