`UnsafePinned<T>` is useful for cases where a value might be shared with
C code but not directly used by it. In particular this is added for
storing additional data in the `MiscDeviceRegistration` which will be
shared between `fops->open` and the containing struct.

Similar to `Opaque` but guarantees that the value is always initialized
and that the inner value is dropped when `UnsafePinned` is dropped.

This was originally proposed for the IRQ abstractions [0] and is also
useful for other where the inner data may be aliased, but is always
valid and automatic `Drop` is desired.

Since then the `UnsafePinned` type was added to upstream Rust [1] by Sky
as a unstable feature, therefore this patch implements the subset of the
upstream API for the `UnsafePinned` type required for additional data in
`MiscDeviceRegistration` and in the implementation of the `Opaque` type.

Some differences to the upstream type definition are required in the
kernel implementation, because upstream type uses some compiler changes
to opt out of certain optimizations, this is documented in the
documentation and a comment on the `UnsafePinned` type.

The documentation on is based on the upstream rust documentation with
minor modifications for the kernel implementation.

Link: https://lore.kernel.org/rust-for-linux/CAH5fLgiOASgjoYKFz6kWwzLaH07DqP2ph+3YyCDh2+gYqGpABA@mail.gmail.com [0]
Link: https://github.com/rust-lang/rust/pull/137043 [1]
Suggested-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Gerald Wisböck <gerald.wisboeck@feather.ink>
Co-developed-by: Sky <sky@sky9.dev>
Signed-off-by: Sky <sky@sky9.dev>
Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
---
 init/Kconfig                       |   3 +
 rust/kernel/lib.rs                 |   1 +
 rust/kernel/types.rs               |   6 ++
 rust/kernel/types/unsafe_pinned.rs | 115 +++++++++++++++++++++++++++++++++++++
 4 files changed, 125 insertions(+)

diff --git a/init/Kconfig b/init/Kconfig
index 63f5974b9fa6ea3f5c92203cedd1f2f82aa468a1..727d85d2b59f555f1c33103bb78698551a41e7ca 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -140,6 +140,9 @@ config LD_CAN_USE_KEEP_IN_OVERLAY
 config RUSTC_HAS_COERCE_POINTEE
 	def_bool RUSTC_VERSION >= 108400
 
+config RUSTC_HAS_UNSAFE_PINNED
+	def_bool RUSTC_VERSION >= 108800
+
 config PAHOLE_VERSION
 	int
 	default $(shell,$(srctree)/scripts/pahole-version.sh $(PAHOLE))
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index de07aadd1ff5fe46fd89517e234b97a6590c8e93..c08f0a50f1d8db95799478caa8e85558a1fcae8d 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -17,6 +17,7 @@
 #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(coerce_unsized))]
 #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(dispatch_from_dyn))]
 #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(unsize))]
+#![cfg_attr(CONFIG_RUSTC_HAS_UNSAFE_PINNED, feature(unsafe_pinned))]
 #![feature(inline_const)]
 #![feature(lint_reasons)]
 // Stable in Rust 1.82
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 9d0471afc9648f2973235488b441eb109069adb1..705f420fdfbc4a576de1c4546578f2f04cdf615e 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -253,6 +253,9 @@ fn drop(&mut self) {
 ///
 /// [`Opaque<T>`] is meant to be used with FFI objects that are never interpreted by Rust code.
 ///
+/// In cases where the contained data is only used by Rust, is not allowed to be
+/// uninitialized and automatic [`Drop`] is desired [`UnsafePinned`] should be used instead.
+///
 /// It is used to wrap structs from the C side, like for example `Opaque<bindings::mutex>`.
 /// It gets rid of all the usual assumptions that Rust has for a value:
 ///
@@ -578,3 +581,6 @@ pub enum Either<L, R> {
 /// [`NotThreadSafe`]: type@NotThreadSafe
 #[allow(non_upper_case_globals)]
 pub const NotThreadSafe: NotThreadSafe = PhantomData;
+
+mod unsafe_pinned;
+pub use unsafe_pinned::UnsafePinned;
diff --git a/rust/kernel/types/unsafe_pinned.rs b/rust/kernel/types/unsafe_pinned.rs
new file mode 100644
index 0000000000000000000000000000000000000000..5a200aac30792bf71098087aee0fd9d2d51c468f
--- /dev/null
+++ b/rust/kernel/types/unsafe_pinned.rs
@@ -0,0 +1,115 @@
+// SPDX-License-Identifier: Apache-2.0 OR MIT
+
+//! The contents of this file partially come from the Rust standard library, hosted in
+//! the <https://github.com/rust-lang/rust> repository, licensed under
+//! "Apache-2.0 OR MIT" and adapted for kernel use. For copyright details,
+//! see <https://github.com/rust-lang/rust/blob/master/COPYRIGHT>.
+//!
+//! This file provides a implementation / polyfill of a subset of the upstream
+//! rust `UnsafePinned` type.
+
+use core::{cell::UnsafeCell, marker::PhantomPinned};
+use pin_init::{cast_pin_init, PinInit, Wrapper};
+
+/// This type provides a way to opt-out of typical aliasing rules;
+/// specifically, `&mut UnsafePinned<T>` is not guaranteed to be a unique pointer.
+///
+/// However, even if you define your type like `pub struct Wrapper(UnsafePinned<...>)`, it is still
+/// very risky to have an `&mut Wrapper` that aliases anything else. Many functions that work
+/// generically on `&mut T` assume that the memory that stores `T` is uniquely owned (such as
+/// `mem::swap`). In other words, while having aliasing with `&mut Wrapper` is not immediate
+/// Undefined Behavior, it is still unsound to expose such a mutable reference to code you do not
+/// control! Techniques such as pinning via [`Pin`](core::pin::Pin) are needed to ensure soundness.
+///
+/// Similar to [`UnsafeCell`], [`UnsafePinned`] will not usually show up in
+/// the public API of a library. It is an internal implementation detail of libraries that need to
+/// support aliasing mutable references.
+///
+/// Further note that this does *not* lift the requirement that shared references must be read-only!
+/// Use [`UnsafeCell`] for that.
+///
+/// This type blocks niches the same way [`UnsafeCell`] does.
+///
+/// # Kernel implementation notes
+///
+/// This implementation works because of the "`!Unpin` hack" in rustc, which allows (some kinds of)
+/// mutual aliasing of `!Unpin` types. This hack might be removed at some point, after which only
+/// the `core::pin::UnsafePinned` type will allow this behavior. In order to simplify the migration
+/// to future rust versions only this polyfill of this type should be used when this behavior is
+/// required.
+///
+/// In order to disable niche optimizations this implementation uses [`UnsafeCell`] internally,
+/// the upstream version however will not. So the fact that [`UnsafePinned`] contains an
+/// [`UnsafeCell`] must not be relied on (Other than the niche blocking).
+// As opposed to the upstream Rust type this contains a `PhantomPinned`` and `UnsafeCell<T>`
+// - `PhantomPinned` to avoid needing a `impl<T> !Unpin for UnsafePinned<T>`
+//      Required to use the `!Unpin hack`.
+// - `UnsafeCell<T>` instead of T to disallow niche optimizations,
+//     which is handled in the compiler in upstream Rust
+#[repr(transparent)]
+pub struct UnsafePinned<T: ?Sized> {
+    _ph: PhantomPinned,
+    value: UnsafeCell<T>,
+}
+
+impl<T> UnsafePinned<T> {
+    /// Constructs a new instance of [`UnsafePinned`] which will wrap the specified value.
+    ///
+    /// All access to the inner value through `&UnsafePinned<T>` or `&mut UnsafePinned<T>` or
+    /// `Pin<&mut UnsafePinned<T>>` requires `unsafe` code.
+    #[inline(always)]
+    #[must_use]
+    pub const fn new(value: T) -> Self {
+        UnsafePinned {
+            value: UnsafeCell::new(value),
+            _ph: PhantomPinned,
+        }
+    }
+}
+impl<T: ?Sized> UnsafePinned<T> {
+    /// Get read-only access to the contents of a shared `UnsafePinned`.
+    ///
+    /// Note that `&UnsafePinned<T>` is read-only if `&T` is read-only. This means that if there is
+    /// mutation of the `T`, future reads from the `*const T` returned here are UB! Use
+    /// [`UnsafeCell`] if you also need interior mutability.
+    ///
+    /// [`UnsafeCell`]: core::cell::UnsafeCell
+    ///
+    /// ```rust,no_build
+    /// use kernel::types::UnsafePinned;
+    ///
+    /// unsafe {
+    ///     let mut x = UnsafePinned::new(0);
+    ///     let ptr = x.get(); // read-only pointer, assumes immutability
+    ///     x.get_mut_unchecked().write(1);
+    ///     ptr.read(); // UB!
+    /// }
+    /// ```
+    ///
+    /// Note that the `get_mut_unchecked` function used by this example is
+    /// currently not implemented in the kernel implementation.
+    #[inline(always)]
+    #[must_use]
+    pub const fn get(&self) -> *const T {
+        self.value.get()
+    }
+
+    /// Gets a mutable pointer to the wrapped value.
+    ///
+    /// The difference from `get_mut_pinned` and `get_mut_unchecked` is that this function
+    /// accepts a raw pointer, which is useful to avoid the creation of temporary references.
+    ///
+    /// These functions mentioned here are currently not implemented in the kernel.
+    #[inline(always)]
+    #[must_use]
+    pub const fn raw_get_mut(this: *mut Self) -> *mut T {
+        this as *mut T
+    }
+}
+
+impl<T> Wrapper<T> for UnsafePinned<T> {
+    fn pin_init<E>(init: impl PinInit<T, E>) -> impl PinInit<Self, E> {
+        // SAFETY: `UnsafePinned<T>` has a compatible layout to `T`.
+        unsafe { cast_pin_init(init) }
+    }
+}

-- 
2.49.0

On 30.04.25 10:36 AM, Christian Schrefl wrote:
> `UnsafePinned<T>` is useful for cases where a value might be shared with
> C code but not directly used by it. In particular this is added for
> storing additional data in the `MiscDeviceRegistration` which will be
> shared between `fops->open` and the containing struct.
> 
> Similar to `Opaque` but guarantees that the value is always initialized
> and that the inner value is dropped when `UnsafePinned` is dropped.
> 
> This was originally proposed for the IRQ abstractions [0] and is also
> useful for other where the inner data may be aliased, but is always
> valid and automatic `Drop` is desired.
> 
> Since then the `UnsafePinned` type was added to upstream Rust [1] by Sky
> as a unstable feature, therefore this patch implements the subset of the
> upstream API for the `UnsafePinned` type required for additional data in
> `MiscDeviceRegistration` and in the implementation of the `Opaque` type.
> 
> Some differences to the upstream type definition are required in the
> kernel implementation, because upstream type uses some compiler changes
> to opt out of certain optimizations, this is documented in the
> documentation and a comment on the `UnsafePinned` type.
> 
> The documentation on is based on the upstream rust documentation with
> minor modifications for the kernel implementation.
> 
> Link: https://lore.kernel.org/rust-for-linux/CAH5fLgiOASgjoYKFz6kWwzLaH07DqP2ph+3YyCDh2+gYqGpABA@mail.gmail.com [0]
> Link: https://github.com/rust-lang/rust/pull/137043 [1]
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Reviewed-by: Gerald Wisböck <gerald.wisboeck@feather.ink>
> Co-developed-by: Sky <sky@sky9.dev>
> Signed-off-by: Sky <sky@sky9.dev>
> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
> ---
>  init/Kconfig                       |   3 +
>  rust/kernel/lib.rs                 |   1 +
>  rust/kernel/types.rs               |   6 ++
>  rust/kernel/types/unsafe_pinned.rs | 115 +++++++++++++++++++++++++++++++++++++
>  4 files changed, 125 insertions(+)
> 
> diff --git a/init/Kconfig b/init/Kconfig
> index 63f5974b9fa6ea3f5c92203cedd1f2f82aa468a1..727d85d2b59f555f1c33103bb78698551a41e7ca 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -140,6 +140,9 @@ config LD_CAN_USE_KEEP_IN_OVERLAY
>  config RUSTC_HAS_COERCE_POINTEE
>  	def_bool RUSTC_VERSION >= 108400
>  
> +config RUSTC_HAS_UNSAFE_PINNED
> +	def_bool RUSTC_VERSION >= 108800
> +
>  config PAHOLE_VERSION
>  	int
>  	default $(shell,$(srctree)/scripts/pahole-version.sh $(PAHOLE))
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index de07aadd1ff5fe46fd89517e234b97a6590c8e93..c08f0a50f1d8db95799478caa8e85558a1fcae8d 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -17,6 +17,7 @@
>  #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(coerce_unsized))]
>  #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(dispatch_from_dyn))]
>  #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(unsize))]
> +#![cfg_attr(CONFIG_RUSTC_HAS_UNSAFE_PINNED, feature(unsafe_pinned))]
>  #![feature(inline_const)]
>  #![feature(lint_reasons)]
>  // Stable in Rust 1.82
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 9d0471afc9648f2973235488b441eb109069adb1..705f420fdfbc4a576de1c4546578f2f04cdf615e 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -253,6 +253,9 @@ fn drop(&mut self) {
>  ///
>  /// [`Opaque<T>`] is meant to be used with FFI objects that are never interpreted by Rust code.
>  ///
> +/// In cases where the contained data is only used by Rust, is not allowed to be
> +/// uninitialized and automatic [`Drop`] is desired [`UnsafePinned`] should be used instead.
> +///
>  /// It is used to wrap structs from the C side, like for example `Opaque<bindings::mutex>`.
>  /// It gets rid of all the usual assumptions that Rust has for a value:
>  ///
> @@ -578,3 +581,6 @@ pub enum Either<L, R> {
>  /// [`NotThreadSafe`]: type@NotThreadSafe
>  #[allow(non_upper_case_globals)]
>  pub const NotThreadSafe: NotThreadSafe = PhantomData;
> +
> +mod unsafe_pinned;
> +pub use unsafe_pinned::UnsafePinned;
> diff --git a/rust/kernel/types/unsafe_pinned.rs b/rust/kernel/types/unsafe_pinned.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..5a200aac30792bf71098087aee0fd9d2d51c468f
> --- /dev/null
> +++ b/rust/kernel/types/unsafe_pinned.rs
> @@ -0,0 +1,115 @@
> +// SPDX-License-Identifier: Apache-2.0 OR MIT
> +
> +//! The contents of this file partially come from the Rust standard library, hosted in
> +//! the <https://github.com/rust-lang/rust> repository, licensed under
> +//! "Apache-2.0 OR MIT" and adapted for kernel use. For copyright details,
> +//! see <https://github.com/rust-lang/rust/blob/master/COPYRIGHT>.
> +//!
> +//! This file provides a implementation / polyfill of a subset of the upstream
> +//! rust `UnsafePinned` type.
> +
> +use core::{cell::UnsafeCell, marker::PhantomPinned};
> +use pin_init::{cast_pin_init, PinInit, Wrapper};
> +
> +/// This type provides a way to opt-out of typical aliasing rules;
> +/// specifically, `&mut UnsafePinned<T>` is not guaranteed to be a unique pointer.
> +///
> +/// However, even if you define your type like `pub struct Wrapper(UnsafePinned<...>)`, it is still
> +/// very risky to have an `&mut Wrapper` that aliases anything else. Many functions that work
> +/// generically on `&mut T` assume that the memory that stores `T` is uniquely owned (such as
> +/// `mem::swap`). In other words, while having aliasing with `&mut Wrapper` is not immediate
> +/// Undefined Behavior, it is still unsound to expose such a mutable reference to code you do not
> +/// control! Techniques such as pinning via [`Pin`](core::pin::Pin) are needed to ensure soundness.
> +///
> +/// Similar to [`UnsafeCell`], [`UnsafePinned`] will not usually show up in
> +/// the public API of a library. It is an internal implementation detail of libraries that need to
> +/// support aliasing mutable references.
> +///
> +/// Further note that this does *not* lift the requirement that shared references must be read-only!
> +/// Use [`UnsafeCell`] for that.

[CC Ralf]

Ralf has replied to me on Github that this will most likely change [0]. How should this be handled?

I would fine with submitting a patch once it changes on the rust side (possibly waiting until the
feature is close to stabilization). I think it is better to only add this guarantee later as it
will be easier to remove unnecessary `UnsafeCell`s than it would be to later add them back in ever
case where they would be needed (in case rust doesn't change `UnsafePinned` to act like `UnsafeCell`).

Also see to the tracking issue [1] for the reason why `UnsafeCell` behavior is most likely required.

[0]: https://github.com/rust-lang/rust/issues/125735#issuecomment-2842926832
[1]: https://github.com/rust-lang/rust/issues/137750

Cheers
Christian

> +///
> +/// This type blocks niches the same way [`UnsafeCell`] does.
> +///
> +/// # Kernel implementation notes
> +///
> +/// This implementation works because of the "`!Unpin` hack" in rustc, which allows (some kinds of)
> +/// mutual aliasing of `!Unpin` types. This hack might be removed at some point, after which only
> +/// the `core::pin::UnsafePinned` type will allow this behavior. In order to simplify the migration
> +/// to future rust versions only this polyfill of this type should be used when this behavior is
> +/// required.
> +///
> +/// In order to disable niche optimizations this implementation uses [`UnsafeCell`] internally,
> +/// the upstream version however will not. So the fact that [`UnsafePinned`] contains an
> +/// [`UnsafeCell`] must not be relied on (Other than the niche blocking).
> +// As opposed to the upstream Rust type this contains a `PhantomPinned`` and `UnsafeCell<T>`
> +// - `PhantomPinned` to avoid needing a `impl<T> !Unpin for UnsafePinned<T>`
> +//      Required to use the `!Unpin hack`.
> +// - `UnsafeCell<T>` instead of T to disallow niche optimizations,
> +//     which is handled in the compiler in upstream Rust
> +#[repr(transparent)]
> +pub struct UnsafePinned<T: ?Sized> {
> +    _ph: PhantomPinned,
> +    value: UnsafeCell<T>,
> +}
> +
> +impl<T> UnsafePinned<T> {
> +    /// Constructs a new instance of [`UnsafePinned`] which will wrap the specified value.
> +    ///
> +    /// All access to the inner value through `&UnsafePinned<T>` or `&mut UnsafePinned<T>` or
> +    /// `Pin<&mut UnsafePinned<T>>` requires `unsafe` code.
> +    #[inline(always)]
> +    #[must_use]
> +    pub const fn new(value: T) -> Self {
> +        UnsafePinned {
> +            value: UnsafeCell::new(value),
> +            _ph: PhantomPinned,
> +        }
> +    }
> +}
> +impl<T: ?Sized> UnsafePinned<T> {
> +    /// Get read-only access to the contents of a shared `UnsafePinned`.
> +    ///
> +    /// Note that `&UnsafePinned<T>` is read-only if `&T` is read-only. This means that if there is
> +    /// mutation of the `T`, future reads from the `*const T` returned here are UB! Use
> +    /// [`UnsafeCell`] if you also need interior mutability.
> +    ///
> +    /// [`UnsafeCell`]: core::cell::UnsafeCell
> +    ///
> +    /// ```rust,no_build
> +    /// use kernel::types::UnsafePinned;
> +    ///
> +    /// unsafe {
> +    ///     let mut x = UnsafePinned::new(0);
> +    ///     let ptr = x.get(); // read-only pointer, assumes immutability
> +    ///     x.get_mut_unchecked().write(1);
> +    ///     ptr.read(); // UB!
> +    /// }
> +    /// ```
> +    ///
> +    /// Note that the `get_mut_unchecked` function used by this example is
> +    /// currently not implemented in the kernel implementation.
> +    #[inline(always)]
> +    #[must_use]
> +    pub const fn get(&self) -> *const T {
> +        self.value.get()
> +    }
> +
> +    /// Gets a mutable pointer to the wrapped value.
> +    ///
> +    /// The difference from `get_mut_pinned` and `get_mut_unchecked` is that this function
> +    /// accepts a raw pointer, which is useful to avoid the creation of temporary references.
> +    ///
> +    /// These functions mentioned here are currently not implemented in the kernel.
> +    #[inline(always)]
> +    #[must_use]
> +    pub const fn raw_get_mut(this: *mut Self) -> *mut T {
> +        this as *mut T
> +    }
> +}
> +
> +impl<T> Wrapper<T> for UnsafePinned<T> {
> +    fn pin_init<E>(init: impl PinInit<T, E>) -> impl PinInit<Self, E> {
> +        // SAFETY: `UnsafePinned<T>` has a compatible layout to `T`.
> +        unsafe { cast_pin_init(init) }
> +    }
> +}
> 

On Thu May 1, 2025 at 7:12 PM CEST, Christian Schrefl wrote:
> On 30.04.25 10:36 AM, Christian Schrefl wrote:
>> +/// This type provides a way to opt-out of typical aliasing rules;
>> +/// specifically, `&mut UnsafePinned<T>` is not guaranteed to be a unique pointer.
>> +///
>> +/// However, even if you define your type like `pub struct Wrapper(UnsafePinned<...>)`, it is still
>> +/// very risky to have an `&mut Wrapper` that aliases anything else. Many functions that work
>> +/// generically on `&mut T` assume that the memory that stores `T` is uniquely owned (such as
>> +/// `mem::swap`). In other words, while having aliasing with `&mut Wrapper` is not immediate
>> +/// Undefined Behavior, it is still unsound to expose such a mutable reference to code you do not
>> +/// control! Techniques such as pinning via [`Pin`](core::pin::Pin) are needed to ensure soundness.
>> +///
>> +/// Similar to [`UnsafeCell`], [`UnsafePinned`] will not usually show up in
>> +/// the public API of a library. It is an internal implementation detail of libraries that need to
>> +/// support aliasing mutable references.
>> +///
>> +/// Further note that this does *not* lift the requirement that shared references must be read-only!
>> +/// Use [`UnsafeCell`] for that.
>
> [CC Ralf]
>
> Ralf has replied to me on Github that this will most likely change [0]. How should this be handled?
>
> I would fine with submitting a patch once it changes on the rust side (possibly waiting until the
> feature is close to stabilization). I think it is better to only add this guarantee later as it
> will be easier to remove unnecessary `UnsafeCell`s than it would be to later add them back in ever
> case where they would be needed (in case rust doesn't change `UnsafePinned` to act like `UnsafeCell`).

Agreed, unless Ralf suggests a different way, we should do it like this.

---
Cheers,
Benno

> Also see to the tracking issue [1] for the reason why `UnsafeCell` behavior is most likely required.
>
> [0]: https://github.com/rust-lang/rust/issues/125735#issuecomment-2842926832
> [1]: https://github.com/rust-lang/rust/issues/137750
>
> Cheers
> Christian

Hi all,

On 01.05.25 20:55, Benno Lossin wrote:
> On Thu May 1, 2025 at 7:12 PM CEST, Christian Schrefl wrote:
>> On 30.04.25 10:36 AM, Christian Schrefl wrote:
>>> +/// This type provides a way to opt-out of typical aliasing rules;
>>> +/// specifically, `&mut UnsafePinned<T>` is not guaranteed to be a unique pointer.
>>> +///
>>> +/// However, even if you define your type like `pub struct Wrapper(UnsafePinned<...>)`, it is still
>>> +/// very risky to have an `&mut Wrapper` that aliases anything else. Many functions that work
>>> +/// generically on `&mut T` assume that the memory that stores `T` is uniquely owned (such as
>>> +/// `mem::swap`). In other words, while having aliasing with `&mut Wrapper` is not immediate
>>> +/// Undefined Behavior, it is still unsound to expose such a mutable reference to code you do not
>>> +/// control! Techniques such as pinning via [`Pin`](core::pin::Pin) are needed to ensure soundness.
>>> +///
>>> +/// Similar to [`UnsafeCell`], [`UnsafePinned`] will not usually show up in
>>> +/// the public API of a library. It is an internal implementation detail of libraries that need to
>>> +/// support aliasing mutable references.
>>> +///
>>> +/// Further note that this does *not* lift the requirement that shared references must be read-only!
>>> +/// Use [`UnsafeCell`] for that.
>>
>> [CC Ralf]
>>
>> Ralf has replied to me on Github that this will most likely change [0]. How should this be handled?
>>
>> I would fine with submitting a patch once it changes on the rust side (possibly waiting until the
>> feature is close to stabilization). I think it is better to only add this guarantee later as it
>> will be easier to remove unnecessary `UnsafeCell`s than it would be to later add them back in ever
>> case where they would be needed (in case rust doesn't change `UnsafePinned` to act like `UnsafeCell`).
> 
> Agreed, unless Ralf suggests a different way, we should do it like this.

Sorry, I replied only on github since that was easier to do more quickly. ;)

Yes I would recommend for now to keep the `UnsafeCell` in the kernel sources, 
until the compiler actually implements the change that removes `noalias` from 
`&UnsafePinned`. And even then you should probably keep the `UnsafeCell` when 
building for older compiler versions from before that patch (or keep it until 
you drop support for those older compiler versions).

This is not a spec-only change, codegen really has to be adjusted to make 
`&UnsafePinned` properly compatible with mutable aliasing, and I see no reason 
to risk potential problems here by prematurely removing that `UnsafeCell`.

Kind regards,
Ralf

> 
> ---
> Cheers,
> Benno
> 
>> Also see to the tracking issue [1] for the reason why `UnsafeCell` behavior is most likely required.
>>
>> [0]: https://github.com/rust-lang/rust/issues/125735#issuecomment-2842926832
>> [1]: https://github.com/rust-lang/rust/issues/137750
>>
>> Cheers
>> Christian

On Wed Apr 30, 2025 at 10:36 AM CEST, Christian Schrefl wrote:
> `UnsafePinned<T>` is useful for cases where a value might be shared with
> C code but not directly used by it. In particular this is added for
> storing additional data in the `MiscDeviceRegistration` which will be
> shared between `fops->open` and the containing struct.
>
> Similar to `Opaque` but guarantees that the value is always initialized
> and that the inner value is dropped when `UnsafePinned` is dropped.
>
> This was originally proposed for the IRQ abstractions [0] and is also
> useful for other where the inner data may be aliased, but is always
> valid and automatic `Drop` is desired.
>
> Since then the `UnsafePinned` type was added to upstream Rust [1] by Sky
> as a unstable feature, therefore this patch implements the subset of the
> upstream API for the `UnsafePinned` type required for additional data in
> `MiscDeviceRegistration` and in the implementation of the `Opaque` type.
>
> Some differences to the upstream type definition are required in the
> kernel implementation, because upstream type uses some compiler changes
> to opt out of certain optimizations, this is documented in the
> documentation and a comment on the `UnsafePinned` type.
>
> The documentation on is based on the upstream rust documentation with
> minor modifications for the kernel implementation.
>
> Link: https://lore.kernel.org/rust-for-linux/CAH5fLgiOASgjoYKFz6kWwzLaH07DqP2ph+3YyCDh2+gYqGpABA@mail.gmail.com [0]
> Link: https://github.com/rust-lang/rust/pull/137043 [1]
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Reviewed-by: Gerald Wisböck <gerald.wisboeck@feather.ink>
> Co-developed-by: Sky <sky@sky9.dev>
> Signed-off-by: Sky <sky@sky9.dev>
> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
> ---
>  init/Kconfig                       |   3 +
>  rust/kernel/lib.rs                 |   1 +
>  rust/kernel/types.rs               |   6 ++
>  rust/kernel/types/unsafe_pinned.rs | 115 +++++++++++++++++++++++++++++++++++++
>  4 files changed, 125 insertions(+)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 63f5974b9fa6ea3f5c92203cedd1f2f82aa468a1..727d85d2b59f555f1c33103bb78698551a41e7ca 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -140,6 +140,9 @@ config LD_CAN_USE_KEEP_IN_OVERLAY
>  config RUSTC_HAS_COERCE_POINTEE
>  	def_bool RUSTC_VERSION >= 108400
>  
> +config RUSTC_HAS_UNSAFE_PINNED
> +	def_bool RUSTC_VERSION >= 108800
> +
>  config PAHOLE_VERSION
>  	int
>  	default $(shell,$(srctree)/scripts/pahole-version.sh $(PAHOLE))
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index de07aadd1ff5fe46fd89517e234b97a6590c8e93..c08f0a50f1d8db95799478caa8e85558a1fcae8d 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -17,6 +17,7 @@
>  #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(coerce_unsized))]
>  #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(dispatch_from_dyn))]
>  #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(unsize))]
> +#![cfg_attr(CONFIG_RUSTC_HAS_UNSAFE_PINNED, feature(unsafe_pinned))]
>  #![feature(inline_const)]
>  #![feature(lint_reasons)]
>  // Stable in Rust 1.82
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 9d0471afc9648f2973235488b441eb109069adb1..705f420fdfbc4a576de1c4546578f2f04cdf615e 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -253,6 +253,9 @@ fn drop(&mut self) {
>  ///
>  /// [`Opaque<T>`] is meant to be used with FFI objects that are never interpreted by Rust code.
>  ///
> +/// In cases where the contained data is only used by Rust, is not allowed to be
> +/// uninitialized and automatic [`Drop`] is desired [`UnsafePinned`] should be used instead.
> +///
>  /// It is used to wrap structs from the C side, like for example `Opaque<bindings::mutex>`.
>  /// It gets rid of all the usual assumptions that Rust has for a value:
>  ///
> @@ -578,3 +581,6 @@ pub enum Either<L, R> {
>  /// [`NotThreadSafe`]: type@NotThreadSafe
>  #[allow(non_upper_case_globals)]
>  pub const NotThreadSafe: NotThreadSafe = PhantomData;
> +
> +mod unsafe_pinned;
> +pub use unsafe_pinned::UnsafePinned;
> diff --git a/rust/kernel/types/unsafe_pinned.rs b/rust/kernel/types/unsafe_pinned.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..5a200aac30792bf71098087aee0fd9d2d51c468f
> --- /dev/null
> +++ b/rust/kernel/types/unsafe_pinned.rs
> @@ -0,0 +1,115 @@
> +// SPDX-License-Identifier: Apache-2.0 OR MIT
> +
> +//! The contents of this file partially come from the Rust standard library, hosted in
> +//! the <https://github.com/rust-lang/rust> repository, licensed under
> +//! "Apache-2.0 OR MIT" and adapted for kernel use. For copyright details,
> +//! see <https://github.com/rust-lang/rust/blob/master/COPYRIGHT>.
> +//!
> +//! This file provides a implementation / polyfill of a subset of the upstream
> +//! rust `UnsafePinned` type.
> +
> +use core::{cell::UnsafeCell, marker::PhantomPinned};
> +use pin_init::{cast_pin_init, PinInit, Wrapper};
> +
> +/// This type provides a way to opt-out of typical aliasing rules;
> +/// specifically, `&mut UnsafePinned<T>` is not guaranteed to be a unique pointer.
> +///
> +/// However, even if you define your type like `pub struct Wrapper(UnsafePinned<...>)`, it is still
> +/// very risky to have an `&mut Wrapper` that aliases anything else. Many functions that work
> +/// generically on `&mut T` assume that the memory that stores `T` is uniquely owned (such as
> +/// `mem::swap`). In other words, while having aliasing with `&mut Wrapper` is not immediate
> +/// Undefined Behavior, it is still unsound to expose such a mutable reference to code you do not
> +/// control! Techniques such as pinning via [`Pin`](core::pin::Pin) are needed to ensure soundness.
> +///
> +/// Similar to [`UnsafeCell`], [`UnsafePinned`] will not usually show up in
> +/// the public API of a library. It is an internal implementation detail of libraries that need to
> +/// support aliasing mutable references.
> +///
> +/// Further note that this does *not* lift the requirement that shared references must be read-only!
> +/// Use [`UnsafeCell`] for that.
> +///
> +/// This type blocks niches the same way [`UnsafeCell`] does.
> +///
> +/// # Kernel implementation notes
> +///
> +/// This implementation works because of the "`!Unpin` hack" in rustc, which allows (some kinds of)
> +/// mutual aliasing of `!Unpin` types. This hack might be removed at some point, after which only
> +/// the `core::pin::UnsafePinned` type will allow this behavior. In order to simplify the migration
> +/// to future rust versions only this polyfill of this type should be used when this behavior is
> +/// required.
> +///
> +/// In order to disable niche optimizations this implementation uses [`UnsafeCell`] internally,
> +/// the upstream version however will not. So the fact that [`UnsafePinned`] contains an
> +/// [`UnsafeCell`] must not be relied on (Other than the niche blocking).

I would make this last paragraph a normal comment, I don't think we
should expose it in the docs.

> +// As opposed to the upstream Rust type this contains a `PhantomPinned`` and `UnsafeCell<T>`
> +// - `PhantomPinned` to avoid needing a `impl<T> !Unpin for UnsafePinned<T>`
> +//      Required to use the `!Unpin hack`.
> +// - `UnsafeCell<T>` instead of T to disallow niche optimizations,
> +//     which is handled in the compiler in upstream Rust
> +#[repr(transparent)]
> +pub struct UnsafePinned<T: ?Sized> {
> +    _ph: PhantomPinned,
> +    value: UnsafeCell<T>,
> +}
> +
> +impl<T> UnsafePinned<T> {
> +    /// Constructs a new instance of [`UnsafePinned`] which will wrap the specified value.
> +    ///
> +    /// All access to the inner value through `&UnsafePinned<T>` or `&mut UnsafePinned<T>` or
> +    /// `Pin<&mut UnsafePinned<T>>` requires `unsafe` code.
> +    #[inline(always)]
> +    #[must_use]
> +    pub const fn new(value: T) -> Self {
> +        UnsafePinned {
> +            value: UnsafeCell::new(value),
> +            _ph: PhantomPinned,
> +        }
> +    }
> +}
> +impl<T: ?Sized> UnsafePinned<T> {
> +    /// Get read-only access to the contents of a shared `UnsafePinned`.
> +    ///
> +    /// Note that `&UnsafePinned<T>` is read-only if `&T` is read-only. This means that if there is
> +    /// mutation of the `T`, future reads from the `*const T` returned here are UB! Use
> +    /// [`UnsafeCell`] if you also need interior mutability.

I agree with copy-pasting the docs from upstream, even though our
implementation already wraps the value in `UnsafeCell`, but I think we
should include a comment at the top of this doc that mentions this
difference. So something along the lines "In order to make replacing
this type with the upstream one, we want to have as little API
divergence as possible. Thus we don't mention the implementation detail
of `UnsafeCell` and people have to use `UnsafePinned<UnsafeCell<T>>`
instead of just `UnsafePinned<T>`." feel free to modify.

If we add that, maybe we don't need the comment about needing
`UnsafeCell` in `Opaque` after all (from my other mail).

> +    ///
> +    /// [`UnsafeCell`]: core::cell::UnsafeCell
> +    ///
> +    /// ```rust,no_build
> +    /// use kernel::types::UnsafePinned;
> +    ///
> +    /// unsafe {
> +    ///     let mut x = UnsafePinned::new(0);
> +    ///     let ptr = x.get(); // read-only pointer, assumes immutability
> +    ///     x.get_mut_unchecked().write(1);
> +    ///     ptr.read(); // UB!
> +    /// }
> +    /// ```
> +    ///
> +    /// Note that the `get_mut_unchecked` function used by this example is
> +    /// currently not implemented in the kernel implementation.
> +    #[inline(always)]
> +    #[must_use]
> +    pub const fn get(&self) -> *const T {
> +        self.value.get()
> +    }
> +
> +    /// Gets a mutable pointer to the wrapped value.
> +    ///
> +    /// The difference from `get_mut_pinned` and `get_mut_unchecked` is that this function
> +    /// accepts a raw pointer, which is useful to avoid the creation of temporary references.

You did not include the `get_mut_{pinned,unchecked}` methods, so
mentioning them here in the docs might confuse people. Do we want to
have those methods?

---
Cheers,
Benno

> +    ///
> +    /// These functions mentioned here are currently not implemented in the kernel.
> +    #[inline(always)]
> +    #[must_use]
> +    pub const fn raw_get_mut(this: *mut Self) -> *mut T {
> +        this as *mut T
> +    }
> +}
> +
> +impl<T> Wrapper<T> for UnsafePinned<T> {
> +    fn pin_init<E>(init: impl PinInit<T, E>) -> impl PinInit<Self, E> {
> +        // SAFETY: `UnsafePinned<T>` has a compatible layout to `T`.
> +        unsafe { cast_pin_init(init) }
> +    }
> +}

On 30.04.25 11:45 AM, Benno Lossin wrote:
> On Wed Apr 30, 2025 at 10:36 AM CEST, Christian Schrefl wrote:
>> `UnsafePinned<T>` is useful for cases where a value might be shared with
>> C code but not directly used by it. In particular this is added for
>> storing additional data in the `MiscDeviceRegistration` which will be
>> shared between `fops->open` and the containing struct.
>>
>> Similar to `Opaque` but guarantees that the value is always initialized
>> and that the inner value is dropped when `UnsafePinned` is dropped.
>>
>> This was originally proposed for the IRQ abstractions [0] and is also
>> useful for other where the inner data may be aliased, but is always
>> valid and automatic `Drop` is desired.
>>
>> Since then the `UnsafePinned` type was added to upstream Rust [1] by Sky
>> as a unstable feature, therefore this patch implements the subset of the
>> upstream API for the `UnsafePinned` type required for additional data in
>> `MiscDeviceRegistration` and in the implementation of the `Opaque` type.
>>
>> Some differences to the upstream type definition are required in the
>> kernel implementation, because upstream type uses some compiler changes
>> to opt out of certain optimizations, this is documented in the
>> documentation and a comment on the `UnsafePinned` type.
>>
>> The documentation on is based on the upstream rust documentation with
>> minor modifications for the kernel implementation.
>>
>> Link: https://lore.kernel.org/rust-for-linux/CAH5fLgiOASgjoYKFz6kWwzLaH07DqP2ph+3YyCDh2+gYqGpABA@mail.gmail.com [0]
>> Link: https://github.com/rust-lang/rust/pull/137043 [1]
>> Suggested-by: Alice Ryhl <aliceryhl@google.com>
>> Reviewed-by: Gerald Wisböck <gerald.wisboeck@feather.ink>
>> Co-developed-by: Sky <sky@sky9.dev>
>> Signed-off-by: Sky <sky@sky9.dev>
>> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
>> ---
>>  init/Kconfig                       |   3 +
>>  rust/kernel/lib.rs                 |   1 +
>>  rust/kernel/types.rs               |   6 ++
>>  rust/kernel/types/unsafe_pinned.rs | 115 +++++++++++++++++++++++++++++++++++++
>>  4 files changed, 125 insertions(+)
>>
>> diff --git a/init/Kconfig b/init/Kconfig
>> index 63f5974b9fa6ea3f5c92203cedd1f2f82aa468a1..727d85d2b59f555f1c33103bb78698551a41e7ca 100644
>> --- a/init/Kconfig
>> +++ b/init/Kconfig
>> @@ -140,6 +140,9 @@ config LD_CAN_USE_KEEP_IN_OVERLAY
>>  config RUSTC_HAS_COERCE_POINTEE
>>  	def_bool RUSTC_VERSION >= 108400
>>  
>> +config RUSTC_HAS_UNSAFE_PINNED
>> +	def_bool RUSTC_VERSION >= 108800
>> +
>>  config PAHOLE_VERSION
>>  	int
>>  	default $(shell,$(srctree)/scripts/pahole-version.sh $(PAHOLE))
>> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
>> index de07aadd1ff5fe46fd89517e234b97a6590c8e93..c08f0a50f1d8db95799478caa8e85558a1fcae8d 100644
>> --- a/rust/kernel/lib.rs
>> +++ b/rust/kernel/lib.rs
>> @@ -17,6 +17,7 @@
>>  #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(coerce_unsized))]
>>  #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(dispatch_from_dyn))]
>>  #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(unsize))]
>> +#![cfg_attr(CONFIG_RUSTC_HAS_UNSAFE_PINNED, feature(unsafe_pinned))]
>>  #![feature(inline_const)]
>>  #![feature(lint_reasons)]
>>  // Stable in Rust 1.82
>> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
>> index 9d0471afc9648f2973235488b441eb109069adb1..705f420fdfbc4a576de1c4546578f2f04cdf615e 100644
>> --- a/rust/kernel/types.rs
>> +++ b/rust/kernel/types.rs
>> @@ -253,6 +253,9 @@ fn drop(&mut self) {
>>  ///
>>  /// [`Opaque<T>`] is meant to be used with FFI objects that are never interpreted by Rust code.
>>  ///
>> +/// In cases where the contained data is only used by Rust, is not allowed to be
>> +/// uninitialized and automatic [`Drop`] is desired [`UnsafePinned`] should be used instead.
>> +///
>>  /// It is used to wrap structs from the C side, like for example `Opaque<bindings::mutex>`.
>>  /// It gets rid of all the usual assumptions that Rust has for a value:
>>  ///
>> @@ -578,3 +581,6 @@ pub enum Either<L, R> {
>>  /// [`NotThreadSafe`]: type@NotThreadSafe
>>  #[allow(non_upper_case_globals)]
>>  pub const NotThreadSafe: NotThreadSafe = PhantomData;
>> +
>> +mod unsafe_pinned;
>> +pub use unsafe_pinned::UnsafePinned;
>> diff --git a/rust/kernel/types/unsafe_pinned.rs b/rust/kernel/types/unsafe_pinned.rs
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..5a200aac30792bf71098087aee0fd9d2d51c468f
>> --- /dev/null
>> +++ b/rust/kernel/types/unsafe_pinned.rs
>> @@ -0,0 +1,115 @@
>> +// SPDX-License-Identifier: Apache-2.0 OR MIT
>> +
>> +//! The contents of this file partially come from the Rust standard library, hosted in
>> +//! the <https://github.com/rust-lang/rust> repository, licensed under
>> +//! "Apache-2.0 OR MIT" and adapted for kernel use. For copyright details,
>> +//! see <https://github.com/rust-lang/rust/blob/master/COPYRIGHT>.
>> +//!
>> +//! This file provides a implementation / polyfill of a subset of the upstream
>> +//! rust `UnsafePinned` type.
>> +
>> +use core::{cell::UnsafeCell, marker::PhantomPinned};
>> +use pin_init::{cast_pin_init, PinInit, Wrapper};
>> +
>> +/// This type provides a way to opt-out of typical aliasing rules;
>> +/// specifically, `&mut UnsafePinned<T>` is not guaranteed to be a unique pointer.
>> +///
>> +/// However, even if you define your type like `pub struct Wrapper(UnsafePinned<...>)`, it is still
>> +/// very risky to have an `&mut Wrapper` that aliases anything else. Many functions that work
>> +/// generically on `&mut T` assume that the memory that stores `T` is uniquely owned (such as
>> +/// `mem::swap`). In other words, while having aliasing with `&mut Wrapper` is not immediate
>> +/// Undefined Behavior, it is still unsound to expose such a mutable reference to code you do not
>> +/// control! Techniques such as pinning via [`Pin`](core::pin::Pin) are needed to ensure soundness.
>> +///
>> +/// Similar to [`UnsafeCell`], [`UnsafePinned`] will not usually show up in
>> +/// the public API of a library. It is an internal implementation detail of libraries that need to
>> +/// support aliasing mutable references.
>> +///
>> +/// Further note that this does *not* lift the requirement that shared references must be read-only!
>> +/// Use [`UnsafeCell`] for that.
>> +///
>> +/// This type blocks niches the same way [`UnsafeCell`] does.
>> +///
>> +/// # Kernel implementation notes
>> +///
>> +/// This implementation works because of the "`!Unpin` hack" in rustc, which allows (some kinds of)
>> +/// mutual aliasing of `!Unpin` types. This hack might be removed at some point, after which only
>> +/// the `core::pin::UnsafePinned` type will allow this behavior. In order to simplify the migration
>> +/// to future rust versions only this polyfill of this type should be used when this behavior is
>> +/// required.
>> +///
>> +/// In order to disable niche optimizations this implementation uses [`UnsafeCell`] internally,
>> +/// the upstream version however will not. So the fact that [`UnsafePinned`] contains an
>> +/// [`UnsafeCell`] must not be relied on (Other than the niche blocking).
> 
> I would make this last paragraph a normal comment, I don't think we
> should expose it in the docs.

I added this as docs since I wanted it to be a bit more visible,
but I can replace the comment text (about `UnsafeCell`) with this paragraph
and drop it from the docs if you want.
 
>> +// As opposed to the upstream Rust type this contains a `PhantomPinned`` and `UnsafeCell<T>`
>> +// - `PhantomPinned` to avoid needing a `impl<T> !Unpin for UnsafePinned<T>`
>> +//      Required to use the `!Unpin hack`.
>> +// - `UnsafeCell<T>` instead of T to disallow niche optimizations,
>> +//     which is handled in the compiler in upstream Rust
>> +#[repr(transparent)]
>> +pub struct UnsafePinned<T: ?Sized> {
>> +    _ph: PhantomPinned,
>> +    value: UnsafeCell<T>,
>> +}
>> +
>> +impl<T> UnsafePinned<T> {
>> +    /// Constructs a new instance of [`UnsafePinned`] which will wrap the specified value.
>> +    ///
>> +    /// All access to the inner value through `&UnsafePinned<T>` or `&mut UnsafePinned<T>` or
>> +    /// `Pin<&mut UnsafePinned<T>>` requires `unsafe` code.
>> +    #[inline(always)]
>> +    #[must_use]
>> +    pub const fn new(value: T) -> Self {
>> +        UnsafePinned {
>> +            value: UnsafeCell::new(value),
>> +            _ph: PhantomPinned,
>> +        }
>> +    }
>> +}
>> +impl<T: ?Sized> UnsafePinned<T> {
>> +    /// Get read-only access to the contents of a shared `UnsafePinned`.
>> +    ///
>> +    /// Note that `&UnsafePinned<T>` is read-only if `&T` is read-only. This means that if there is
>> +    /// mutation of the `T`, future reads from the `*const T` returned here are UB! Use
>> +    /// [`UnsafeCell`] if you also need interior mutability.
> 
> I agree with copy-pasting the docs from upstream, even though our
> implementation already wraps the value in `UnsafeCell`, but I think we
> should include a comment at the top of this doc that mentions this
> difference. So something along the lines "In order to make replacing
> this type with the upstream one, we want to have as little API
> divergence as possible. Thus we don't mention the implementation detail
> of `UnsafeCell` and people have to use `UnsafePinned<UnsafeCell<T>>`
> instead of just `UnsafePinned<T>`." feel free to modify.
> 

I already wrote about this in comments (and documentation in this version)
on the `UnsafePinned` type definition.

I'm not sure where exactly we want to have this, but I think having it
at the top of the file and on the type definition is a bit redundant.

>
> If we add that, maybe we don't need the comment about needing
> `UnsafeCell` in `Opaque` after all (from my other mail).

I think I should still add a comment there.

> 
>> +    ///
>> +    /// [`UnsafeCell`]: core::cell::UnsafeCell
>> +    ///
>> +    /// ```rust,no_build
>> +    /// use kernel::types::UnsafePinned;
>> +    ///
>> +    /// unsafe {
>> +    ///     let mut x = UnsafePinned::new(0);
>> +    ///     let ptr = x.get(); // read-only pointer, assumes immutability
>> +    ///     x.get_mut_unchecked().write(1);
>> +    ///     ptr.read(); // UB!
>> +    /// }
>> +    /// ```
>> +    ///
>> +    /// Note that the `get_mut_unchecked` function used by this example is
>> +    /// currently not implemented in the kernel implementation.
>> +    #[inline(always)]
>> +    #[must_use]
>> +    pub const fn get(&self) -> *const T {
>> +        self.value.get()
>> +    }
>> +
>> +    /// Gets a mutable pointer to the wrapped value.
>> +    ///
>> +    /// The difference from `get_mut_pinned` and `get_mut_unchecked` is that this function
>> +    /// accepts a raw pointer, which is useful to avoid the creation of temporary references.
> 
> You did not include the `get_mut_{pinned,unchecked}` methods, so
> mentioning them here in the docs might confuse people. Do we want to
> have those methods?

I only included the functions that we needed for `Opaque` and my
`miscdevice' patches. I think these functions should only be added
once they have a user. That's why I wrote the next sentence in the
documents.

Should I handle this differently?

It should be a really simple patch to add these functions and I can
do that if someone needs them or I can just include them in this
patch set.

Cheers
Christian

> 
> ---
> Cheers,
> Benno
> 
>> +    ///
>> +    /// These functions mentioned here are currently not implemented in the kernel.
>> +    #[inline(always)]
>> +    #[must_use]
>> +    pub const fn raw_get_mut(this: *mut Self) -> *mut T {
>> +        this as *mut T
>> +    }
>> +}
>> +
>> +impl<T> Wrapper<T> for UnsafePinned<T> {
>> +    fn pin_init<E>(init: impl PinInit<T, E>) -> impl PinInit<Self, E> {
>> +        // SAFETY: `UnsafePinned<T>` has a compatible layout to `T`.
>> +        unsafe { cast_pin_init(init) }
>> +    }
>> +}
> 

On Wed Apr 30, 2025 at 7:30 PM CEST, Christian Schrefl wrote:
> On 30.04.25 11:45 AM, Benno Lossin wrote:
>> On Wed Apr 30, 2025 at 10:36 AM CEST, Christian Schrefl wrote:
>>> +/// This implementation works because of the "`!Unpin` hack" in rustc, which allows (some kinds of)
>>> +/// mutual aliasing of `!Unpin` types. This hack might be removed at some point, after which only
>>> +/// the `core::pin::UnsafePinned` type will allow this behavior. In order to simplify the migration
>>> +/// to future rust versions only this polyfill of this type should be used when this behavior is
>>> +/// required.
>>> +///
>>> +/// In order to disable niche optimizations this implementation uses [`UnsafeCell`] internally,
>>> +/// the upstream version however will not. So the fact that [`UnsafePinned`] contains an
>>> +/// [`UnsafeCell`] must not be relied on (Other than the niche blocking).
>> 
>> I would make this last paragraph a normal comment, I don't think we
>> should expose it in the docs.
>
> I added this as docs since I wanted it to be a bit more visible,
> but I can replace the comment text (about `UnsafeCell`) with this paragraph
> and drop it from the docs if you want.

I think we shouldn't talk about these implementation details in the
docs.

>>> +// As opposed to the upstream Rust type this contains a `PhantomPinned`` and `UnsafeCell<T>`
>>> +// - `PhantomPinned` to avoid needing a `impl<T> !Unpin for UnsafePinned<T>`
>>> +//      Required to use the `!Unpin hack`.
>>> +// - `UnsafeCell<T>` instead of T to disallow niche optimizations,
>>> +//     which is handled in the compiler in upstream Rust
>>> +#[repr(transparent)]
>>> +pub struct UnsafePinned<T: ?Sized> {
>>> +    _ph: PhantomPinned,
>>> +    value: UnsafeCell<T>,
>>> +}
>>> +
>>> +impl<T> UnsafePinned<T> {
>>> +    /// Constructs a new instance of [`UnsafePinned`] which will wrap the specified value.
>>> +    ///
>>> +    /// All access to the inner value through `&UnsafePinned<T>` or `&mut UnsafePinned<T>` or
>>> +    /// `Pin<&mut UnsafePinned<T>>` requires `unsafe` code.
>>> +    #[inline(always)]
>>> +    #[must_use]
>>> +    pub const fn new(value: T) -> Self {
>>> +        UnsafePinned {
>>> +            value: UnsafeCell::new(value),
>>> +            _ph: PhantomPinned,
>>> +        }
>>> +    }
>>> +}
>>> +impl<T: ?Sized> UnsafePinned<T> {
>>> +    /// Get read-only access to the contents of a shared `UnsafePinned`.
>>> +    ///
>>> +    /// Note that `&UnsafePinned<T>` is read-only if `&T` is read-only. This means that if there is
>>> +    /// mutation of the `T`, future reads from the `*const T` returned here are UB! Use
>>> +    /// [`UnsafeCell`] if you also need interior mutability.
>> 
>> I agree with copy-pasting the docs from upstream, even though our
>> implementation already wraps the value in `UnsafeCell`, but I think we
>> should include a comment at the top of this doc that mentions this
>> difference. So something along the lines "In order to make replacing
>> this type with the upstream one, we want to have as little API
>> divergence as possible. Thus we don't mention the implementation detail
>> of `UnsafeCell` and people have to use `UnsafePinned<UnsafeCell<T>>`
>> instead of just `UnsafePinned<T>`." feel free to modify.
>> 
>
> I already wrote about this in comments (and documentation in this version)
> on the `UnsafePinned` type definition.
>
> I'm not sure where exactly we want to have this, but I think having it
> at the top of the file and on the type definition is a bit redundant.

Sure.

>>> +    /// Gets a mutable pointer to the wrapped value.
>>> +    ///
>>> +    /// The difference from `get_mut_pinned` and `get_mut_unchecked` is that this function
>>> +    /// accepts a raw pointer, which is useful to avoid the creation of temporary references.
>> 
>> You did not include the `get_mut_{pinned,unchecked}` methods, so
>> mentioning them here in the docs might confuse people. Do we want to
>> have those methods?
>
> I only included the functions that we needed for `Opaque` and my
> `miscdevice' patches. I think these functions should only be added
> once they have a user. That's why I wrote the next sentence in the
> documents.
>
> Should I handle this differently?
>
> It should be a really simple patch to add these functions and I can
> do that if someone needs them or I can just include them in this
> patch set.

Then I'd remove the sentence referencing the functions you don't add.

---
Cheers,
Benno

On 01.05.25 8:51 PM, Benno Lossin wrote:
> On Wed Apr 30, 2025 at 7:30 PM CEST, Christian Schrefl wrote:
>> On 30.04.25 11:45 AM, Benno Lossin wrote:
>>> On Wed Apr 30, 2025 at 10:36 AM CEST, Christian Schrefl wrote:
>>>> +/// This implementation works because of the "`!Unpin` hack" in rustc, which allows (some kinds of)
>>>> +/// mutual aliasing of `!Unpin` types. This hack might be removed at some point, after which only
>>>> +/// the `core::pin::UnsafePinned` type will allow this behavior. In order to simplify the migration
>>>> +/// to future rust versions only this polyfill of this type should be used when this behavior is
>>>> +/// required.
>>>> +///
>>>> +/// In order to disable niche optimizations this implementation uses [`UnsafeCell`] internally,
>>>> +/// the upstream version however will not. So the fact that [`UnsafePinned`] contains an
>>>> +/// [`UnsafeCell`] must not be relied on (Other than the niche blocking).
>>>
>>> I would make this last paragraph a normal comment, I don't think we
>>> should expose it in the docs.
>>
>> I added this as docs since I wanted it to be a bit more visible,
>> but I can replace the comment text (about `UnsafeCell`) with this paragraph
>> and drop it from the docs if you want.
> 
> I think we shouldn't talk about these implementation details in the
> docs.

Alright, what do you think of:

// As opposed to the upstream Rust type this contains a `PhantomPinned`` and `UnsafeCell<T>`
// - `PhantomPinned` to avoid needing a `impl<T> !Unpin for UnsafePinned<T>`
//   Required to use the `!Unpin hack`.
// - In order to disable niche optimizations this implementation uses `UnsafeCell` internally,
//   the upstream version however currently does not. This will most likely change in the future
//   but for now we don't expose this in the documentation, since adding the guarantee is simpler
//   than removing it. Meaning that for now the fact that `UnsafePinned` contains an `UnsafeCell`
//   must not be relied on (Other than the niche blocking).

> 
>>>> +// As opposed to the upstream Rust type this contains a `PhantomPinned`` and `UnsafeCell<T>`
>>>> +// - `PhantomPinned` to avoid needing a `impl<T> !Unpin for UnsafePinned<T>`
>>>> +//      Required to use the `!Unpin hack`.
>>>> +// - `UnsafeCell<T>` instead of T to disallow niche optimizations,
>>>> +//     which is handled in the compiler in upstream Rust
>>>> +#[repr(transparent)]
>>>> +pub struct UnsafePinned<T: ?Sized> {
>>>> +    _ph: PhantomPinned,
>>>> +    value: UnsafeCell<T>,
>>>> +}
>>>> +
>>>> +impl<T> UnsafePinned<T> {
>>>> +    /// Constructs a new instance of [`UnsafePinned`] which will wrap the specified value.
>>>> +    ///
>>>> +    /// All access to the inner value through `&UnsafePinned<T>` or `&mut UnsafePinned<T>` or
>>>> +    /// `Pin<&mut UnsafePinned<T>>` requires `unsafe` code.
>>>> +    #[inline(always)]
>>>> +    #[must_use]
>>>> +    pub const fn new(value: T) -> Self {
>>>> +        UnsafePinned {
>>>> +            value: UnsafeCell::new(value),
>>>> +            _ph: PhantomPinned,
>>>> +        }
>>>> +    }
>>>> +}
>>>> +impl<T: ?Sized> UnsafePinned<T> {
>>>> +    /// Get read-only access to the contents of a shared `UnsafePinned`.
>>>> +    ///
>>>> +    /// Note that `&UnsafePinned<T>` is read-only if `&T` is read-only. This means that if there is
>>>> +    /// mutation of the `T`, future reads from the `*const T` returned here are UB! Use
>>>> +    /// [`UnsafeCell`] if you also need interior mutability.
>>>
>>> I agree with copy-pasting the docs from upstream, even though our
>>> implementation already wraps the value in `UnsafeCell`, but I think we
>>> should include a comment at the top of this doc that mentions this
>>> difference. So something along the lines "In order to make replacing
>>> this type with the upstream one, we want to have as little API
>>> divergence as possible. Thus we don't mention the implementation detail
>>> of `UnsafeCell` and people have to use `UnsafePinned<UnsafeCell<T>>`
>>> instead of just `UnsafePinned<T>`." feel free to modify.
>>>
>>
>> I already wrote about this in comments (and documentation in this version)
>> on the `UnsafePinned` type definition.
>>
>> I'm not sure where exactly we want to have this, but I think having it
>> at the top of the file and on the type definition is a bit redundant.
> 
> Sure.

I'll add the following sentence to the end of the module documentation:

For details on the difference to the upstream implementation see the
comment on the [`UnsafePinned`] struct definition.

> 
>>>> +    /// Gets a mutable pointer to the wrapped value.
>>>> +    ///
>>>> +    /// The difference from `get_mut_pinned` and `get_mut_unchecked` is that this function
>>>> +    /// accepts a raw pointer, which is useful to avoid the creation of temporary references.
>>>
>>> You did not include the `get_mut_{pinned,unchecked}` methods, so
>>> mentioning them here in the docs might confuse people. Do we want to
>>> have those methods?
>>
>> I only included the functions that we needed for `Opaque` and my
>> `miscdevice' patches. I think these functions should only be added
>> once they have a user. That's why I wrote the next sentence in the
>> documents.
>>
>> Should I handle this differently?
>>
>> It should be a really simple patch to add these functions and I can
>> do that if someone needs them or I can just include them in this
>> patch set.
> 
> Then I'd remove the sentence referencing the functions you don't add.

Alright

Cheers
Christian

On Thu May 1, 2025 at 9:11 PM CEST, Christian Schrefl wrote:
> On 01.05.25 8:51 PM, Benno Lossin wrote:
>> On Wed Apr 30, 2025 at 7:30 PM CEST, Christian Schrefl wrote:
>>> On 30.04.25 11:45 AM, Benno Lossin wrote:
>>>> On Wed Apr 30, 2025 at 10:36 AM CEST, Christian Schrefl wrote:
>>>>> +/// This implementation works because of the "`!Unpin` hack" in rustc, which allows (some kinds of)
>>>>> +/// mutual aliasing of `!Unpin` types. This hack might be removed at some point, after which only
>>>>> +/// the `core::pin::UnsafePinned` type will allow this behavior. In order to simplify the migration
>>>>> +/// to future rust versions only this polyfill of this type should be used when this behavior is
>>>>> +/// required.
>>>>> +///
>>>>> +/// In order to disable niche optimizations this implementation uses [`UnsafeCell`] internally,
>>>>> +/// the upstream version however will not. So the fact that [`UnsafePinned`] contains an
>>>>> +/// [`UnsafeCell`] must not be relied on (Other than the niche blocking).
>>>>
>>>> I would make this last paragraph a normal comment, I don't think we
>>>> should expose it in the docs.
>>>
>>> I added this as docs since I wanted it to be a bit more visible,
>>> but I can replace the comment text (about `UnsafeCell`) with this paragraph
>>> and drop it from the docs if you want.
>> 
>> I think we shouldn't talk about these implementation details in the
>> docs.
>
> Alright, what do you think of:
>
> // As opposed to the upstream Rust type this contains a `PhantomPinned`` and `UnsafeCell<T>`

There are two '`' after PhantomPinned.

> // - `PhantomPinned` to avoid needing a `impl<T> !Unpin for UnsafePinned<T>`

s/ a / an /

I find the phrasing 'avoid needing <negative impl>' a bit weird, I'd
just say "`PhantomPinned` to ensure the struct always is `!Unpin` and
thus enables the `!Unpin` hack".

If you have a link to somewhere that explains that hack, then I'd also
put it there. I forgot if it's written down somewhere.

> //   Required to use the `!Unpin hack`.
> // - In order to disable niche optimizations this implementation uses `UnsafeCell` internally,
> //   the upstream version however currently does not. This will most likely change in the future
> //   but for now we don't expose this in the documentation, since adding the guarantee is simpler
> //   than removing it. Meaning that for now the fact that `UnsafePinned` contains an `UnsafeCell`
> //   must not be relied on (Other than the niche blocking).

---
Cheers,
Benno

[cc Ralf]

On 02.05.25 12:51 AM, Benno Lossin wrote:
> On Thu May 1, 2025 at 9:11 PM CEST, Christian Schrefl wrote:
>> On 01.05.25 8:51 PM, Benno Lossin wrote:
>>> On Wed Apr 30, 2025 at 7:30 PM CEST, Christian Schrefl wrote:
>>>> On 30.04.25 11:45 AM, Benno Lossin wrote:
>>>>> On Wed Apr 30, 2025 at 10:36 AM CEST, Christian Schrefl wrote:
>>>>>> +/// This implementation works because of the "`!Unpin` hack" in rustc, which allows (some kinds of)
>>>>>> +/// mutual aliasing of `!Unpin` types. This hack might be removed at some point, after which only
>>>>>> +/// the `core::pin::UnsafePinned` type will allow this behavior. In order to simplify the migration
>>>>>> +/// to future rust versions only this polyfill of this type should be used when this behavior is
>>>>>> +/// required.
>>>>>> +///
>>>>>> +/// In order to disable niche optimizations this implementation uses [`UnsafeCell`] internally,
>>>>>> +/// the upstream version however will not. So the fact that [`UnsafePinned`] contains an
>>>>>> +/// [`UnsafeCell`] must not be relied on (Other than the niche blocking).
>>>>>
>>>>> I would make this last paragraph a normal comment, I don't think we
>>>>> should expose it in the docs.
>>>>
>>>> I added this as docs since I wanted it to be a bit more visible,
>>>> but I can replace the comment text (about `UnsafeCell`) with this paragraph
>>>> and drop it from the docs if you want.
>>>
>>> I think we shouldn't talk about these implementation details in the
>>> docs.
>>
>> Alright, what do you think of:
>>
>> // As opposed to the upstream Rust type this contains a `PhantomPinned`` and `UnsafeCell<T>`
> 
> There are two '`' after PhantomPinned.
> 
>> // - `PhantomPinned` to avoid needing a `impl<T> !Unpin for UnsafePinned<T>`
> 
> s/ a / an /
> 
> I find the phrasing 'avoid needing <negative impl>' a bit weird, I'd
> just say "`PhantomPinned` to ensure the struct always is `!Unpin` and
> thus enables the `!Unpin` hack".

Thanks I'll use that.

> 
> If you have a link to somewhere that explains that hack, then I'd also
> put it there. I forgot if it's written down somewhere.

I haven't found anything that describes the hack in detail.
From what I understand its a combination of disabling `noalias`
[0] (this PR enables it for `Unpin` types) and disabling 
`dereferencable` [1] on `&mut !Unpin` types.
Related rust issue about this [2].

Maybe Alice, Ralf or someone else form the rust side can provide
a better reference?

[0]: https://github.com/rust-lang/rust/pull/82834
[1]: https://github.com/rust-lang/rust/pull/106180
[2]: https://github.com/rust-lang/rust/issues/63818

Cheers
Christian

>> //   Required to use the `!Unpin hack`.
>> // - In order to disable niche optimizations this implementation uses `UnsafeCell` internally,
>> //   the upstream version however currently does not. This will most likely change in the future
>> //   but for now we don't expose this in the documentation, since adding the guarantee is simpler
>> //   than removing it. Meaning that for now the fact that `UnsafePinned` contains an `UnsafeCell`
>> //   must not be relied on (Other than the niche blocking).

Hi all,

>> If you have a link to somewhere that explains that hack, then I'd also
>> put it there. I forgot if it's written down somewhere.
> 
> I haven't found anything that describes the hack in detail.
>  From what I understand its a combination of disabling `noalias`
> [0] (this PR enables it for `Unpin` types) and disabling
> `dereferencable` [1] on `&mut !Unpin` types.
> Related rust issue about this [2].
> 
> Maybe Alice, Ralf or someone else form the rust side can provide
> a better reference?

The `!Unpick` hack simply removes `noalias` and `dereferenceable` from `&mut 
!Unpin` and `Box<!Unpin>` types (similar to how we already remove  `noalias` and 
`dereferenceable` from `&!Freeze` types). This is not a stable guarantee and 
will probably change in future version of the compiler.

Kind regards,
Ralf

On Fri, May 02, 2025 at 02:08:13AM +0200, Christian Schrefl wrote:
> [cc Ralf]
> 
> On 02.05.25 12:51 AM, Benno Lossin wrote:
> > On Thu May 1, 2025 at 9:11 PM CEST, Christian Schrefl wrote:
> >> On 01.05.25 8:51 PM, Benno Lossin wrote:
> >>> On Wed Apr 30, 2025 at 7:30 PM CEST, Christian Schrefl wrote:
> >>>> On 30.04.25 11:45 AM, Benno Lossin wrote:
> >>>>> On Wed Apr 30, 2025 at 10:36 AM CEST, Christian Schrefl wrote:
> >>>>>> +/// This implementation works because of the "`!Unpin` hack" in rustc, which allows (some kinds of)
> >>>>>> +/// mutual aliasing of `!Unpin` types. This hack might be removed at some point, after which only
> >>>>>> +/// the `core::pin::UnsafePinned` type will allow this behavior. In order to simplify the migration
> >>>>>> +/// to future rust versions only this polyfill of this type should be used when this behavior is
> >>>>>> +/// required.
> >>>>>> +///
> >>>>>> +/// In order to disable niche optimizations this implementation uses [`UnsafeCell`] internally,
> >>>>>> +/// the upstream version however will not. So the fact that [`UnsafePinned`] contains an
> >>>>>> +/// [`UnsafeCell`] must not be relied on (Other than the niche blocking).
> >>>>>
> >>>>> I would make this last paragraph a normal comment, I don't think we
> >>>>> should expose it in the docs.
> >>>>
> >>>> I added this as docs since I wanted it to be a bit more visible,
> >>>> but I can replace the comment text (about `UnsafeCell`) with this paragraph
> >>>> and drop it from the docs if you want.
> >>>
> >>> I think we shouldn't talk about these implementation details in the
> >>> docs.
> >>
> >> Alright, what do you think of:
> >>
> >> // As opposed to the upstream Rust type this contains a `PhantomPinned`` and `UnsafeCell<T>`
> > 
> > There are two '`' after PhantomPinned.
> > 
> >> // - `PhantomPinned` to avoid needing a `impl<T> !Unpin for UnsafePinned<T>`
> > 
> > s/ a / an /
> > 
> > I find the phrasing 'avoid needing <negative impl>' a bit weird, I'd
> > just say "`PhantomPinned` to ensure the struct always is `!Unpin` and
> > thus enables the `!Unpin` hack".
> 
> Thanks I'll use that.
> 
> > 
> > If you have a link to somewhere that explains that hack, then I'd also
> > put it there. I forgot if it's written down somewhere.
> 
> I haven't found anything that describes the hack in detail.
> From what I understand its a combination of disabling `noalias`
> [0] (this PR enables it for `Unpin` types) and disabling 
> `dereferencable` [1] on `&mut !Unpin` types.
> Related rust issue about this [2].
> 
> Maybe Alice, Ralf or someone else form the rust side can provide
> a better reference?
> 
> [0]: https://github.com/rust-lang/rust/pull/82834
> [1]: https://github.com/rust-lang/rust/pull/106180
> [2]: https://github.com/rust-lang/rust/issues/63818

I wrote this a long time ago:
https://gist.github.com/Darksonn/1567538f56af1a8038ecc3c664a42462

But it doesn't really take the angle of explaining the !Unpin hack. It's
more of an early doc arguing for having an UnsafePinned type.

Alice

On Wed, Apr 30, 2025 at 10:36:11AM +0200, Christian Schrefl wrote:
> `UnsafePinned<T>` is useful for cases where a value might be shared with
> C code but not directly used by it. In particular this is added for
> storing additional data in the `MiscDeviceRegistration` which will be
> shared between `fops->open` and the containing struct.
> 
> Similar to `Opaque` but guarantees that the value is always initialized
> and that the inner value is dropped when `UnsafePinned` is dropped.
> 
> This was originally proposed for the IRQ abstractions [0] and is also
> useful for other where the inner data may be aliased, but is always
> valid and automatic `Drop` is desired.
> 
> Since then the `UnsafePinned` type was added to upstream Rust [1] by Sky
> as a unstable feature, therefore this patch implements the subset of the
> upstream API for the `UnsafePinned` type required for additional data in
> `MiscDeviceRegistration` and in the implementation of the `Opaque` type.
> 
> Some differences to the upstream type definition are required in the
> kernel implementation, because upstream type uses some compiler changes
> to opt out of certain optimizations, this is documented in the
> documentation and a comment on the `UnsafePinned` type.
> 
> The documentation on is based on the upstream rust documentation with
> minor modifications for the kernel implementation.
> 
> Link: https://lore.kernel.org/rust-for-linux/CAH5fLgiOASgjoYKFz6kWwzLaH07DqP2ph+3YyCDh2+gYqGpABA@mail.gmail.com [0]
> Link: https://github.com/rust-lang/rust/pull/137043 [1]
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Reviewed-by: Gerald Wisböck <gerald.wisboeck@feather.ink>
> Co-developed-by: Sky <sky@sky9.dev>
> Signed-off-by: Sky <sky@sky9.dev>
> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>

> +config RUSTC_HAS_UNSAFE_PINNED
> +	def_bool RUSTC_VERSION >= 108800

> +#![cfg_attr(CONFIG_RUSTC_HAS_UNSAFE_PINNED, feature(unsafe_pinned))]

Actually, I missed this on the first look. Where is this feature used?
You only have a re-implementation right now.

Alice

On 30.04.25 11:19 AM, Alice Ryhl wrote:
> On Wed, Apr 30, 2025 at 10:36:11AM +0200, Christian Schrefl wrote:
>> `UnsafePinned<T>` is useful for cases where a value might be shared with
>> C code but not directly used by it. In particular this is added for
>> storing additional data in the `MiscDeviceRegistration` which will be
>> shared between `fops->open` and the containing struct.
>>
>> Similar to `Opaque` but guarantees that the value is always initialized
>> and that the inner value is dropped when `UnsafePinned` is dropped.
>>
>> This was originally proposed for the IRQ abstractions [0] and is also
>> useful for other where the inner data may be aliased, but is always
>> valid and automatic `Drop` is desired.
>>
>> Since then the `UnsafePinned` type was added to upstream Rust [1] by Sky
>> as a unstable feature, therefore this patch implements the subset of the
>> upstream API for the `UnsafePinned` type required for additional data in
>> `MiscDeviceRegistration` and in the implementation of the `Opaque` type.
>>
>> Some differences to the upstream type definition are required in the
>> kernel implementation, because upstream type uses some compiler changes
>> to opt out of certain optimizations, this is documented in the
>> documentation and a comment on the `UnsafePinned` type.
>>
>> The documentation on is based on the upstream rust documentation with
>> minor modifications for the kernel implementation.
>>
>> Link: https://lore.kernel.org/rust-for-linux/CAH5fLgiOASgjoYKFz6kWwzLaH07DqP2ph+3YyCDh2+gYqGpABA@mail.gmail.com [0]
>> Link: https://github.com/rust-lang/rust/pull/137043 [1]
>> Suggested-by: Alice Ryhl <aliceryhl@google.com>
>> Reviewed-by: Gerald Wisböck <gerald.wisboeck@feather.ink>
>> Co-developed-by: Sky <sky@sky9.dev>
>> Signed-off-by: Sky <sky@sky9.dev>
>> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
> 
>> +config RUSTC_HAS_UNSAFE_PINNED
>> +	def_bool RUSTC_VERSION >= 108800
> 
>> +#![cfg_attr(CONFIG_RUSTC_HAS_UNSAFE_PINNED, feature(unsafe_pinned))]
> 
> Actually, I missed this on the first look. Where is this feature used?
> You only have a re-implementation right now.
Was just a leftover from the previous version, removing the feature and
config flag (to be reintroduced once we want to use the upstream version).

Cheers
Christian

On Wed, Apr 30, 2025 at 10:36:11AM +0200, Christian Schrefl wrote:
> `UnsafePinned<T>` is useful for cases where a value might be shared with
> C code but not directly used by it. In particular this is added for
> storing additional data in the `MiscDeviceRegistration` which will be
> shared between `fops->open` and the containing struct.
> 
> Similar to `Opaque` but guarantees that the value is always initialized
> and that the inner value is dropped when `UnsafePinned` is dropped.
> 
> This was originally proposed for the IRQ abstractions [0] and is also
> useful for other where the inner data may be aliased, but is always
> valid and automatic `Drop` is desired.
> 
> Since then the `UnsafePinned` type was added to upstream Rust [1] by Sky
> as a unstable feature, therefore this patch implements the subset of the
> upstream API for the `UnsafePinned` type required for additional data in
> `MiscDeviceRegistration` and in the implementation of the `Opaque` type.
> 
> Some differences to the upstream type definition are required in the
> kernel implementation, because upstream type uses some compiler changes
> to opt out of certain optimizations, this is documented in the
> documentation and a comment on the `UnsafePinned` type.
> 
> The documentation on is based on the upstream rust documentation with
> minor modifications for the kernel implementation.
> 
> Link: https://lore.kernel.org/rust-for-linux/CAH5fLgiOASgjoYKFz6kWwzLaH07DqP2ph+3YyCDh2+gYqGpABA@mail.gmail.com [0]
> Link: https://github.com/rust-lang/rust/pull/137043 [1]
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Reviewed-by: Gerald Wisböck <gerald.wisboeck@feather.ink>
> Co-developed-by: Sky <sky@sky9.dev>
> Signed-off-by: Sky <sky@sky9.dev>
> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>