Re: [PATCH v21 6/9] rust: Add `OwnableRefCounted`

[Andreas Hindborg <a.hindborg@kernel.org>]

"Alice Ryhl" <aliceryhl@google.com> writes:

> On Thu, Sep 10, 2026 at 11:00:10AM +0200, Andreas Hindborg wrote:
>> From: Oliver Mangold <oliver.mangold@pm.me>
>>
>> Types implementing one of these traits can safely convert between an
>> `ARef<T>` and an `Owned<T>`.
>>
>> This is useful for types which generally are accessed through an `ARef`
>> but have methods which can only safely be called when the reference is
>> unique, like e.g. `block::mq::Request::end_ok()`.
>>
>> Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
>> [ Andreas: Fix formatting, update documentation, fix error handling in
>>   examples. ]
>> Assisted-by: LLM
>> Co-developed-by: Andreas Hindborg <a.hindborg@kernel.org>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
>>  /// 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.
>> +/// 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 an object 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
>> +///    of type [`Owned<_>`] pointing to the object is dropped.
>
> This diff looks like it should be in the patch introducing Ownable.

Yes, wrong target commit for the edit.

>
>> +/// # #![expect(clippy::disallowed_names)]
>> +/// # use core::ptr::NonNull;
>> +/// # use kernel::alloc::{flags, kbox::KBox, AllocError};
>> +/// # use kernel::sync::aref::{ARef, RefCounted};
>> +/// # use kernel::sync::atomic::Acquire;
>> +/// # use kernel::sync::Refcount;
>> +/// # use kernel::types::{Owned, Ownable, OwnableRefCounted};
>> +///
>
> This newline will appear weirdly in generated docs because no content is
> shown above it. Consider not hiding the imports.

Will remove.

>
>> +pub trait OwnableRefCounted: RefCounted + Ownable + Sized {
>> +    /// Checks if the [`ARef`] is unique and converts it to an [`Owned`] if that is the case.
>> +    /// Otherwise it returns again an [`ARef`] to the same underlying object.
>> +    fn try_from_shared(this: ARef<Self>) -> Result<Owned<Self>, ARef<Self>>;
>> +
>> +    /// Converts the [`Owned`] into an [`ARef`].
>> +    fn into_shared(this: Owned<Self>) -> ARef<Self>;
>> +}
>
> This trait provides two methods that have very different conditions for
> existing, IMO.
>
> The into_shared() method is so likely to exist, that I would almost
> consider placing it on the Ownable trait.
>
> pub trait Ownable {
>     unsafe fn release(this: NonNull<Self>);
>
>     fn into_shared(this: Owned<Self>) -> ARef<Self>
>     where
>     	Self: Refcounted;
> }
>
> On the other hand, I think try_from_shared() is more likely to be a
> tricky operation you cannot necessarily implement, and as such it can be
> on its own separate trait like you have here.

Ownable does not require RefCounted, so this method is not always
implementable.

>
> At the very least, I can easily imagine that some types can never become
> unique again after having been shared.

It could make sense to split the capability in a separate trait. But
perhaps we should wait with that till we need it and not introduce
premature complexity?

Best regards,
Andreas Hindborg