[PATCH 1/7] rust: types: support fallible PinInit types in Opaque::pin_init

Danilo Krummrich posted 7 patches 6 months, 2 weeks ago
[PATCH 1/7] rust: types: support fallible PinInit types in Opaque::pin_init
Posted by Danilo Krummrich 6 months, 2 weeks ago
Currently, Opaque::pin_init only supports infallible PinInit
implementations, i.e. impl PinInit<T, Infallible>.

This has been sufficient so far, since users such as Revocable do not
support fallibility.

Since this is about to change, make Opaque::pin_init() generic over the
error type E.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/types.rs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 22985b6f6982..75c99d6facf9 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -354,13 +354,13 @@ pub const fn zeroed() -> Self {
     }
 
     /// Create an opaque pin-initializer from the given pin-initializer.
-    pub fn pin_init(slot: impl PinInit<T>) -> impl PinInit<Self> {
-        Self::ffi_init(|ptr: *mut T| {
+    pub fn pin_init<E>(slot: impl PinInit<T, E>) -> impl PinInit<Self, E> {
+        Self::try_ffi_init(|ptr: *mut T| -> Result<(), E> {
             // SAFETY:
             //   - `ptr` is a valid pointer to uninitialized memory,
-            //   - `slot` is not accessed on error; the call is infallible,
+            //   - `slot` is not accessed on error,
             //   - `slot` is pinned in memory.
-            let _ = unsafe { PinInit::<T>::__pinned_init(slot, ptr) };
+            unsafe { PinInit::<T, E>::__pinned_init(slot, ptr) }
         })
     }
 
-- 
2.49.0
Re: [PATCH 1/7] rust: types: support fallible PinInit types in Opaque::pin_init
Posted by Benno Lossin 6 months, 2 weeks ago
On Fri May 30, 2025 at 4:24 PM CEST, Danilo Krummrich wrote:
> Currently, Opaque::pin_init only supports infallible PinInit
> implementations, i.e. impl PinInit<T, Infallible>.
>
> This has been sufficient so far, since users such as Revocable do not
> support fallibility.
>
> Since this is about to change, make Opaque::pin_init() generic over the
> error type E.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  rust/kernel/types.rs | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 22985b6f6982..75c99d6facf9 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -354,13 +354,13 @@ pub const fn zeroed() -> Self {
>      }
>  
>      /// Create an opaque pin-initializer from the given pin-initializer.
> -    pub fn pin_init(slot: impl PinInit<T>) -> impl PinInit<Self> {
> -        Self::ffi_init(|ptr: *mut T| {
> +    pub fn pin_init<E>(slot: impl PinInit<T, E>) -> impl PinInit<Self, E> {
> +        Self::try_ffi_init(|ptr: *mut T| -> Result<(), E> {
>              // SAFETY:
>              //   - `ptr` is a valid pointer to uninitialized memory,
> -            //   - `slot` is not accessed on error; the call is infallible,
> +            //   - `slot` is not accessed on error,
>              //   - `slot` is pinned in memory.
> -            let _ = unsafe { PinInit::<T>::__pinned_init(slot, ptr) };
> +            unsafe { PinInit::<T, E>::__pinned_init(slot, ptr) }

Could you move this function into an `impl pin_init::Wrapper<T>` block?
(it's the same function, but in a trait that was recently added)

Thanks!

---
Cheers,
Benno

>          })
>      }
>  
Re: [PATCH 1/7] rust: types: support fallible PinInit types in Opaque::pin_init
Posted by Christian Schrefl 6 months, 2 weeks ago

On 30.05.25 9:29 PM, Benno Lossin wrote:
> On Fri May 30, 2025 at 4:24 PM CEST, Danilo Krummrich wrote:
>> Currently, Opaque::pin_init only supports infallible PinInit
>> implementations, i.e. impl PinInit<T, Infallible>.
>>
>> This has been sufficient so far, since users such as Revocable do not
>> support fallibility.
>>
>> Since this is about to change, make Opaque::pin_init() generic over the
>> error type E.
>>
>> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
>> ---
>>  rust/kernel/types.rs | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
>> index 22985b6f6982..75c99d6facf9 100644
>> --- a/rust/kernel/types.rs
>> +++ b/rust/kernel/types.rs
>> @@ -354,13 +354,13 @@ pub const fn zeroed() -> Self {
>>      }
>>  
>>      /// Create an opaque pin-initializer from the given pin-initializer.
>> -    pub fn pin_init(slot: impl PinInit<T>) -> impl PinInit<Self> {
>> -        Self::ffi_init(|ptr: *mut T| {
>> +    pub fn pin_init<E>(slot: impl PinInit<T, E>) -> impl PinInit<Self, E> {
>> +        Self::try_ffi_init(|ptr: *mut T| -> Result<(), E> {
>>              // SAFETY:
>>              //   - `ptr` is a valid pointer to uninitialized memory,
>> -            //   - `slot` is not accessed on error; the call is infallible,
>> +            //   - `slot` is not accessed on error,
>>              //   - `slot` is pinned in memory.
>> -            let _ = unsafe { PinInit::<T>::__pinned_init(slot, ptr) };
>> +            unsafe { PinInit::<T, E>::__pinned_init(slot, ptr) }
> 
> Could you move this function into an `impl pin_init::Wrapper<T>` block?
> (it's the same function, but in a trait that was recently added)

This is then basically this patch [0] from my `UnsafePinned` series.
Just that I did not update the comment. :)

[0]: https://lore.kernel.org/rust-for-linux/20250511-rust_unsafe_pinned-v4-2-a86c32e47e3d@gmail.com/ 

> 
> Thanks!
> 
> ---
> Cheers,
> Benno
> 
>>          })
>>      }
>>  
>
Re: [PATCH 1/7] rust: types: support fallible PinInit types in Opaque::pin_init
Posted by Danilo Krummrich 6 months, 2 weeks ago
On Fri, May 30, 2025 at 10:11:00PM +0200, Christian Schrefl wrote:
> 
> 
> On 30.05.25 9:29 PM, Benno Lossin wrote:
> > On Fri May 30, 2025 at 4:24 PM CEST, Danilo Krummrich wrote:
> >> Currently, Opaque::pin_init only supports infallible PinInit
> >> implementations, i.e. impl PinInit<T, Infallible>.
> >>
> >> This has been sufficient so far, since users such as Revocable do not
> >> support fallibility.
> >>
> >> Since this is about to change, make Opaque::pin_init() generic over the
> >> error type E.
> >>
> >> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> >> ---
> >>  rust/kernel/types.rs | 8 ++++----
> >>  1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> >> index 22985b6f6982..75c99d6facf9 100644
> >> --- a/rust/kernel/types.rs
> >> +++ b/rust/kernel/types.rs
> >> @@ -354,13 +354,13 @@ pub const fn zeroed() -> Self {
> >>      }
> >>  
> >>      /// Create an opaque pin-initializer from the given pin-initializer.
> >> -    pub fn pin_init(slot: impl PinInit<T>) -> impl PinInit<Self> {
> >> -        Self::ffi_init(|ptr: *mut T| {
> >> +    pub fn pin_init<E>(slot: impl PinInit<T, E>) -> impl PinInit<Self, E> {
> >> +        Self::try_ffi_init(|ptr: *mut T| -> Result<(), E> {
> >>              // SAFETY:
> >>              //   - `ptr` is a valid pointer to uninitialized memory,
> >> -            //   - `slot` is not accessed on error; the call is infallible,
> >> +            //   - `slot` is not accessed on error,
> >>              //   - `slot` is pinned in memory.
> >> -            let _ = unsafe { PinInit::<T>::__pinned_init(slot, ptr) };
> >> +            unsafe { PinInit::<T, E>::__pinned_init(slot, ptr) }
> > 
> > Could you move this function into an `impl pin_init::Wrapper<T>` block?
> > (it's the same function, but in a trait that was recently added)
> 
> This is then basically this patch [0] from my `UnsafePinned` series.
> Just that I did not update the comment. :)

As mentioned in [0], I wasn't aware of this patch -- let's go with yours then.

> [0]: https://lore.kernel.org/rust-for-linux/20250511-rust_unsafe_pinned-v4-2-a86c32e47e3d@gmail.com/
Re: [PATCH 1/7] rust: types: support fallible PinInit types in Opaque::pin_init
Posted by Benno Lossin 6 months, 2 weeks ago
On Fri May 30, 2025 at 10:11 PM CEST, Christian Schrefl wrote:
> On 30.05.25 9:29 PM, Benno Lossin wrote:
>> On Fri May 30, 2025 at 4:24 PM CEST, Danilo Krummrich wrote:
>>> -    pub fn pin_init(slot: impl PinInit<T>) -> impl PinInit<Self> {
>>> -        Self::ffi_init(|ptr: *mut T| {
>>> +    pub fn pin_init<E>(slot: impl PinInit<T, E>) -> impl PinInit<Self, E> {
>>> +        Self::try_ffi_init(|ptr: *mut T| -> Result<(), E> {
>>>              // SAFETY:
>>>              //   - `ptr` is a valid pointer to uninitialized memory,
>>> -            //   - `slot` is not accessed on error; the call is infallible,
>>> +            //   - `slot` is not accessed on error,
>>>              //   - `slot` is pinned in memory.
>>> -            let _ = unsafe { PinInit::<T>::__pinned_init(slot, ptr) };
>>> +            unsafe { PinInit::<T, E>::__pinned_init(slot, ptr) }
>> 
>> Could you move this function into an `impl pin_init::Wrapper<T>` block?
>> (it's the same function, but in a trait that was recently added)
>
> This is then basically this patch [0] from my `UnsafePinned` series.
> Just that I did not update the comment. :)
>
> [0]: https://lore.kernel.org/rust-for-linux/20250511-rust_unsafe_pinned-v4-2-a86c32e47e3d@gmail.com/ 

Oh yeah, I completely forgot we had this... I even reviewed it haha!

---
Cheers,
Benno
Re: [PATCH 1/7] rust: types: support fallible PinInit types in Opaque::pin_init
Posted by Christian Schrefl 6 months, 2 weeks ago
On 30.05.25 4:24 PM, Danilo Krummrich wrote:
> Currently, Opaque::pin_init only supports infallible PinInit
> implementations, i.e. impl PinInit<T, Infallible>.
> 
> This has been sufficient so far, since users such as Revocable do not
> support fallibility.
> 
> Since this is about to change, make Opaque::pin_init() generic over the
> error type E.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---

Reviewed-by: Christian Schrefl <chrisi.schrefl@gmail.com>

>  rust/kernel/types.rs | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 22985b6f6982..75c99d6facf9 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -354,13 +354,13 @@ pub const fn zeroed() -> Self {
>      }
>  
>      /// Create an opaque pin-initializer from the given pin-initializer.
> -    pub fn pin_init(slot: impl PinInit<T>) -> impl PinInit<Self> {
> -        Self::ffi_init(|ptr: *mut T| {
> +    pub fn pin_init<E>(slot: impl PinInit<T, E>) -> impl PinInit<Self, E> {
> +        Self::try_ffi_init(|ptr: *mut T| -> Result<(), E> {
>              // SAFETY:
>              //   - `ptr` is a valid pointer to uninitialized memory,
> -            //   - `slot` is not accessed on error; the call is infallible,
> +            //   - `slot` is not accessed on error,
>              //   - `slot` is pinned in memory.
> -            let _ = unsafe { PinInit::<T>::__pinned_init(slot, ptr) };
> +            unsafe { PinInit::<T, E>::__pinned_init(slot, ptr) }
>          })
>      }
>