This change makes the semantics of the `Opaque` implementation
clearer and prepares for the switch to the upstream rust UnsafePinned`
type in the future.
`Opaque` still uses `UnsafeCell` even though the kernel implementation
of `UnsafePinned` already includes it, since the current upstream
version does not.
Reviewed-by: Gerald Wisböck <gerald.wisboeck@feather.ink>
Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
---
rust/kernel/types.rs | 29 ++++++++++++-----------------
1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index f06e8720e012102e5c41e79fd97b0607e927d71c..44d96423a8a6c358bb7ebf12c24fad98e5c2cb61 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -4,12 +4,12 @@
use core::{
cell::UnsafeCell,
- marker::{PhantomData, PhantomPinned},
+ marker::PhantomData,
mem::{ManuallyDrop, MaybeUninit},
ops::{Deref, DerefMut},
ptr::NonNull,
};
-use pin_init::{PinInit, Wrapper, Zeroable};
+use pin_init::{cast_pin_init, PinInit, Wrapper, Zeroable};
/// Used to transfer ownership to and from foreign (non-Rust) languages.
///
@@ -308,8 +308,7 @@ fn drop(&mut self) {
/// ```
#[repr(transparent)]
pub struct Opaque<T> {
- value: UnsafeCell<MaybeUninit<T>>,
- _pin: PhantomPinned,
+ value: UnsafePinned<UnsafeCell<MaybeUninit<T>>>,
}
// SAFETY: `Opaque<T>` allows the inner value to be any bit pattern, including all zeros.
@@ -319,16 +318,14 @@ impl<T> Opaque<T> {
/// Creates a new opaque value.
pub const fn new(value: T) -> Self {
Self {
- value: UnsafeCell::new(MaybeUninit::new(value)),
- _pin: PhantomPinned,
+ value: UnsafePinned::new(UnsafeCell::new(MaybeUninit::new(value))),
}
}
/// Creates an uninitialised value.
pub const fn uninit() -> Self {
Self {
- value: UnsafeCell::new(MaybeUninit::uninit()),
- _pin: PhantomPinned,
+ value: UnsafePinned::new(UnsafeCell::new(MaybeUninit::uninit())),
}
}
@@ -371,7 +368,7 @@ pub fn try_ffi_init<E>(
/// Returns a raw pointer to the opaque data.
pub const fn get(&self) -> *mut T {
- UnsafeCell::get(&self.value).cast::<T>()
+ UnsafeCell::raw_get(self.value.get()).cast::<T>()
}
/// Gets the value behind `this`.
@@ -384,14 +381,12 @@ pub const fn raw_get(this: *const Self) -> *mut T {
}
impl<T> Wrapper<T> for Opaque<T> {
/// Create an opaque pin-initializer from the given pin-initializer.
- fn pin_init<E>(slot: impl PinInit<T, E>) -> impl PinInit<Self, E> {
- Self::try_ffi_init(|ptr: *mut T| {
- // SAFETY:
- // - `ptr` is a valid pointer to uninitialized memory,
- // - `slot` is not accessed on error; the call is infallible,
- // - `slot` is pinned in memory.
- unsafe { PinInit::<T, E>::__pinned_init(slot, ptr) }
- })
+ fn pin_init<E>(value_init: impl PinInit<T, E>) -> impl PinInit<Self, E> {
+ let value_init =
+ UnsafePinned::pin_init(UnsafeCell::pin_init(MaybeUninit::pin_init(value_init)));
+ // SAFETY: `Opaque<T>` is a `repr(transparent)` wrapper around
+ // `UnsafePinned<UnsafeCell<MabeUninit<T>>>` so the memory representation is compatible.
+ unsafe { cast_pin_init(value_init) }
}
}
--
2.49.0
[Cc Ralf]
On Wed, Apr 30, 2025 at 10:36:13AM +0200, Christian Schrefl wrote:
> This change makes the semantics of the `Opaque` implementation
> clearer and prepares for the switch to the upstream rust UnsafePinned`
> type in the future.
>
> `Opaque` still uses `UnsafeCell` even though the kernel implementation
> of `UnsafePinned` already includes it, since the current upstream
> version does not.
>
> Reviewed-by: Gerald Wisböck <gerald.wisboeck@feather.ink>
> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
> ---
> rust/kernel/types.rs | 29 ++++++++++++-----------------
> 1 file changed, 12 insertions(+), 17 deletions(-)
>
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index f06e8720e012102e5c41e79fd97b0607e927d71c..44d96423a8a6c358bb7ebf12c24fad98e5c2cb61 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -4,12 +4,12 @@
>
> use core::{
> cell::UnsafeCell,
> - marker::{PhantomData, PhantomPinned},
> + marker::PhantomData,
> mem::{ManuallyDrop, MaybeUninit},
> ops::{Deref, DerefMut},
> ptr::NonNull,
> };
> -use pin_init::{PinInit, Wrapper, Zeroable};
> +use pin_init::{cast_pin_init, PinInit, Wrapper, Zeroable};
>
> /// Used to transfer ownership to and from foreign (non-Rust) languages.
> ///
> @@ -308,8 +308,7 @@ fn drop(&mut self) {
> /// ```
> #[repr(transparent)]
> pub struct Opaque<T> {
> - value: UnsafeCell<MaybeUninit<T>>,
> - _pin: PhantomPinned,
> + value: UnsafePinned<UnsafeCell<MaybeUninit<T>>>,
What's the Rust upstream opinion on `&UnsafePinned` vs `&UnsafeCell`?
Does `&UnsafePinned` provide the same noalias behavior as `&UnsafeCell`?
I'm wondering whether we should just do:
pub struct Opaque<T> {
value: UnsafePinned<MaybeUninit<T>>,
_not_sync: PhantomData<UnsafeCell<()>>,
}
, instead.
Regards,
Boqun
> }
>
> // SAFETY: `Opaque<T>` allows the inner value to be any bit pattern, including all zeros.
> @@ -319,16 +318,14 @@ impl<T> Opaque<T> {
> /// Creates a new opaque value.
> pub const fn new(value: T) -> Self {
> Self {
> - value: UnsafeCell::new(MaybeUninit::new(value)),
> - _pin: PhantomPinned,
> + value: UnsafePinned::new(UnsafeCell::new(MaybeUninit::new(value))),
> }
> }
>
> /// Creates an uninitialised value.
> pub const fn uninit() -> Self {
> Self {
> - value: UnsafeCell::new(MaybeUninit::uninit()),
> - _pin: PhantomPinned,
> + value: UnsafePinned::new(UnsafeCell::new(MaybeUninit::uninit())),
> }
> }
>
> @@ -371,7 +368,7 @@ pub fn try_ffi_init<E>(
>
> /// Returns a raw pointer to the opaque data.
> pub const fn get(&self) -> *mut T {
> - UnsafeCell::get(&self.value).cast::<T>()
> + UnsafeCell::raw_get(self.value.get()).cast::<T>()
> }
>
> /// Gets the value behind `this`.
> @@ -384,14 +381,12 @@ pub const fn raw_get(this: *const Self) -> *mut T {
> }
> impl<T> Wrapper<T> for Opaque<T> {
> /// Create an opaque pin-initializer from the given pin-initializer.
> - fn pin_init<E>(slot: impl PinInit<T, E>) -> impl PinInit<Self, E> {
> - Self::try_ffi_init(|ptr: *mut T| {
> - // SAFETY:
> - // - `ptr` is a valid pointer to uninitialized memory,
> - // - `slot` is not accessed on error; the call is infallible,
> - // - `slot` is pinned in memory.
> - unsafe { PinInit::<T, E>::__pinned_init(slot, ptr) }
> - })
> + fn pin_init<E>(value_init: impl PinInit<T, E>) -> impl PinInit<Self, E> {
> + let value_init =
> + UnsafePinned::pin_init(UnsafeCell::pin_init(MaybeUninit::pin_init(value_init)));
> + // SAFETY: `Opaque<T>` is a `repr(transparent)` wrapper around
> + // `UnsafePinned<UnsafeCell<MabeUninit<T>>>` so the memory representation is compatible.
> + unsafe { cast_pin_init(value_init) }
> }
> }
>
>
> --
> 2.49.0
>
>
On 30.04.25 6:44 PM, Boqun Feng wrote:
> [Cc Ralf]
>
> On Wed, Apr 30, 2025 at 10:36:13AM +0200, Christian Schrefl wrote:
>> This change makes the semantics of the `Opaque` implementation
>> clearer and prepares for the switch to the upstream rust UnsafePinned`
>> type in the future.
>>
>> `Opaque` still uses `UnsafeCell` even though the kernel implementation
>> of `UnsafePinned` already includes it, since the current upstream
>> version does not.
>>
>> Reviewed-by: Gerald Wisböck <gerald.wisboeck@feather.ink>
>> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
>> ---
>> rust/kernel/types.rs | 29 ++++++++++++-----------------
>> 1 file changed, 12 insertions(+), 17 deletions(-)
>>
>> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
>> index f06e8720e012102e5c41e79fd97b0607e927d71c..44d96423a8a6c358bb7ebf12c24fad98e5c2cb61 100644
>> --- a/rust/kernel/types.rs
>> +++ b/rust/kernel/types.rs
>> @@ -4,12 +4,12 @@
>>
>> use core::{
>> cell::UnsafeCell,
>> - marker::{PhantomData, PhantomPinned},
>> + marker::PhantomData,
>> mem::{ManuallyDrop, MaybeUninit},
>> ops::{Deref, DerefMut},
>> ptr::NonNull,
>> };
>> -use pin_init::{PinInit, Wrapper, Zeroable};
>> +use pin_init::{cast_pin_init, PinInit, Wrapper, Zeroable};
>>
>> /// Used to transfer ownership to and from foreign (non-Rust) languages.
>> ///
>> @@ -308,8 +308,7 @@ fn drop(&mut self) {
>> /// ```
>> #[repr(transparent)]
>> pub struct Opaque<T> {
>> - value: UnsafeCell<MaybeUninit<T>>,
>> - _pin: PhantomPinned,
>> + value: UnsafePinned<UnsafeCell<MaybeUninit<T>>>,
>
> What's the Rust upstream opinion on `&UnsafePinned` vs `&UnsafeCell`?
> Does `&UnsafePinned` provide the same noalias behavior as `&UnsafeCell`?
From the upsteam rust docs [0]:
> Further note that this does not lift the requirement that shared
references must be read-only! Use `UnsafeCell` for that.
I at some point there was discussion about possibly needing to use
`UnsafeCell` internally (because of `pin::deref`). But since the
current upstream documentation explicitly mentions still needing
`UnsafeCell` I assume that it will stay this was. If the upstream
implementation changes I can add a patch that removes the
unnecessary `UnsafeCell` and changes the documentation.
I asked about this on the `UnsafePinned` rust tracking issue [1].
Link: https://doc.rust-lang.org/nightly/std/pin/struct.UnsafePinned.html [0]
Link: https://github.com/rust-lang/rust/issues/125735#issuecomment-2842668816 [1]
Cheers
Christian
> I'm wondering whether we should just do:
>
> pub struct Opaque<T> {
> value: UnsafePinned<MaybeUninit<T>>,
> _not_sync: PhantomData<UnsafeCell<()>>,
> }
>
> , instead.
>
> Regards,
> Boqun
>
>> }
>>
>> // SAFETY: `Opaque<T>` allows the inner value to be any bit pattern, including all zeros.
>> @@ -319,16 +318,14 @@ impl<T> Opaque<T> {
>> /// Creates a new opaque value.
>> pub const fn new(value: T) -> Self {
>> Self {
>> - value: UnsafeCell::new(MaybeUninit::new(value)),
>> - _pin: PhantomPinned,
>> + value: UnsafePinned::new(UnsafeCell::new(MaybeUninit::new(value))),
>> }
>> }
>>
>> /// Creates an uninitialised value.
>> pub const fn uninit() -> Self {
>> Self {
>> - value: UnsafeCell::new(MaybeUninit::uninit()),
>> - _pin: PhantomPinned,
>> + value: UnsafePinned::new(UnsafeCell::new(MaybeUninit::uninit())),
>> }
>> }
>>
>> @@ -371,7 +368,7 @@ pub fn try_ffi_init<E>(
>>
>> /// Returns a raw pointer to the opaque data.
>> pub const fn get(&self) -> *mut T {
>> - UnsafeCell::get(&self.value).cast::<T>()
>> + UnsafeCell::raw_get(self.value.get()).cast::<T>()
>> }
>>
>> /// Gets the value behind `this`.
>> @@ -384,14 +381,12 @@ pub const fn raw_get(this: *const Self) -> *mut T {
>> }
>> impl<T> Wrapper<T> for Opaque<T> {
>> /// Create an opaque pin-initializer from the given pin-initializer.
>> - fn pin_init<E>(slot: impl PinInit<T, E>) -> impl PinInit<Self, E> {
>> - Self::try_ffi_init(|ptr: *mut T| {
>> - // SAFETY:
>> - // - `ptr` is a valid pointer to uninitialized memory,
>> - // - `slot` is not accessed on error; the call is infallible,
>> - // - `slot` is pinned in memory.
>> - unsafe { PinInit::<T, E>::__pinned_init(slot, ptr) }
>> - })
>> + fn pin_init<E>(value_init: impl PinInit<T, E>) -> impl PinInit<Self, E> {
>> + let value_init =
>> + UnsafePinned::pin_init(UnsafeCell::pin_init(MaybeUninit::pin_init(value_init)));
>> + // SAFETY: `Opaque<T>` is a `repr(transparent)` wrapper around
>> + // `UnsafePinned<UnsafeCell<MabeUninit<T>>>` so the memory representation is compatible.
>> + unsafe { cast_pin_init(value_init) }
>> }
>> }
>>
>>
>> --
>> 2.49.0
>>
>>
On Wed Apr 30, 2025 at 10:36 AM CEST, Christian Schrefl wrote: > This change makes the semantics of the `Opaque` implementation > clearer and prepares for the switch to the upstream rust UnsafePinned` > type in the future. s/This change makes/Make/ s/prepares/prepare/ s/UnsafePinned`/`UnsafePinned`/ > `Opaque` still uses `UnsafeCell` even though the kernel implementation > of `UnsafePinned` already includes it, since the current upstream > version does not. This would be very important to add as a comment on the `value` field. > Reviewed-by: Gerald Wisböck <gerald.wisboeck@feather.ink> > Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com> The change itself looks good. With the comment added: Reviewed-by: Benno Lossin <lossin@kernel.org> --- Cheers, Benno > --- > rust/kernel/types.rs | 29 ++++++++++++----------------- > 1 file changed, 12 insertions(+), 17 deletions(-)
On Wed, Apr 30, 2025 at 10:36:13AM +0200, Christian Schrefl wrote: > This change makes the semantics of the `Opaque` implementation > clearer and prepares for the switch to the upstream rust UnsafePinned` > type in the future. > > `Opaque` still uses `UnsafeCell` even though the kernel implementation > of `UnsafePinned` already includes it, since the current upstream > version does not. > > Reviewed-by: Gerald Wisböck <gerald.wisboeck@feather.ink> > Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
© 2016 - 2026 Red Hat, Inc.