Re: [PATCH 1/3] rust: add Aliased type

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

On 20.01.25 6:24 PM, Boqun Feng wrote:
> Hi Christian,
> 
> [Cc Daniel and Danilo]
> 
> Thanks for the patch!
> 
> On Sun, Jan 19, 2025 at 11:11:13PM +0100, Christian Schrefl wrote:
>> This type is useful for cases where a value might be shared with C code
>> but not interpreted by it.
>> In partiquarly this is added to for data that is shared between a Driver
>> and a MiscDevice implementation.
>>
>> Similar to Opaque but guarantees that the value is initialized and the
>> inner value is dropped when Aliased is dropped.
>>
>> This was origianally proposed for the IRQ abstractions [0], but also
>> useful for other cases where Data may be aliased, but is always valid
>> and automatic drop is desired.
>>
>> Link: https://lore.kernel.org/rust-for-linux/CAH5fLgiOASgjoYKFz6kWwzLaH07DqP2ph+3YyCDh2+gYqGpABA@mail.gmail.com [0]
>> Suggested-by: Alice Ryhl <aliceryhl@google.com>
>> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
>> ---
>>  rust/kernel/types.rs | 40 ++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 40 insertions(+)
>>
>> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
>> index 3aea6af9a0bca70ee42b4bad2fe31a99750cbf11..5640128c9a9055476a0040033946ba6caa6e7076 100644
>> --- a/rust/kernel/types.rs
>> +++ b/rust/kernel/types.rs
>> @@ -528,3 +528,43 @@ pub enum Either<L, R> {
>>  /// [`NotThreadSafe`]: type@NotThreadSafe
>>  #[allow(non_upper_case_globals)]
>>  pub const NotThreadSafe: NotThreadSafe = PhantomData;
>> +
>> +/// Stores a value that may be aliased.
>> +///
>> +/// This is similar to `Opaque<T>` but is guaranteed to contain valid data and will
>> +/// Call the Drop implementation of T when dropped.
>> +#[repr(transparent)]
>> +pub struct Aliased<T> {
> 
> As I already mentioned [1], the name `Aliased` is more reflecting the
> fact that this wrapper will avoid generating the "noalias" attribute(?)
> on the reference/pointer to the type rather than an intuitive idea about
> "why or when do I need this". Moreover, I think the argument about the
> naming of the counterpart in unstable Rust (UnsafePinned) makes sense to
> me [2]: this type alone won't prevent `&mut Aliased` getting `swap`, and
> it has to be used with `Pin`.
> 
> Therefore, I think we should use a different name, perhaps
> `(Always)Shared`, or just use `UnsafePinned`, or, as always, looking
> fowards to a better name from anybody ;-)
> 
> [1]: https://lore.kernel.org/rust-for-linux/Z407egxOy7oNLpq8@boqun-archlinux/
> [2]: https://rust-lang.github.io/rfcs/3467-unsafe-pinned.html#naming

I don't particularly care about the name, I mostly used aliased, because that's
the name that Alice originally used.

`(Always)Shared` seems confusing to me.

I guess we can use `UnsafePinned`, but most people won't know what that means,
also I'm not sure if it's a good Idea to use the same name as a (future) 
language type.

If there is a consensus on a good name I'll use that for the next version.

> 
> Regards,
> Boqun
> 
>> +    value: UnsafeCell<T>,
>> +    _pin: PhantomPinned,
>> +}
>> +
>> +impl<T> Aliased<T> {
>> +    /// Creates a new `Aliased` value.
>> +    pub const fn new(value: T) -> Self {
>> +        Self {
>> +            value: UnsafeCell::new(value),
>> +            _pin: PhantomPinned,
>> +        }
>> +    }
>> +    /// Create an `Aliased` pin-initializer from the given pin-initializer.
>> +    pub fn try_pin_init<E>(value: impl PinInit<T, E>) -> impl PinInit<Self, E> {
>> +        // SAFETY:
>> +        // In case of an error in value the error is returned, otherwise the slot is fully initialized,
>> +        // since value is initialized and _pin is a Zero sized type.
>> +        // The pin invariants of value are upheld, since no moving occurs.
>> +        unsafe { init::pin_init_from_closure(move |slot| value.__pinned_init(Self::raw_get(slot))) }
>> +    }
>> +    /// Returns a raw pointer to the opaque data.
>> +    pub const fn get(&self) -> *mut T {
>> +        UnsafeCell::get(&self.value).cast::<T>()
>> +    }
>> +
>> +    /// Gets the value behind `this`.
>> +    ///
>> +    /// This function is useful to get access to the value without creating intermediate
>> +    /// references.
>> +    pub const fn raw_get(this: *const Self) -> *mut T {
>> +        UnsafeCell::raw_get(this.cast::<UnsafeCell<MaybeUninit<T>>>()).cast::<T>()
>> +    }
>> +}
>>
>> -- 
>> 2.48.1
>>