[PATCH v4 0/3] rust: add `UnsafePinned` type

[PATCH v4 0/3] rust: add `UnsafePinned` type

[Christian Schrefl]

This version now only has the kernel implementation without the
config flag for using the upstream version. Additionally now
commits for using `UnsafePinned` in `Opaque` were added.

Checkpatch warns about `rust/kernel/types/unsafe_pinned.rs`
missing a MAINTAINERS entry. This should be fine because it
will be included in the RUST entry.

This patchset is based on the pin-init-next branch.

Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
---
Changes in v4:
- Reworked `UnsafePinned::add` doctest to not use get_mut_unchecked.
- Link to v3: https://lore.kernel.org/r/20250510-rust_unsafe_pinned-v3-0-57ce151123f9@gmail.com

Changes in v3:
- Dropped CONFIG_RUSTC_HAS_UNSAFE_PINNED and feature(unsafe_pinned) (Alice)
- Add comment to `Opaque` reasoning about included `UnsafeCell` (Benno)
- Small changes in commit message of Patch 3 (Benno)
- Removed docs mentioning not included functions (Benno)
- Removed docs mentioning implementation details and added that as
    comment instead (Benno)
- Link to v2: https://lore.kernel.org/r/20250430-rust_unsafe_pinned-v2-0-fc8617a74024@gmail.com

Changes in v2:
- Expanded `UnsafePinned` documentation to describe differences
    with upstream rust implementation.
- Removed config flag for using upstream `UnsafePinned` type.
- Add patch implementing `Wrapper` for `Opaque`
- Add patch for using `UnsafePinned` internally in `Opaque`
- Link to v1: https://lore.kernel.org/r/20250418-rust_unsafe_pinned-v1-1-c4c7558399f8@gmail.com

---
Christian Schrefl (3):
      rust: add UnsafePinned type
      rust: implement `Wrapper<T>` for `Opaque<T>`
      rust: use `UnsafePinned` in the implementation of `Opaque`

 rust/kernel/revocable.rs           |   2 +
 rust/kernel/types.rs               |  45 ++++++++-------
 rust/kernel/types/unsafe_pinned.rs | 111 +++++++++++++++++++++++++++++++++++++
 3 files changed, 138 insertions(+), 20 deletions(-)
---
base-commit: 9de1a293c8ece00d226b21a35751ec178be2a9fa
change-id: 20250418-rust_unsafe_pinned-891dce27418d

Best regards,
-- 
Christian Schrefl <chrisi.schrefl@gmail.com>

[PATCH v4 1/3] rust: add UnsafePinned type

[Christian Schrefl]

`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>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Co-developed-by: Sky <sky@sky9.dev>
Signed-off-by: Sky <sky@sky9.dev>
Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
---
 rust/kernel/types.rs               |   6 ++
 rust/kernel/types/unsafe_pinned.rs | 111 +++++++++++++++++++++++++++++++++++++
 2 files changed, 117 insertions(+)

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..315248cb88c089239bd672c889b5107060175ec3
--- /dev/null
+++ b/rust/kernel/types/unsafe_pinned.rs
@@ -0,0 +1,111 @@
+// 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. For details on the difference to the upstream
+//! implementation see the comment on the [`UnsafePinned`] struct definition.
+
+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.
+//
+// As opposed to the upstream Rust type this contains a `PhantomPinned` and `UnsafeCell<T>`
+// - `PhantomPinned` to ensure the struct always is `!Unpin` and thus enables the `!Unpin` hack.
+//   This causes the LLVM `noalias` and `dereferenceable` attributes to be removed from
+//   `&mut !Unpin` types.
+// - 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).
+//   See this Rust tracking issue: https://github.com/rust-lang/rust/issues/137750
+#[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_run
+    /// use kernel::types::UnsafePinned;
+    ///
+    /// unsafe {
+    ///     let mut x = UnsafePinned::new(0);
+    ///     let ptr = x.get(); // read-only pointer, assumes immutability
+    ///     # // Upstream Rust uses `UnsafePinned::get_mut_unchecked` here.
+    ///     UnsafePinned::raw_get_mut(&raw mut x).write(1);
+    ///     ptr.read(); // UB!
+    /// }
+    /// ```
+    #[inline(always)]
+    #[must_use]
+    pub const fn get(&self) -> *const T {
+        self.value.get()
+    }
+
+    /// Gets a mutable pointer to the wrapped value.
+    #[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

[PATCH v4 2/3] rust: implement `Wrapper<T>` for `Opaque<T>`

[Christian Schrefl]

Moves the implementation for `pin-init` from an associated function
to the trait function of the `Wrapper` trait and extends the
implementation to support pin-initializers with error types.

This allows to declare functions that are generic over `Wrapper`
types.

Adds a use for the `Wrapper` trait in `revocable.rs`, to use the new
`pin-init` function. This is currently the only usage in the kernel.

Reviewed-by: Gerald Wisböck <gerald.wisboeck@feather.ink>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
---
 rust/kernel/revocable.rs |  2 ++
 rust/kernel/types.rs     | 25 +++++++++++++------------
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/rust/kernel/revocable.rs b/rust/kernel/revocable.rs
index 1e5a9d25c21b279b01f90b02997492aa4880d84f..4db68ea2207ebafcc09d082fdc1e281f31846a38 100644
--- a/rust/kernel/revocable.rs
+++ b/rust/kernel/revocable.rs
@@ -5,6 +5,8 @@
 //! The [`Revocable`] type wraps other types and allows access to them to be revoked. The existence
 //! of a [`RevocableGuard`] ensures that objects remain valid.
 
+use pin_init::Wrapper;
+
 use crate::{bindings, prelude::*, sync::rcu, types::Opaque};
 use core::{
     marker::PhantomData,
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 705f420fdfbc4a576de1c4546578f2f04cdf615e..f06e8720e012102e5c41e79fd97b0607e927d71c 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -9,7 +9,7 @@
     ops::{Deref, DerefMut},
     ptr::NonNull,
 };
-use pin_init::{PinInit, Zeroable};
+use pin_init::{PinInit, Wrapper, Zeroable};
 
 /// Used to transfer ownership to and from foreign (non-Rust) languages.
 ///
@@ -332,17 +332,6 @@ pub const fn uninit() -> 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| {
-            // SAFETY:
-            //   - `ptr` is a valid pointer to uninitialized memory,
-            //   - `slot` is not accessed on error; the call is infallible,
-            //   - `slot` is pinned in memory.
-            let _ = unsafe { PinInit::<T>::__pinned_init(slot, ptr) };
-        })
-    }
-
     /// Creates a pin-initializer from the given initializer closure.
     ///
     /// The returned initializer calls the given closure with the pointer to the inner `T` of this
@@ -393,6 +382,18 @@ pub const fn raw_get(this: *const Self) -> *mut T {
         UnsafeCell::raw_get(this.cast::<UnsafeCell<MaybeUninit<T>>>()).cast::<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) }
+        })
+    }
+}
 
 /// Types that are _always_ reference counted.
 ///

-- 
2.49.0

[PATCH v4 3/3] rust: use `UnsafePinned` in the implementation of `Opaque`

[Christian Schrefl]

Make the semantics of the `Opaque` implementation clearer and prepare
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>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
---
 rust/kernel/types.rs | 32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index f06e8720e012102e5c41e79fd97b0607e927d71c..e32905c42453132fbea49d37a6457547d42465ce 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,10 @@ fn drop(&mut self) {
 /// ```
 #[repr(transparent)]
 pub struct Opaque<T> {
-    value: UnsafeCell<MaybeUninit<T>>,
-    _pin: PhantomPinned,
+    // The kernel implementation of `UnsafePinned` uses `UnsafeCell` internally, but the
+    // upstream rust `UnsafePinned` will not. So to make sure this is compatible with
+    // the upstream version use `UnsafeCell` here.
+    value: UnsafePinned<UnsafeCell<MaybeUninit<T>>>,
 }
 
 // SAFETY: `Opaque<T>` allows the inner value to be any bit pattern, including all zeros.
@@ -319,16 +321,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 +371,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 +384,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

Re: [PATCH v4 1/3] rust: add UnsafePinned type

[Benno Lossin]

On Sun May 11, 2025 at 8:21 PM 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>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Co-developed-by: Sky <sky@sky9.dev>
> Signed-off-by: Sky <sky@sky9.dev>
> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>

One nit below, with that fixed:

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

> ---
>  rust/kernel/types.rs               |   6 ++
>  rust/kernel/types/unsafe_pinned.rs | 111 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 117 insertions(+)
>
> 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
> @@ -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;

I would put `mod` to the top of the 

---
Cheers,
Benno

Re: [PATCH v4 1/3] rust: add UnsafePinned type

[Christian Schrefl]

Hi Benno,

On 13.05.25 10:51 PM, Benno Lossin wrote:
> On Sun May 11, 2025 at 8:21 PM 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>
>> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>> Co-developed-by: Sky <sky@sky9.dev>
>> Signed-off-by: Sky <sky@sky9.dev>
>> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
> 
> One nit below, with that fixed:
> 
> Reviewed-by: Benno Lossin <lossin@kernel.org>
> 
>> ---
>>  rust/kernel/types.rs               |   6 ++
>>  rust/kernel/types/unsafe_pinned.rs | 111 +++++++++++++++++++++++++++++++++++++
>>  2 files changed, 117 insertions(+)
>>
>> 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
>> @@ -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;
> 
> I would put `mod` to the top of the 

Your sentence was cut off, I assume you mean:

> I would put `mod` to the top of the file.

I can do that, let me know if I should send a
new version or if this will be fixed when applying.

Cheers
Christian

Re: [PATCH v4 1/3] rust: add UnsafePinned type

[Benno Lossin]

On Sat May 17, 2025 at 1:36 PM CEST, Christian Schrefl wrote:
> Hi Benno,
>
> On 13.05.25 10:51 PM, Benno Lossin wrote:
>> On Sun May 11, 2025 at 8:21 PM 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>
>>> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>>> Co-developed-by: Sky <sky@sky9.dev>
>>> Signed-off-by: Sky <sky@sky9.dev>
>>> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
>> 
>> One nit below, with that fixed:
>> 
>> Reviewed-by: Benno Lossin <lossin@kernel.org>
>> 
>>> ---
>>>  rust/kernel/types.rs               |   6 ++
>>>  rust/kernel/types/unsafe_pinned.rs | 111 +++++++++++++++++++++++++++++++++++++
>>>  2 files changed, 117 insertions(+)
>>>
>>> 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
>>> @@ -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;
>> 
>> I would put `mod` to the top of the 
>
> Your sentence was cut off, I assume you mean:
>
>> I would put `mod` to the top of the file.

Oh yeah sorry about that.

> I can do that, let me know if I should send a
> new version or if this will be fixed when applying.

I think Miguel can do this when picking the patch :)

---
Cheers,
Benno

Re: [PATCH v4 0/3] rust: add `UnsafePinned` type

[Boqun Feng]

On Sun, May 11, 2025 at 08:21:37PM +0200, Christian Schrefl wrote:
> This version now only has the kernel implementation without the
> config flag for using the upstream version. Additionally now
> commits for using `UnsafePinned` in `Opaque` were added.
> 
> Checkpatch warns about `rust/kernel/types/unsafe_pinned.rs`
> missing a MAINTAINERS entry. This should be fine because it
> will be included in the RUST entry.
> 
> This patchset is based on the pin-init-next branch.
> 
> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>

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

Thanks a lot for doing this!

Regards,
Boqun

> ---
> Changes in v4:
> - Reworked `UnsafePinned::add` doctest to not use get_mut_unchecked.
> - Link to v3: https://lore.kernel.org/r/20250510-rust_unsafe_pinned-v3-0-57ce151123f9@gmail.com
> 
> Changes in v3:
> - Dropped CONFIG_RUSTC_HAS_UNSAFE_PINNED and feature(unsafe_pinned) (Alice)
> - Add comment to `Opaque` reasoning about included `UnsafeCell` (Benno)
> - Small changes in commit message of Patch 3 (Benno)
> - Removed docs mentioning not included functions (Benno)
> - Removed docs mentioning implementation details and added that as
>     comment instead (Benno)
> - Link to v2: https://lore.kernel.org/r/20250430-rust_unsafe_pinned-v2-0-fc8617a74024@gmail.com
> 
> Changes in v2:
> - Expanded `UnsafePinned` documentation to describe differences
>     with upstream rust implementation.
> - Removed config flag for using upstream `UnsafePinned` type.
> - Add patch implementing `Wrapper` for `Opaque`
> - Add patch for using `UnsafePinned` internally in `Opaque`
> - Link to v1: https://lore.kernel.org/r/20250418-rust_unsafe_pinned-v1-1-c4c7558399f8@gmail.com
> 
> ---
> Christian Schrefl (3):
>       rust: add UnsafePinned type
>       rust: implement `Wrapper<T>` for `Opaque<T>`
>       rust: use `UnsafePinned` in the implementation of `Opaque`
> 
>  rust/kernel/revocable.rs           |   2 +
>  rust/kernel/types.rs               |  45 ++++++++-------
>  rust/kernel/types/unsafe_pinned.rs | 111 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 138 insertions(+), 20 deletions(-)
> ---
> base-commit: 9de1a293c8ece00d226b21a35751ec178be2a9fa
> change-id: 20250418-rust_unsafe_pinned-891dce27418d
> 
> Best regards,
> -- 
> Christian Schrefl <chrisi.schrefl@gmail.com>
> 

Re: [PATCH v4 1/3] rust: add UnsafePinned type

[Miguel Ojeda]

On Sun, May 11, 2025 at 8:21 PM Christian Schrefl
<chrisi.schrefl@gmail.com> wrote:
>
> Signed-off-by: Sky <sky@sky9.dev>

Apologies for not noticing this earlier...

Since this is a Signed-off-by, the DCO applies, and it requires that
the name is a "known identity":

    https://docs.kernel.org/process/submitting-patches.html#developer-s-certificate-of-origin-1-1

Sky: is that name one you use to sign paperwork etc.? If so, that is
fine (and apologies in that case!) -- please let me know. If not,
please feel free to ping me in private if needed.

Thanks!

Cheers,
Miguel

Re: [PATCH v4 1/3] rust: add UnsafePinned type

[Christian Schrefl]

On 20.05.25 11:26 PM, Miguel Ojeda wrote:
> On Sun, May 11, 2025 at 8:21 PM Christian Schrefl
> <chrisi.schrefl@gmail.com> wrote:
>>
>> Signed-off-by: Sky <sky@sky9.dev>
> 
> Apologies for not noticing this earlier...
> 
> Since this is a Signed-off-by, the DCO applies, and it requires that
> the name is a "known identity":
> 
>     https://docs.kernel.org/process/submitting-patches.html#developer-s-certificate-of-origin-1-1
> 
> Sky: is that name one you use to sign paperwork etc.? If so, that is
> fine (and apologies in that case!) -- please let me know. If not,
> please feel free to ping me in private if needed.

Since it seems like Sky has not responded for 10 days
is should be fine to just drop their COB & SOB.

I only offered to add it since the upstream implementation
that this is based on was entirely done by them.

If you want to wait for some more time that's fine as well.

Cheers
Christian

Re: [PATCH v4 1/3] rust: add UnsafePinned type

[Christian Schrefl]

On 30.05.25 10:22 PM, Christian Schrefl wrote:
> On 20.05.25 11:26 PM, Miguel Ojeda wrote:
>> On Sun, May 11, 2025 at 8:21 PM Christian Schrefl
>> <chrisi.schrefl@gmail.com> wrote:
>>>
>>> Signed-off-by: Sky <sky@sky9.dev>
>>
>> Apologies for not noticing this earlier...
>>
>> Since this is a Signed-off-by, the DCO applies, and it requires that
>> the name is a "known identity":
>>
>>     https://docs.kernel.org/process/submitting-patches.html#developer-s-certificate-of-origin-1-1
>>
>> Sky: is that name one you use to sign paperwork etc.? If so, that is
>> fine (and apologies in that case!) -- please let me know. If not,
>> please feel free to ping me in private if needed.
> 
> Since it seems like Sky has not responded for 10 days
> is should be fine to just drop their COB & SOB.
> 
> I only offered to add it since the upstream implementation
> that this is based on was entirely done by them.
> 
> If you want to wait for some more time that's fine as well.

It might also make sense to wait for this PR [0]
and use the new code as the basis for the kernel
implementation, since that will change `UnsafePinned`
to include `UnsafeCell` semantics.

[0]: https://github.com/rust-lang/rust/pull/140638

> 
> Cheers
> Christian

Re: [PATCH v4 1/3] rust: add UnsafePinned type

[Miguel Ojeda]

On Fri, May 30, 2025 at 10:22 PM Christian Schrefl
<chrisi.schrefl@gmail.com> wrote:
>
> Since it seems like Sky has not responded for 10 days
> is should be fine to just drop their COB & SOB.
>
> I only offered to add it since the upstream implementation
> that this is based on was entirely done by them.
>
> If you want to wait for some more time that's fine as well.

So Sky and I shared a couple emails in private a week ago, but I am
waiting on a reply. There is no rush, since this is not going into
this merge window, so it is fine.

As for the tags -- it is up to you both; just please make sure it is
fair and that the DCO is respected. You can also consider another tag
that is not a SoB, such as Inspired-by, if that is more fitting.

Thanks!

Cheers,
Miguel

Re: [PATCH v4 1/3] rust: add UnsafePinned type

[Christian Schrefl]

On 11.05.25 8:21 PM, 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>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Co-developed-by: Sky <sky@sky9.dev>
> Signed-off-by: Sky <sky@sky9.dev>
> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
> ---
>  rust/kernel/types.rs               |   6 ++
>  rust/kernel/types/unsafe_pinned.rs | 111 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 117 insertions(+)
> 
> 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..315248cb88c089239bd672c889b5107060175ec3
> --- /dev/null
> +++ b/rust/kernel/types/unsafe_pinned.rs
> @@ -0,0 +1,111 @@
> +// 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. For details on the difference to the upstream
> +//! implementation see the comment on the [`UnsafePinned`] struct definition.
> +
> +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.

The upstream rust PR [0] that changes this was just merged. So now `UnsafePinned` includes
`UnsafeCell` semantics. It's probably best to also change this in the kernel docs.
Though it's still the case that removing the guarantee is simpler than adding it back later,
so let me know what you all think.

[0]: https://github.com/rust-lang/rust/pull/140638

> +///
> +/// 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.
> +//
> +// As opposed to the upstream Rust type this contains a `PhantomPinned` and `UnsafeCell<T>`
> +// - `PhantomPinned` to ensure the struct always is `!Unpin` and thus enables the `!Unpin` hack.
> +//   This causes the LLVM `noalias` and `dereferenceable` attributes to be removed from
> +//   `&mut !Unpin` types.
> +// - 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).
> +//   See this Rust tracking issue: https://github.com/rust-lang/rust/issues/137750
> +#[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_run
> +    /// use kernel::types::UnsafePinned;
> +    ///
> +    /// unsafe {
> +    ///     let mut x = UnsafePinned::new(0);
> +    ///     let ptr = x.get(); // read-only pointer, assumes immutability
> +    ///     # // Upstream Rust uses `UnsafePinned::get_mut_unchecked` here.
> +    ///     UnsafePinned::raw_get_mut(&raw mut x).write(1);
> +    ///     ptr.read(); // UB!
> +    /// }
> +    /// ```
> +    #[inline(always)]
> +    #[must_use]
> +    pub const fn get(&self) -> *const T {
> +        self.value.get()
> +    }
> +
> +    /// Gets a mutable pointer to the wrapped value.
> +    #[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) }
> +    }
> +}
> 

Re: [PATCH v4 1/3] rust: add UnsafePinned type

[Miguel Ojeda]

On Thu, Jun 5, 2025 at 7:03 PM Christian Schrefl
<chrisi.schrefl@gmail.com> wrote:
>
> The upstream rust PR [0] that changes this was just merged. So now `UnsafePinned` includes
> `UnsafeCell` semantics. It's probably best to also change this in the kernel docs.
> Though it's still the case that removing the guarantee is simpler than adding it back later,
> so let me know what you all think.

Since upstream's will imply `UnsafeCell`, then I assume they will not
take it away, and thus we should just document it the same way, so
that eventually we can just alias the upstream one.

But that last part can only happen in a long time, when our minimum
upgrades past 1.89, since otherwise we would lose the `UnsafeCell`
with an alias.

If we really wanted a type that does not do that, then we could have
another one, with a different name.

Thanks!

(By the way, please try to trim unneeded quotes in replies; otherwise,
threads become harder to read in clients such as lore.kernel.org, and
it also becomes harder to reply)

Cheers,
Miguel

Re: [PATCH v4 1/3] rust: add UnsafePinned type

[Benno Lossin]

On Thu Jun 5, 2025 at 7:03 PM CEST, Christian Schrefl wrote:
> On 11.05.25 8:21 PM, 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.
>
> The upstream rust PR [0] that changes this was just merged. So now `UnsafePinned` includes
> `UnsafeCell` semantics. It's probably best to also change this in the kernel docs.
> Though it's still the case that removing the guarantee is simpler than adding it back later,
> so let me know what you all think.

Depends on how "stable" this decision is. I haven't followed the
discussion, but given that this once changed to the "non-backwards"
compatible case it feels permanent.

How close is it to stabilization?

If it's close-ish, then I'd suggest we change this to reflect the new
semantics. If not, then we should leave it as-is.

---
Cheers,
Benno

> [0]: https://github.com/rust-lang/rust/pull/140638

Re: [PATCH v4 1/3] rust: add UnsafePinned type

[Christian Schrefl]

On 05.06.25 7:30 PM, Benno Lossin wrote:
> On Thu Jun 5, 2025 at 7:03 PM CEST, Christian Schrefl wrote:
>> On 11.05.25 8:21 PM, 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.
>>
>> The upstream rust PR [0] that changes this was just merged. So now `UnsafePinned` includes
>> `UnsafeCell` semantics. It's probably best to also change this in the kernel docs.
>> Though it's still the case that removing the guarantee is simpler than adding it back later,
>> so let me know what you all think.
> 
> Depends on how "stable" this decision is. I haven't followed the
> discussion, but given that this once changed to the "non-backwards"
> compatible case it feels permanent.

It seems pretty permanent, from what I understand its hard to
define the exact semantics `UnsafePinned` without `UnsafeCell`
in a way that is sound and because of some interactions with
`Pin::deref` it would have some backwards compatibility issues.
See this comment by Ralf on github [1].

[1]: https://github.com/rust-lang/rust/pull/137043#discussion_r1973978597

> 
> How close is it to stabilization?
> 
> If it's close-ish, then I'd suggest we change this to reflect the new
> semantics. If not, then we should leave it as-is.

It's pretty new, I'm not sure how long it's going to stay in nightly,
but it's probably going to be quite some time.

I wouldn't change it if it would already be in the kernel, but I think
its probably good to add the current state of the feature. This
would also reduce the difference between docs and implementation.

Cheers
Christian

Re: [PATCH v4 1/3] rust: add UnsafePinned type

[Benno Lossin]

On Thu Jun 5, 2025 at 7:57 PM CEST, Christian Schrefl wrote:
> On 05.06.25 7:30 PM, Benno Lossin wrote:
>> On Thu Jun 5, 2025 at 7:03 PM CEST, Christian Schrefl wrote:
>>> On 11.05.25 8:21 PM, 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.
>>>
>>> The upstream rust PR [0] that changes this was just merged. So now `UnsafePinned` includes
>>> `UnsafeCell` semantics. It's probably best to also change this in the kernel docs.
>>> Though it's still the case that removing the guarantee is simpler than adding it back later,
>>> so let me know what you all think.
>> 
>> Depends on how "stable" this decision is. I haven't followed the
>> discussion, but given that this once changed to the "non-backwards"
>> compatible case it feels permanent.
>
> It seems pretty permanent, from what I understand its hard to
> define the exact semantics `UnsafePinned` without `UnsafeCell`
> in a way that is sound and because of some interactions with
> `Pin::deref` it would have some backwards compatibility issues.
> See this comment by Ralf on github [1].

Oh yeah that seems like a done deal.

> [1]: https://github.com/rust-lang/rust/pull/137043#discussion_r1973978597
>
>> 
>> How close is it to stabilization?
>> 
>> If it's close-ish, then I'd suggest we change this to reflect the new
>> semantics. If not, then we should leave it as-is.
>
> It's pretty new, I'm not sure how long it's going to stay in nightly,
> but it's probably going to be quite some time.
>
> I wouldn't change it if it would already be in the kernel, but I think
> its probably good to add the current state of the feature. This
> would also reduce the difference between docs and implementation.

Makes sense, I agree with changing it. Thanks for your work!

---
Cheers,
Benno