Re: [PATCH v18 3/8] rust: implement `ForeignOwnable` for `Owned`

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

"Gary Guo" <gary@garyguo.net> writes:

> On Thu Jun 25, 2026 at 11:15 AM BST, Andreas Hindborg wrote:
>> Implement `ForeignOwnable` for `Owned<T>`. This allows use of `Owned<T>` in
>> places such as the `XArray`.
>>
>> Note that `T` does not need to implement `ForeignOwnable` for `Owned<T>` to
>> implement `ForeignOwnable`.
>>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> ---
>>  rust/kernel/owned.rs | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 53 insertions(+)
>>
>> diff --git a/rust/kernel/owned.rs b/rust/kernel/owned.rs
>> index 7fe9ec3e55126..9c92d4a83cc1b 100644
>> --- a/rust/kernel/owned.rs
>> +++ b/rust/kernel/owned.rs
>> @@ -15,6 +15,8 @@
>>      ptr::NonNull, //
>>  };
>>  
>> +use kernel::types::ForeignOwnable;
>> +
>>  /// Types that specify their own way of performing allocation and destruction. Typically, this trait
>>  /// is implemented on types from the C side.
>>  ///
>> @@ -186,3 +188,54 @@ fn drop(&mut self) {
>>          unsafe { T::release(self.ptr) };
>>      }
>>  }
>> +
>> +// SAFETY: We derive the pointer to `T` from a valid `T`, so the returned
>> +// pointer satisfy alignment requirements of `T`.
>> +unsafe impl<T: Ownable> ForeignOwnable for Owned<T> {
>> +    const FOREIGN_ALIGN: usize = core::mem::align_of::<T>();
>> +
>> +    type Borrowed<'a>
>> +        = &'a T
>> +    where
>> +        Self: 'a;
>> +    type BorrowedMut<'a>
>> +        = Pin<&'a mut T>
>> +    where
>> +        Self: 'a;
>> +
>> +    #[inline]
>> +    fn into_foreign(self) -> *mut kernel::ffi::c_void {
>> +        let ptr = self.ptr.as_ptr().cast();
>> +        core::mem::forget(self);
>> +        ptr
>
> I think the pattern in `into_raw` is better:
>
>     ManuallyDrop::new(self).ptr.as_ptr().cast()
>
> Or perhaps this can just use `Self::into_raw(self).as_ptr().cast()`.
>
>> +    }
>> +
>> +    #[inline]
>> +    unsafe fn from_foreign(ptr: *mut kernel::ffi::c_void) -> Self {
>> +        // INVARIANT: By the function safety contract, `ptr` was returned by `into_foreign`, which
>> +        // gave up exclusive ownership of a valid, pinned `T`; we retake that ownership here.
>> +        Self {
>> +            // SAFETY: By function safety contract, `ptr` came from
>> +            // `into_foreign` and cannot be null.
>> +            ptr: unsafe { NonNull::new_unchecked(ptr.cast()) },
>> +        }
>> +    }
>
> Same here, could be using `Self::from_raw`.
>
> However, the current code looks correct to me regardless, so:
>
> Reviewed-by: Gary Guo <gary@garyguo.net>

I'll defer to `{into,from}_raw`. Keeping your tag.


Best regards,
Andreas Hindborg