Message-ID: <20240827-static-mutex-v2-1-17fc32b20332@google.com>
Date: Tue, 27 Aug 2024 08:41:56 +0000
From: Alice Ryhl <aliceryhl@...gle.com>
To: Miguel Ojeda <ojeda@...nel.org>
Cc: Alex Gaynor <alex.gaynor@...il.com>, Wedson Almeida Filho <wedsonaf@...il.com>, 
	Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, 
	"Björn Roy Baron" <bjorn3_gh@...tonmail.com>, Benno Lossin <benno.lossin@...ton.me>, 
	Andreas Hindborg <a.hindborg@...sung.com>, rust-for-linux@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Alice Ryhl <aliceryhl@...gle.com>
Subject: [PATCH v2] rust: add global lock support

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@...gle.com>
---
Changes in v2:
- Require `self: Pin<&Self>` and recommend `Pin::static_ref`.
- Other doc improvements including new example.
- Link to v1: https://lore.kernel.org/r/20240826-static-mutex-v1-1-a14ee71561f3@google.com
---
 rust/kernel/sync/lock.rs | 64 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 63 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
index f6c34ca4d819..cfc5e160d78c 100644
--- a/rust/kernel/sync/lock.rs
+++ b/rust/kernel/sync/lock.rs
@@ -7,7 +7,7 @@
 
 use super::LockClassKey;
 use crate::{init::PinInit, pin_init, str::CStr, types::Opaque, types::ScopeGuard};
-use core::{cell::UnsafeCell, marker::PhantomData, marker::PhantomPinned};
+use core::{cell::UnsafeCell, marker::PhantomData, marker::PhantomPinned, pin::Pin};
 use macros::pin_data;
 
 pub mod mutex;
@@ -117,6 +117,68 @@ 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.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::sync::Mutex;
+    ///
+    /// // SAFETY: We initialize the mutex before first use.
+    /// static MY_MUTEX: Mutex<()> = unsafe { Mutex::unsafe_const_new(()) };
+    ///
+    /// // For the sake of this example, assume that this is the module initializer.
+    /// fn module_init() {
+    ///     // SAFETY:
+    ///     // * `MY_MUTEX` was created using `unsafe_const_new`.
+    ///     // * This call is in the module initializer, which doesn't runs more than once.
+    ///     unsafe {
+    ///         core::pin::Pin::static_ref(&MY_MUTEX)
+    ///             .unsafe_const_init(kernel::c_str!("MY_MUTEX"), kernel::static_lock_class!())
+    ///     };
+    /// }
+    /// ```
+    ///
+    /// # 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.
+    ///
+    /// When using this to initialize a `static` lock, you can use [`Pin::static_ref`] to construct
+    /// the pinned reference.
+    ///
+    /// See the docs for [`unsafe_const_new`] for examples.
+    ///
+    /// # Safety
+    ///
+    /// * This lock must have been created with [`unsafe_const_new`].
+    /// * 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: Pin<&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@...gle.com>

Powered by blists - more mailing lists