Re: [PATCH v3 2/2] rust: lockdep: Use Pin for all LockClassKey usages

[Benno Lossin <benno.lossin@proton.me>]

On 06.02.25 22:27, Boqun Feng wrote:
> On Wed, Feb 05, 2025 at 08:30:50PM +0000, Benno Lossin wrote:
>> On 05.02.25 20:59, Mitchell Levy wrote:
>>> diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
>>> index 41dcddac69e2..119e5f569bdb 100644
>>> --- a/rust/kernel/sync/lock.rs
>>> +++ b/rust/kernel/sync/lock.rs
>>> @@ -7,12 +7,9 @@
>>>
>>>  use super::LockClassKey;
>>>  use crate::{
>>> -    init::PinInit,
>>> -    pin_init,
>>> -    str::CStr,
>>> -    types::{NotThreadSafe, Opaque, ScopeGuard},
>>> +    init::PinInit, pin_init, str::CStr, types::NotThreadSafe, types::Opaque, types::ScopeGuard,
>>>  };
>>> -use core::{cell::UnsafeCell, marker::PhantomPinned};
>>> +use core::{cell::UnsafeCell, marker::PhantomPinned, pin::Pin};
>>>  use macros::pin_data;
>>>
>>>  pub mod mutex;
>>> @@ -121,7 +118,7 @@ unsafe impl<T: ?Sized + Send, B: Backend> Sync for Lock<T, B> {}
>>>
>>>  impl<T, B: Backend> Lock<T, B> {
>>>      /// Constructs a new lock initialiser.
>>> -    pub fn new(t: T, name: &'static CStr, key: &'static LockClassKey) -> impl PinInit<Self> {
>>> +    pub fn new(t: T, name: &'static CStr, key: Pin<&'static LockClassKey>) -> impl PinInit<Self> {
>>
>> Static references do not need `Pin`, since `Pin::static_ref` [1] exists,
>> so you can just as well not add the `Pin` here and the other places
>> where you have `Pin<&'static T>`.
>>
> 
> You're right about `Pin` not needed for 'static. However, the
> `Pin<&'static LockClassKey>` signature is the intermediate state, and
> eventually we will need to support initializing a lock (and others) with
> a dynamically allocated `LockClassKey`, and when that is available, the
> type of `key` will become `Pin<&'a LockClassKey>`, so `Pin` is needed.
> 
> So I would like to keep this patch as it is. Works for you?

Makes sense and fine by me. It would be nice if it was also mentioned in
the commit message. Thanks!

---
Cheers,
Benno

> 
> Regards,
> Boqun
> 
>> The reasoning is that since the data lives for `'static` at that
>> location, it will never move (since it is borrowed for `'static` after
>> all).
>>
>> [1]: https://doc.rust-lang.org/std/pin/struct.Pin.html#method.static_ref
>>
>> ---
>> Cheers,
>> Benno
>>
>>>          pin_init!(Self {
>>>              data: UnsafeCell::new(t),
>>>              _pin: PhantomPinned,
>>> diff --git a/rust/kernel/sync/lock/global.rs b/rust/kernel/sync/lock/global.rs
>>> index 480ee724e3cc..d65f94b5caf2 100644
>>> --- a/rust/kernel/sync/lock/global.rs
>>> +++ b/rust/kernel/sync/lock/global.rs
>>> @@ -13,6 +13,7 @@
>>>  use core::{
>>>      cell::UnsafeCell,
>>>      marker::{PhantomData, PhantomPinned},
>>> +    pin::Pin,
>>>  };
>>>
>>>  /// Trait implemented for marker types for global locks.
>>
>>