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

[Christian Schrefl <chrisi.schrefl@gmail.com>]

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