Re: [PATCH v15 1/9] rust: types: Add Ownable/Owned types

[<aliceryhl@google.com>]

On Mon, Feb 23, 2026 at 03:59:22PM +0100, Andreas Hindborg wrote:
> Alice Ryhl <aliceryhl@google.com> writes:
> 
> > 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().
> 
> I can add that.
> 
> >
> >> +///    let foo = Foo::new().expect("Failed to allocate a Foo. This shouldn't happen");
> >> +///    assert!(*FOO_ALLOC_COUNT.lock() == 1);
> >
> > Use ? here.
> 
> Ok.
> 
> >
> >> +/// }
> >> +/// // `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<_>.
> 
> I do not understand your use case.
> 
> You are not supposed to have both an `ARef` and an `Owned` at the same
> time. The `Owned` is to `ARef` what `UniqueArc` is to `Arc`. It is
> supposed to be unique and no `ARef` can be live while the `Owned` is
> live.
> 
> A `ListArc` is "at most one per list link" and it takes a refcount on
> the object by owning an `Arc`. As far as I recall, it does not provide
> mutable access to anything but the list link. To me, that is a very
> different situation.

I mean, even Page is kind of an example like that.

Pages are refcounted, but when you have a higher-order page, the
__free_pages() call does something special beyond what put_page(). For
example, if you have an order-2 page, which consists of 4 pages, then
the refcount only keeps the first page alive, and __free_pages() frees
the 3 extra pages right away even if refcount is still non-zero. The
first page then stays alive until the last put_page() is called.

> > 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.
> 
> I don't think we need the "permission" wording. How about this:
> 
> 
> /// A mutable reference to an owned `T`.
> ///
> /// The [`Ownable`] is automatically freed or released when an instance of [`Owned`] is
> /// dropped.
> ///
> /// # Invariants
> ///
> /// - Until `T::release` is called, this `Owned<T>` exclusively owns the underlying `T`.
> /// - The `T` value is pinned.
> pub struct Owned<T: Ownable> {...}
> 
> 
> impl<T: Ownable> Owned<T> {
>     /// Creates a new instance of [`Owned`].
>     ///
>     /// This function takes over ownership of the underlying object.
>     ///
>     /// # Safety
>     ///
>     /// Callers must ensure that:
>     /// - `ptr` points to a valid instance of `T`.
>     /// - Until `T::release` is called, the returned `Owned<T>` exclusively owns the underlying `T`.
>     pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {...}
> }
> 
> pub trait Ownable {
>     /// Tear down this `Ownable`.
>     ///
>     /// Implementers of `Ownable` can use this function to clean up the use of `Self`. This can
>     /// include freeing the underlying object.
>     ///
>     /// # Safety
>     ///
>     /// Callers must ensure that the caller has exclusive ownership of `T`, and this ownership can
>     /// be transferred to the `release` method.
>     unsafe fn release(&mut self);
> }
> 
> 
> Note `Ownable` not being an unsafe trait.

It looks ok but see my above reply.

> >> +/// 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`.
> 
> I do not like the wording for mutable access. Formulating safety
> requirements for `from_raw` and safety comments for that function
> becomes convoluted like this. I'd rather formulate the
> access capability in terms of ownership;
> 
>  - Until `T::release()` is called, this `Owned<T>` exclusively owns the
>    underlying `T`.
> 
> How is that?
> 
> > - The `T` value is pinned.
> 
> I am unsure about the pinning terminology. If we say that `T` is pinned,
> does this mean that it will never move, even if `T: Unpin`? Or is it
> implied that `T` may move if it is `Unpin`?

Values that are `Unpin` can always move - pinning is a no-op for them.

Alice