[PATCH v4 4/5] rust: types: ForeignOwnable: Add type Target

Danilo Krummrich posted 5 patches 3 months, 1 week ago
[PATCH v4 4/5] rust: types: ForeignOwnable: Add type Target
Posted by Danilo Krummrich 3 months, 1 week ago
ForeignOwnable::Target defines the payload data of a ForeignOwnable. For
Arc<T> for instance, ForeignOwnable::Target would just be T.

This is useful for cases where a trait bound is required on the target
type of the ForeignOwnable. For instance:

	fn example<P>(data: P)
	   where
	      P: ForeignOwnable,
	      P::Target: MyTrait,
	{}

Suggested-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/alloc/kbox.rs | 2 ++
 rust/kernel/sync/arc.rs   | 1 +
 rust/kernel/types.rs      | 4 ++++
 3 files changed, 7 insertions(+)

diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs
index c386ff771d50..66fad9777567 100644
--- a/rust/kernel/alloc/kbox.rs
+++ b/rust/kernel/alloc/kbox.rs
@@ -403,6 +403,7 @@ unsafe impl<T: 'static, A> ForeignOwnable for Box<T, A>
 where
     A: Allocator,
 {
+    type Target = T;
     type PointedTo = T;
     type Borrowed<'a> = &'a T;
     type BorrowedMut<'a> = &'a mut T;
@@ -435,6 +436,7 @@ unsafe impl<T: 'static, A> ForeignOwnable for Pin<Box<T, A>>
 where
     A: Allocator,
 {
+    type Target = T;
     type PointedTo = T;
     type Borrowed<'a> = Pin<&'a T>;
     type BorrowedMut<'a> = Pin<&'a mut T>;
diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index c7af0aa48a0a..24fb63597d35 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -374,6 +374,7 @@ pub fn into_unique_or_drop(self) -> Option<Pin<UniqueArc<T>>> {
 
 // SAFETY: The `into_foreign` function returns a pointer that is well-aligned.
 unsafe impl<T: 'static> ForeignOwnable for Arc<T> {
+    type Target = T;
     type PointedTo = ArcInner<T>;
     type Borrowed<'a> = ArcBorrow<'a, T>;
     type BorrowedMut<'a> = Self::Borrowed<'a>;
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 3958a5f44d56..74c787b352a9 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -27,6 +27,9 @@
 /// [`into_foreign`]: Self::into_foreign
 /// [`PointedTo`]: Self::PointedTo
 pub unsafe trait ForeignOwnable: Sized {
+    /// The payload type of the foreign-owned value.
+    type Target;
+
     /// Type used when the value is foreign-owned. In practical terms only defines the alignment of
     /// the pointer.
     type PointedTo;
@@ -128,6 +131,7 @@ unsafe fn try_from_foreign(ptr: *mut Self::PointedTo) -> Option<Self> {
 
 // SAFETY: The `into_foreign` function returns a pointer that is dangling, but well-aligned.
 unsafe impl ForeignOwnable for () {
+    type Target = ();
     type PointedTo = ();
     type Borrowed<'a> = ();
     type BorrowedMut<'a> = ();
-- 
2.49.0
Re: [PATCH v4 4/5] rust: types: ForeignOwnable: Add type Target
Posted by Alice Ryhl 3 months, 1 week ago
On Thu, Jun 26, 2025 at 10:01 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> ForeignOwnable::Target defines the payload data of a ForeignOwnable. For
> Arc<T> for instance, ForeignOwnable::Target would just be T.
>
> This is useful for cases where a trait bound is required on the target
> type of the ForeignOwnable. For instance:
>
>         fn example<P>(data: P)
>            where
>               P: ForeignOwnable,
>               P::Target: MyTrait,
>         {}
>
> Suggested-by: Benno Lossin <lossin@kernel.org>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

I am skeptical about this patch. I think that most of the time, you
should just place the trait bound on `P` instead of having a Target
type like this.

Alice
Re: [PATCH v4 4/5] rust: types: ForeignOwnable: Add type Target
Posted by Miguel Ojeda 3 months, 1 week ago
On Thu, Jun 26, 2025 at 10:01 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> ForeignOwnable::Target defines the payload data of a ForeignOwnable. For
> Arc<T> for instance, ForeignOwnable::Target would just be T.
>
> This is useful for cases where a trait bound is required on the target
> type of the ForeignOwnable. For instance:
>
>         fn example<P>(data: P)
>            where
>               P: ForeignOwnable,
>               P::Target: MyTrait,
>         {}
>
> Suggested-by: Benno Lossin <lossin@kernel.org>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Acked-by: Miguel Ojeda <ojeda@kernel.org>

Cheers,
Miguel
Re: [PATCH v4 4/5] rust: types: ForeignOwnable: Add type Target
Posted by Benno Lossin 3 months, 1 week ago
On Thu Jun 26, 2025 at 10:00 PM CEST, Danilo Krummrich wrote:
> ForeignOwnable::Target defines the payload data of a ForeignOwnable. For
> Arc<T> for instance, ForeignOwnable::Target would just be T.
>
> This is useful for cases where a trait bound is required on the target
> type of the ForeignOwnable. For instance:
>
> 	fn example<P>(data: P)
> 	   where
> 	      P: ForeignOwnable,
> 	      P::Target: MyTrait,
> 	{}
>
> Suggested-by: Benno Lossin <lossin@kernel.org>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Reviewed-by: Benno Lossin <lossin@kernel.org>

We might also be able to add a `Deref<Target = Self::Target>` bound on
`Borrowed` and `BorrowedMut`, but we should only do that when necessary.

---
Cheers,
Benno

> ---
>  rust/kernel/alloc/kbox.rs | 2 ++
>  rust/kernel/sync/arc.rs   | 1 +
>  rust/kernel/types.rs      | 4 ++++
>  3 files changed, 7 insertions(+)
Re: [PATCH v4 4/5] rust: types: ForeignOwnable: Add type Target
Posted by Boqun Feng 3 months, 1 week ago
On Thu, Jun 26, 2025 at 10:00:42PM +0200, Danilo Krummrich wrote:
> ForeignOwnable::Target defines the payload data of a ForeignOwnable. For
> Arc<T> for instance, ForeignOwnable::Target would just be T.
> 
> This is useful for cases where a trait bound is required on the target
> type of the ForeignOwnable. For instance:
> 
> 	fn example<P>(data: P)
> 	   where
> 	      P: ForeignOwnable,
> 	      P::Target: MyTrait,
> 	{}
> 
> Suggested-by: Benno Lossin <lossin@kernel.org>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Reviewed-by: Boqun Feng <boqun.feng@gmail.com>

One nit below:

> ---
>  rust/kernel/alloc/kbox.rs | 2 ++
>  rust/kernel/sync/arc.rs   | 1 +
>  rust/kernel/types.rs      | 4 ++++
>  3 files changed, 7 insertions(+)
> 
> diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs
> index c386ff771d50..66fad9777567 100644
> --- a/rust/kernel/alloc/kbox.rs
> +++ b/rust/kernel/alloc/kbox.rs
> @@ -403,6 +403,7 @@ unsafe impl<T: 'static, A> ForeignOwnable for Box<T, A>
>  where
>      A: Allocator,
>  {
> +    type Target = T;
>      type PointedTo = T;
>      type Borrowed<'a> = &'a T;
>      type BorrowedMut<'a> = &'a mut T;
> @@ -435,6 +436,7 @@ unsafe impl<T: 'static, A> ForeignOwnable for Pin<Box<T, A>>
>  where
>      A: Allocator,
>  {
> +    type Target = T;
>      type PointedTo = T;
>      type Borrowed<'a> = Pin<&'a T>;
>      type BorrowedMut<'a> = Pin<&'a mut T>;
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index c7af0aa48a0a..24fb63597d35 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -374,6 +374,7 @@ pub fn into_unique_or_drop(self) -> Option<Pin<UniqueArc<T>>> {
>  
>  // SAFETY: The `into_foreign` function returns a pointer that is well-aligned.
>  unsafe impl<T: 'static> ForeignOwnable for Arc<T> {
> +    type Target = T;
>      type PointedTo = ArcInner<T>;
>      type Borrowed<'a> = ArcBorrow<'a, T>;
>      type BorrowedMut<'a> = Self::Borrowed<'a>;
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 3958a5f44d56..74c787b352a9 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -27,6 +27,9 @@
>  /// [`into_foreign`]: Self::into_foreign
>  /// [`PointedTo`]: Self::PointedTo
>  pub unsafe trait ForeignOwnable: Sized {
> +    /// The payload type of the foreign-owned value.
> +    type Target;

I think `ForeignOwnable` also implies a `T` maybe get dropped via a
pointer from `into_foreign()`. Not sure it's worth mentioning though.

Regards,
Boqun

> +
>      /// Type used when the value is foreign-owned. In practical terms only defines the alignment of
>      /// the pointer.
>      type PointedTo;
> @@ -128,6 +131,7 @@ unsafe fn try_from_foreign(ptr: *mut Self::PointedTo) -> Option<Self> {
>  
>  // SAFETY: The `into_foreign` function returns a pointer that is dangling, but well-aligned.
>  unsafe impl ForeignOwnable for () {
> +    type Target = ();
>      type PointedTo = ();
>      type Borrowed<'a> = ();
>      type BorrowedMut<'a> = ();
> -- 
> 2.49.0
>
Re: [PATCH v4 4/5] rust: types: ForeignOwnable: Add type Target
Posted by Benno Lossin 3 months, 1 week ago
On Thu Jun 26, 2025 at 10:20 PM CEST, Boqun Feng wrote:
> On Thu, Jun 26, 2025 at 10:00:42PM +0200, Danilo Krummrich wrote:
>> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
>> index 3958a5f44d56..74c787b352a9 100644
>> --- a/rust/kernel/types.rs
>> +++ b/rust/kernel/types.rs
>> @@ -27,6 +27,9 @@
>>  /// [`into_foreign`]: Self::into_foreign
>>  /// [`PointedTo`]: Self::PointedTo
>>  pub unsafe trait ForeignOwnable: Sized {
>> +    /// The payload type of the foreign-owned value.
>> +    type Target;
>
> I think `ForeignOwnable` also implies a `T` maybe get dropped via a
> pointer from `into_foreign()`. Not sure it's worth mentioning though.

What? How would that happen?

---
Cheers,
Benno
Re: [PATCH v4 4/5] rust: types: ForeignOwnable: Add type Target
Posted by Boqun Feng 3 months, 1 week ago

On Thu, Jun 26, 2025, at 4:17 PM, Benno Lossin wrote:
> On Thu Jun 26, 2025 at 10:20 PM CEST, Boqun Feng wrote:
>> On Thu, Jun 26, 2025 at 10:00:42PM +0200, Danilo Krummrich wrote:
>>> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
>>> index 3958a5f44d56..74c787b352a9 100644
>>> --- a/rust/kernel/types.rs
>>> +++ b/rust/kernel/types.rs
>>> @@ -27,6 +27,9 @@
>>>  /// [`into_foreign`]: Self::into_foreign
>>>  /// [`PointedTo`]: Self::PointedTo
>>>  pub unsafe trait ForeignOwnable: Sized {
>>> +    /// The payload type of the foreign-owned value.
>>> +    type Target;
>>
>> I think `ForeignOwnable` also implies a `T` maybe get dropped via a
>> pointer from `into_foreign()`. Not sure it's worth mentioning though.
>
> What? How would that happen?
>

The owner of the pointer can do from_foreign() and eventually drop
the ForeignOwnable, hence dropping T.

Regards,
Boqun

> ---
> Cheers,
> Benno
Re: [PATCH v4 4/5] rust: types: ForeignOwnable: Add type Target
Posted by Benno Lossin 3 months, 1 week ago
On Fri Jun 27, 2025 at 1:21 AM CEST, Boqun Feng wrote:
> On Thu, Jun 26, 2025, at 4:17 PM, Benno Lossin wrote:
>> On Thu Jun 26, 2025 at 10:20 PM CEST, Boqun Feng wrote:
>>> On Thu, Jun 26, 2025 at 10:00:42PM +0200, Danilo Krummrich wrote:
>>>> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
>>>> index 3958a5f44d56..74c787b352a9 100644
>>>> --- a/rust/kernel/types.rs
>>>> +++ b/rust/kernel/types.rs
>>>> @@ -27,6 +27,9 @@
>>>>  /// [`into_foreign`]: Self::into_foreign
>>>>  /// [`PointedTo`]: Self::PointedTo
>>>>  pub unsafe trait ForeignOwnable: Sized {
>>>> +    /// The payload type of the foreign-owned value.
>>>> +    type Target;
>>>
>>> I think `ForeignOwnable` also implies a `T` maybe get dropped via a
>>> pointer from `into_foreign()`. Not sure it's worth mentioning though.
>>
>> What? How would that happen?
>
> The owner of the pointer can do from_foreign() and eventually drop
> the ForeignOwnable, hence dropping T.

I'm confused, you said `into_foreign` above. I don't think any sensible
ForeignOwnable implementation will drop a T in any of its functions.

---
Cheers,
Benno
Re: [PATCH v4 4/5] rust: types: ForeignOwnable: Add type Target
Posted by Boqun Feng 3 months, 1 week ago

On Thu, Jun 26, 2025, at 4:36 PM, Benno Lossin wrote:
> On Fri Jun 27, 2025 at 1:21 AM CEST, Boqun Feng wrote:
>> On Thu, Jun 26, 2025, at 4:17 PM, Benno Lossin wrote:
>>> On Thu Jun 26, 2025 at 10:20 PM CEST, Boqun Feng wrote:
>>>> On Thu, Jun 26, 2025 at 10:00:42PM +0200, Danilo Krummrich wrote:
>>>>> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
>>>>> index 3958a5f44d56..74c787b352a9 100644
>>>>> --- a/rust/kernel/types.rs
>>>>> +++ b/rust/kernel/types.rs
>>>>> @@ -27,6 +27,9 @@
>>>>>  /// [`into_foreign`]: Self::into_foreign
>>>>>  /// [`PointedTo`]: Self::PointedTo
>>>>>  pub unsafe trait ForeignOwnable: Sized {
>>>>> +    /// The payload type of the foreign-owned value.
>>>>> +    type Target;
>>>>
>>>> I think `ForeignOwnable` also implies a `T` maybe get dropped via a
>>>> pointer from `into_foreign()`. Not sure it's worth mentioning though.
>>>
>>> What? How would that happen?
>>
>> The owner of the pointer can do from_foreign() and eventually drop
>> the ForeignOwnable, hence dropping T.
>
> I'm confused, you said `into_foreign` above. I don't think any sensible
> ForeignOwnable implementation will drop a T in any of its functions.
>

A KBox<T> would drop T when it gets dropped, no?
A Arc<T> would drop T when it gets dropped if it’s the last refcount.

I was trying to say “ForeignOwnable::drop() may implies Target::drop()”,
that’s what a “payload” means. Maybe that I used “T” instead of “Target”
in the original message caused confusion?

Regards,
Boqun

> ---
> Cheers,
> Benno
Re: [PATCH v4 4/5] rust: types: ForeignOwnable: Add type Target
Posted by Boqun Feng 3 months, 1 week ago

On Thu, Jun 26, 2025, at 4:45 PM, Boqun Feng wrote:
> On Thu, Jun 26, 2025, at 4:36 PM, Benno Lossin wrote:
>> On Fri Jun 27, 2025 at 1:21 AM CEST, Boqun Feng wrote:
>>> On Thu, Jun 26, 2025, at 4:17 PM, Benno Lossin wrote:
>>>> On Thu Jun 26, 2025 at 10:20 PM CEST, Boqun Feng wrote:
>>>>> On Thu, Jun 26, 2025 at 10:00:42PM +0200, Danilo Krummrich wrote:
>>>>>> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
>>>>>> index 3958a5f44d56..74c787b352a9 100644
>>>>>> --- a/rust/kernel/types.rs
>>>>>> +++ b/rust/kernel/types.rs
>>>>>> @@ -27,6 +27,9 @@
>>>>>>  /// [`into_foreign`]: Self::into_foreign
>>>>>>  /// [`PointedTo`]: Self::PointedTo
>>>>>>  pub unsafe trait ForeignOwnable: Sized {
>>>>>> +    /// The payload type of the foreign-owned value.
>>>>>> +    type Target;
>>>>>
>>>>> I think `ForeignOwnable` also implies a `T` maybe get dropped via a
>>>>> pointer from `into_foreign()`. Not sure it's worth mentioning though.
>>>>
>>>> What? How would that happen?
>>>
>>> The owner of the pointer can do from_foreign() and eventually drop
>>> the ForeignOwnable, hence dropping T.
>>
>> I'm confused, you said `into_foreign` above. I don't think any sensible
>> ForeignOwnable implementation will drop a T in any of its functions.
>>
>
> A KBox<T> would drop T when it gets dropped, no?
> A Arc<T> would drop T when it gets dropped if it’s the last refcount.
>
> I was trying to say “ForeignOwnable::drop() may implies Target::drop()”,
> that’s what a “payload” means. Maybe that I used “T” instead of “Target”
> in the original message caused confusion?
>

The point is whichever receives the pointer from a into_foreign()
would owns the Target, because it can from_foreign() and
drop the ForeignOwnable. So for example, if the pointer can
be passed across threads, that means Target needs to be Send.

Regards,
Boqun

> Regards,
> Boqun
>
>> ---
>> Cheers,
>> Benno
Re: [PATCH v4 4/5] rust: types: ForeignOwnable: Add type Target
Posted by Benno Lossin 3 months, 1 week ago
On Fri Jun 27, 2025 at 1:55 AM CEST, Boqun Feng wrote:
> On Thu, Jun 26, 2025, at 4:45 PM, Boqun Feng wrote:
>> On Thu, Jun 26, 2025, at 4:36 PM, Benno Lossin wrote:
>>> On Fri Jun 27, 2025 at 1:21 AM CEST, Boqun Feng wrote:
>>>> On Thu, Jun 26, 2025, at 4:17 PM, Benno Lossin wrote:
>>>>> On Thu Jun 26, 2025 at 10:20 PM CEST, Boqun Feng wrote:
>>>>>> On Thu, Jun 26, 2025 at 10:00:42PM +0200, Danilo Krummrich wrote:
>>>>>>> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
>>>>>>> index 3958a5f44d56..74c787b352a9 100644
>>>>>>> --- a/rust/kernel/types.rs
>>>>>>> +++ b/rust/kernel/types.rs
>>>>>>> @@ -27,6 +27,9 @@
>>>>>>>  /// [`into_foreign`]: Self::into_foreign
>>>>>>>  /// [`PointedTo`]: Self::PointedTo
>>>>>>>  pub unsafe trait ForeignOwnable: Sized {
>>>>>>> +    /// The payload type of the foreign-owned value.
>>>>>>> +    type Target;
>>>>>>
>>>>>> I think `ForeignOwnable` also implies a `T` maybe get dropped via a
>>>>>> pointer from `into_foreign()`. Not sure it's worth mentioning though.
>>>>>
>>>>> What? How would that happen?
>>>>
>>>> The owner of the pointer can do from_foreign() and eventually drop
>>>> the ForeignOwnable, hence dropping T.
>>>
>>> I'm confused, you said `into_foreign` above. I don't think any sensible
>>> ForeignOwnable implementation will drop a T in any of its functions.
>>>
>>
>> A KBox<T> would drop T when it gets dropped, no?
>> A Arc<T> would drop T when it gets dropped if it’s the last refcount.
>>
>> I was trying to say “ForeignOwnable::drop() may implies Target::drop()”,
>> that’s what a “payload” means. Maybe that I used “T” instead of “Target”
>> in the original message caused confusion?

Ah now I understand what you are saying. Your mentioning of
`into_foreign` and `from_foreign` confused me. Yes a `ForeignOwnable`
may drop a `T`/`Target` in its own drop function. But I don't think we
need to document that.

> The point is whichever receives the pointer from a into_foreign()
> would owns the Target, because it can from_foreign() and
> drop the ForeignOwnable. So for example, if the pointer can
> be passed across threads, that means Target needs to be Send.

We should solve this in a different manner. Document the `Send` & `Sync`
requirements on `into_foreign`. So when you turn a `P: ForeignOwnable`
that is `!Send` into a raw pointer, you are not allowed to call
`from_foreign` on a different thread.

If `P: !Sync` then you're not allowed to call `borrow[_mut]` on the
pointer from two different threads (ie only the one that is currently
owning the value is allowed to call that).

---
Cheers,
Benno