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

[Asahi Lina]

On 2/3/25 6:13 PM, Alice Ryhl wrote:
> On Sun, Feb 2, 2025 at 2:06 PM Asahi Lina <lina@xxxxxxxxxxxxx> wrote:
>>
>> 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.
>>
>> Signed-off-by: Asahi Lina <lina@xxxxxxxxxxxxx>
> 
> Overall LGTM.
> 
>> +/// A subtrait of Ownable that asserts that an `Owned<T>` Rust reference is not only unique
>> +/// within Rust and keeps the `T` alive, but also guarantees that the C code follows the
>> +/// usual mutable reference requirements. That is, the kernel will never mutate the
>> +/// `T` (excluding internal mutability that follows the usual rules) while Rust owns it.
>> +///
>> +/// When this type is implemented for an [`Ownable`] type, it allows `Owned<T>` to be
>> +/// dereferenced into a &mut T.
>> +///
>> +/// # Safety
>> +///
>> +/// Implementers must ensure that the kernel never mutates the underlying type while
>> +/// Rust owns it.
>> +pub unsafe trait OwnableMut: Ownable {}
> 
> Giving out mutable references allows users to call core::mem::swap on
> the object. We must require that this is allowed.

Hmm, yeah. I don't use this yet, and I'm not sure if it makes much sense
with that caveat. I'll drop it for v2.

> 
>> +impl<T: Ownable> Owned<T> {
>> +    /// Creates a new instance of [`Owned`].
>> +    ///
>> +    /// It takes over ownership of the underlying object.
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// Callers must ensure that the underlying object is acquired and can be considered owned by
>> +    /// Rust.
>> +    pub(crate) 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`.
>> +    #[allow(dead_code)]
>> +    pub(crate) fn into_raw(me: Self) -> NonNull<T> {
> 
> I would just make these methods public, like the ARef ones. Then you
> can drop the #[allow(dead_code)] annotation.

Does it make sense to ever have drivers doing this? I feel like these
methods should be limited to the kernel crate.

~~ Lina