Re: [PATCH] rust: add global lock support

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

On 26.08.24 17:27, Alice Ryhl wrote:
> We don't currently have any support for global locks in Rust, however
> they are very useful and I have needed to work around this limitation
> several times. My workarounds generally involve initializing the mutex
> in the module's init function, and this workaround is reflected here.
> 
> Due to the initialization requirement, constructing a global mutex is
> unsafe with the current approach. In the future, it would be really nice
> to support global mutexes that don't need to be initialized, which would
> make them safe. Unfortunately, this is not possible today because
> bindgen refuses to expose __ARCH_SPIN_LOCK_UNLOCKED to Rust as a
> compile-time constant. It just generates an `extern "C"` global
> reference instead.
> 
> On most architectures, we could initialize the lock to just contain all
> zeros. A possible improvement would be to create a Kconfig constant
> that is set whenever the current architecture uses all zeros for the
> initializer and have `unsafe_const_init` be a no-op on those
> architectures. We could also provide a safe const initializer that is
> only available when that Kconfig option is set.
> 
> For architectures that don't use all-zeros for the unlocked case, we
> will most likely have to hard-code the correct representation on the
> Rust side.
> 
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/kernel/sync/lock.rs | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
> index f6c34ca4d819..19e8ecd8d194 100644
> --- a/rust/kernel/sync/lock.rs
> +++ b/rust/kernel/sync/lock.rs
> @@ -117,6 +117,40 @@ pub fn new(t: T, name: &'static CStr, key: &'static LockClassKey) -> impl PinIni
>              }),
>          })
>      }
> +
> +    /// Create a global lock that has not yet been initialized.
> +    ///
> +    /// Since global locks is not yet fully supported, this method implements global locks by
> +    /// requiring you to initialize them before you start using it. Usually this is best done in
> +    /// the module's init function.
> +    ///
> +    /// # Safety
> +    ///
> +    /// You must call [`unsafe_const_init`] before calling any other method on this lock.
> +    ///
> +    /// [`unsafe_const_init`]: Self::unsafe_const_init
> +    pub const unsafe fn unsafe_const_new(t: T) -> Self {
> +        Self {
> +            data: UnsafeCell::new(t),
> +            state: Opaque::uninit(),
> +            _pin: PhantomPinned,
> +        }
> +    }
> +
> +    /// Initialize a global lock.
> +    ///
> +    /// # Safety
> +    ///
> +    /// * This lock must have been created with [`unsafe_const_new`].
> +    /// * This lock must be pinned.

You could also ask for `self: Pin<&Self>` and remove this constraint, or
is that not possible in your use-case?

---
Cheers,
Benno

> +    /// * This method must not be called more than once on a given lock.
> +    ///
> +    /// [`unsafe_const_new`]: Self::unsafe_const_new
> +    pub unsafe fn unsafe_const_init(&self, name: &'static CStr, key: &'static LockClassKey) {
> +        // SAFETY: The pointer to `state` is valid for the duration of this call, and both `name`
> +        // and `key` are valid indefinitely.
> +        unsafe { B::init(self.state.get(), name.as_char_ptr(), key.as_ptr()) }
> +    }
>  }
> 
>  impl<T: ?Sized, B: Backend> Lock<T, B> {
> 
> ---
> base-commit: b204bbc53f958fc3119d63bf2cda5a526e7267a4
> change-id: 20240826-static-mutex-a4b228e0e6aa
> 
> Best regards,
> --
> Alice Ryhl <aliceryhl@google.com>
>