Add support for creating global variables that are wrapped in a mutex or
spinlock.
The implementation here is intended to replace the global mutex
workaround found in the Rust Binder RFC [1]. In both cases, the global
lock must be initialized before first use. The macro is unsafe to use
for the same reason.
The separate initialization step is required because it is tricky to
access the value of __ARCH_SPIN_LOCK_UNLOCKED from Rust. Doing so will
require changes to the C side. That change will happen as a follow-up to
this patch.
Link: https://lore.kernel.org/rust-for-linux/20231101-rust-binder-v1-2-08ba9197f637@google.com/#Z31drivers:android:context.rs [1]
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
This patch is based on top of rust-next as it depends on:
https://lore.kernel.org/r/BL0PR02MB4914579914884B5D7473B3D6E96A2@BL0PR02MB4914.namprd02.prod.outlook.com
---
Changes in v6:
- Completely rewrote the implementation to move almost everything
outside the macro.
- Link to v5: https://lore.kernel.org/r/20241021-static-mutex-v5-1-8d118a6a99b7@google.com
Changes in v5:
- Adjust syntax as discussed in the last meeting.
- Fix various warnings.
- Link to v4: https://lore.kernel.org/r/20240930-static-mutex-v4-1-c59555413127@google.com
Changes in v4:
- Evaluate `$value` in the surrounding scope.
- Make types `pub(crate)` to avoid "private type in public interface"
errors when using the macro.
- Add trylock method.
- using https://lore.kernel.org/r/BL0PR02MB4914579914884B5D7473B3D6E96A2@BL0PR02MB4914.namprd02.prod.outlook.com
- Add Send/Sync implementations of LockedBy.
- Fix examples so they compile.
- Link to v3: https://lore.kernel.org/r/20240910-static-mutex-v3-1-5bebd11bdf3b@google.com
Changes in v3:
- Rewrite to provide a macro instead.
- Link to v2: https://lore.kernel.org/r/20240827-static-mutex-v2-1-17fc32b20332@google.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.rs | 1 +
rust/kernel/sync/lock.rs | 3 +
rust/kernel/sync/lock/global.rs | 303 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 307 insertions(+)
diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
index 0ab20975a3b5..2bdd1cffcdab 100644
--- a/rust/kernel/sync.rs
+++ b/rust/kernel/sync.rs
@@ -14,6 +14,7 @@
pub use arc::{Arc, ArcBorrow, UniqueArc};
pub use condvar::{new_condvar, CondVar, CondVarTimeoutResult};
+pub use lock::global::{global_lock, GlobalGuard, GlobalLock, GlobalLockBackend, GlobalLockedBy};
pub use lock::mutex::{new_mutex, Mutex};
pub use lock::spinlock::{new_spinlock, SpinLock};
pub use locked_by::LockedBy;
diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
index 90cc5416529b..a5d89cebf106 100644
--- a/rust/kernel/sync/lock.rs
+++ b/rust/kernel/sync/lock.rs
@@ -13,6 +13,9 @@
pub mod mutex;
pub mod spinlock;
+pub(super) mod global;
+pub use global::{GlobalGuard, GlobalLock, GlobalLockBackend, GlobalLockedBy};
+
/// The "backend" of a lock.
///
/// It is the actual implementation of the lock, without the need to repeat patterns used in all
diff --git a/rust/kernel/sync/lock/global.rs b/rust/kernel/sync/lock/global.rs
new file mode 100644
index 000000000000..5ba90b7f4074
--- /dev/null
+++ b/rust/kernel/sync/lock/global.rs
@@ -0,0 +1,303 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! Support for defining statics containing locks.
+
+use crate::{
+ str::CStr,
+ sync::lock::{Backend, Guard, Lock},
+ sync::{LockClassKey, LockedBy},
+ types::Opaque,
+};
+use core::{
+ cell::UnsafeCell,
+ marker::{PhantomData, PhantomPinned},
+};
+
+/// Trait implemented for marker types for global locks.
+///
+/// See [`global_lock!`](crate::sync::global_lock) for examples.
+pub trait GlobalLockBackend {
+ /// The name for this global lock.
+ const NAME: &'static CStr;
+ /// Item type stored in this global lock.
+ type Item: 'static;
+ /// The backend used for this global lock.
+ type Backend: Backend + 'static;
+ /// The class for this global lock.
+ fn get_lock_class() -> &'static LockClassKey;
+}
+
+/// Type used for global locks.
+///
+/// See [`global_lock!`](crate::sync::global_lock) for examples.
+pub struct GlobalLock<B: GlobalLockBackend> {
+ inner: Lock<B::Item, B::Backend>,
+}
+
+impl<B: GlobalLockBackend> GlobalLock<B> {
+ /// Creates a global lock.
+ ///
+ /// # Safety
+ ///
+ /// * Before any other method on this lock is called, [`init`] must be called.
+ /// * The type `B` must not be used with any other lock.
+ ///
+ /// [`init`]: Self::init
+ pub const unsafe fn new(data: B::Item) -> Self {
+ Self {
+ inner: Lock {
+ state: Opaque::uninit(),
+ data: UnsafeCell::new(data),
+ _pin: PhantomPinned,
+ },
+ }
+ }
+
+ /// Initializes a global lock.
+ ///
+ /// # Safety
+ ///
+ /// Must not be called more than once on a given lock.
+ pub unsafe fn init(&'static self) {
+ // SAFETY: The pointer to `state` is valid for the duration of this call, and both `name`
+ // and `key` are valid indefinitely. The `state` is pinned since we have a `'static`
+ // reference to `self`.
+ //
+ // We have exclusive access to the `state` since the caller of `new` promised to call
+ // `init` before using any other methods. As `init` can only be called once, all other
+ // uses of this lock must happen after this call.
+ unsafe {
+ B::Backend::init(
+ self.inner.state.get(),
+ B::NAME.as_char_ptr(),
+ B::get_lock_class().as_ptr(),
+ )
+ }
+ }
+
+ /// Lock this global lock.
+ pub fn lock(&'static self) -> GlobalGuard<B> {
+ GlobalGuard {
+ inner: self.inner.lock(),
+ }
+ }
+
+ /// Try to lock this global lock.
+ pub fn try_lock(&'static self) -> Option<GlobalGuard<B>> {
+ Some(GlobalGuard {
+ inner: self.inner.try_lock()?,
+ })
+ }
+}
+
+/// A guard for a [`GlobalLock`].
+///
+/// See [`global_lock!`](crate::sync::global_lock) for examples.
+pub struct GlobalGuard<B: GlobalLockBackend> {
+ inner: Guard<'static, B::Item, B::Backend>,
+}
+
+impl<B: GlobalLockBackend> core::ops::Deref for GlobalGuard<B> {
+ type Target = B::Item;
+
+ fn deref(&self) -> &Self::Target {
+ &self.inner
+ }
+}
+
+impl<B: GlobalLockBackend> core::ops::DerefMut for GlobalGuard<B> {
+ fn deref_mut(&mut self) -> &mut Self::Target {
+ &mut self.inner
+ }
+}
+
+/// A version of [`LockedBy`] for a [`GlobalLock`].
+///
+/// See [`global_lock!`](crate::sync::global_lock) for examples.
+pub struct GlobalLockedBy<T: ?Sized, B: GlobalLockBackend> {
+ _backend: PhantomData<B>,
+ value: UnsafeCell<T>,
+}
+
+// SAFETY: The same thread-safety rules as `LockedBy` apply to `GlobalLockedBy`.
+unsafe impl<T, B> Send for GlobalLockedBy<T, B>
+where
+ T: ?Sized,
+ B: GlobalLockBackend,
+ LockedBy<T, B::Item>: Send,
+{
+}
+
+// SAFETY: The same thread-safety rules as `LockedBy` apply to `GlobalLockedBy`.
+unsafe impl<T, B> Sync for GlobalLockedBy<T, B>
+where
+ T: ?Sized,
+ B: GlobalLockBackend,
+ LockedBy<T, B::Item>: Sync,
+{
+}
+
+impl<T, B: GlobalLockBackend> GlobalLockedBy<T, B> {
+ /// Create a new [`GlobalLockedBy`].
+ ///
+ /// The provided value will be protected by the global lock indicated by `B`.
+ pub fn new(val: T) -> Self {
+ Self {
+ value: UnsafeCell::new(val),
+ _backend: PhantomData,
+ }
+ }
+}
+
+impl<T: ?Sized, B: GlobalLockBackend> GlobalLockedBy<T, B> {
+ /// Access the value immutably.
+ ///
+ /// The caller must prove shared access to the lock.
+ pub fn as_ref<'a>(&'a self, _guard: &'a GlobalGuard<B>) -> &'a T {
+ // SAFETY: The lock is globally unique, so there can only be one guard.
+ unsafe { &*self.value.get() }
+ }
+
+ /// Access the value mutably.
+ ///
+ /// The caller must prove shared exclusive to the lock.
+ pub fn as_mut<'a>(&'a self, _guard: &'a mut GlobalGuard<B>) -> &'a mut T {
+ // SAFETY: The lock is globally unique, so there can only be one guard.
+ unsafe { &mut *self.value.get() }
+ }
+
+ /// Access the value mutably directly.
+ ///
+ /// The caller has exclusive access to this `GlobalLockedBy`, so they do not need to hold the
+ /// lock.
+ pub fn get_mut(&mut self) -> &mut T {
+ self.value.get_mut()
+ }
+}
+
+/// Defines a global lock.
+///
+/// The global mutex must be initialized before first use. Usually this is done by calling
+/// [`init`](GlobalLock::init) in the module initializer.
+///
+/// # Examples
+///
+/// A global counter.
+///
+/// ```
+/// # mod ex {
+/// # use kernel::prelude::*;
+/// kernel::sync::global_lock! {
+/// // SAFETY: Initialized in module initializer before first use.
+/// unsafe(uninit) static MY_COUNTER: Mutex<u32> = 0;
+/// }
+///
+/// fn increment_counter() -> u32 {
+/// let mut guard = MY_COUNTER.lock();
+/// *guard += 1;
+/// *guard
+/// }
+///
+/// impl kernel::Module for MyModule {
+/// fn init(_module: &'static ThisModule) -> Result<Self> {
+/// // SAFETY: called exactly once
+/// unsafe { MY_COUNTER.init() };
+///
+/// Ok(MyModule {})
+/// }
+/// }
+/// # struct MyModule {}
+/// # }
+/// ```
+///
+/// A global mutex used to protect all instances of a given struct.
+///
+/// ```
+/// # mod ex {
+/// # use kernel::prelude::*;
+/// use kernel::sync::{GlobalGuard, GlobalLockedBy};
+///
+/// kernel::sync::global_lock! {
+/// // SAFETY: Initialized in module initializer before first use.
+/// unsafe(uninit) static MY_MUTEX: Mutex<()> = ();
+/// }
+///
+/// /// All instances of this struct are protected by `MY_MUTEX`.
+/// struct MyStruct {
+/// my_counter: GlobalLockedBy<u32, MY_MUTEX>,
+/// }
+///
+/// impl MyStruct {
+/// /// Increment the counter in this instance.
+/// ///
+/// /// The caller must hold the `MY_MUTEX` mutex.
+/// fn increment(&self, guard: &mut GlobalGuard<MY_MUTEX>) -> u32 {
+/// let my_counter = self.my_counter.as_mut(guard);
+/// *my_counter += 1;
+/// *my_counter
+/// }
+/// }
+///
+/// impl kernel::Module for MyModule {
+/// fn init(_module: &'static ThisModule) -> Result<Self> {
+/// // SAFETY: called exactly once
+/// unsafe { MY_MUTEX.init() };
+///
+/// Ok(MyModule {})
+/// }
+/// }
+/// # struct MyModule {}
+/// # }
+/// ```
+#[macro_export]
+macro_rules! global_lock {
+ {
+ $(#[$meta:meta])* $pub:vis
+ unsafe(uninit) static $name:ident: $kind:ident<$valuety:ty> = $value:expr;
+ } => {
+ #[doc = ::core::concat!(
+ "Backend type used by [`",
+ ::core::stringify!($name),
+ "`](static@",
+ ::core::stringify!($name),
+ ")."
+ )]
+ #[allow(non_camel_case_types, unreachable_pub)]
+ $pub enum $name {}
+
+ impl $crate::sync::lock::GlobalLockBackend for $name {
+ const NAME: &'static $crate::str::CStr = $crate::c_str!(::core::stringify!($name));
+ type Item = $valuety;
+ type Backend = $crate::global_lock_inner!(backend $kind);
+
+ fn get_lock_class() -> &'static $crate::sync::LockClassKey {
+ $crate::static_lock_class!()
+ }
+ }
+
+ $(#[$meta])*
+ $pub static $name: $crate::sync::lock::GlobalLock<$name> = {
+ // Defined here to be outside the unsafe scope.
+ let init: $valuety = $value;
+
+ // SAFETY:
+ // * The user of this macro promises to initialize the macro before use.
+ // * We are only generating one static with this backend type.
+ unsafe { $crate::sync::lock::GlobalLock::new(init) }
+ };
+ };
+}
+pub use global_lock;
+
+#[doc(hidden)]
+#[macro_export]
+macro_rules! global_lock_inner {
+ (backend Mutex) => {
+ $crate::sync::lock::mutex::MutexBackend
+ };
+ (backend SpinLock) => {
+ $crate::sync::lock::spinlock::SpinLockBackend
+ };
+}
---
base-commit: 6ce162a002657910104c7a07fb50017681bc476c
change-id: 20240826-static-mutex-a4b228e0e6aa
Best regards,
--
Alice Ryhl <aliceryhl@google.com>
On Wed, Oct 23, 2024 at 3:23 PM Alice Ryhl <aliceryhl@google.com> wrote: > > Add support for creating global variables that are wrapped in a mutex or > spinlock. > > The implementation here is intended to replace the global mutex > workaround found in the Rust Binder RFC [1]. In both cases, the global > lock must be initialized before first use. The macro is unsafe to use > for the same reason. > > The separate initialization step is required because it is tricky to > access the value of __ARCH_SPIN_LOCK_UNLOCKED from Rust. Doing so will > require changes to the C side. That change will happen as a follow-up to > this patch. > > Link: https://lore.kernel.org/rust-for-linux/20231101-rust-binder-v1-2-08ba9197f637@google.com/#Z31drivers:android:context.rs [1] > Signed-off-by: Alice Ryhl <aliceryhl@google.com> Applied to `rust-next` -- thanks everyone! [ Simplified a few intra-doc links. Formatted a few comments. Reworded title. - Miguel ] Cheers, Miguel
On Wed, Oct 23, 2024 at 01:23:18PM +0000, Alice Ryhl wrote: > Add support for creating global variables that are wrapped in a mutex or > spinlock. > > The implementation here is intended to replace the global mutex > workaround found in the Rust Binder RFC [1]. In both cases, the global > lock must be initialized before first use. The macro is unsafe to use > for the same reason. > > The separate initialization step is required because it is tricky to > access the value of __ARCH_SPIN_LOCK_UNLOCKED from Rust. Doing so will > require changes to the C side. That change will happen as a follow-up to > this patch. > > Link: https://lore.kernel.org/rust-for-linux/20231101-rust-binder-v1-2-08ba9197f637@google.com/#Z31drivers:android:context.rs [1] > Signed-off-by: Alice Ryhl <aliceryhl@google.com> Reviewed-by: Boqun Feng <boqun.feng@gmail.com> In addition, I've also queued it in my lockdep-for-tip branch: https://git.kernel.org/pub/scm/linux/kernel/git/boqun/linux.git/log/?h=lockdep-for-tip as I want to help route Rust lock-related patches into tip, so this is a practice round for me ;-) Miguel, feel free to take it via rust-next with my Reviewed-by, I will drop it from my branch once if I see it shows up in v6.13-rc1. Regards, Boqun > --- > This patch is based on top of rust-next as it depends on: > https://lore.kernel.org/r/BL0PR02MB4914579914884B5D7473B3D6E96A2@BL0PR02MB4914.namprd02.prod.outlook.com > --- [...]
On Mon, Oct 28, 2024 at 6:49 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > Reviewed-by: Boqun Feng <boqun.feng@gmail.com> > > In addition, I've also queued it in my lockdep-for-tip branch: > > https://git.kernel.org/pub/scm/linux/kernel/git/boqun/linux.git/log/?h=lockdep-for-tip > > as I want to help route Rust lock-related patches into tip, so this is > a practice round for me ;-) > > Miguel, feel free to take it via rust-next with my Reviewed-by, I will > drop it from my branch once if I see it shows up in v6.13-rc1. No, no, it is great if you can take them :) By "if I see it shows up in v6.13-rc1", do you mean your branch is not meant for v6.13? Couple of things I noticed that I would normally double-check/fix in place: the "// SAFETY: called exactly once" comment could be formatted, and I think the "Link:" tag should be before your SoB (well, at least b4 does that, which makes sense since you added it, but I have seen commits done differently too). Thanks! Cheers, Miguel
On Tue, Oct 29, 2024 at 07:48:21PM +0100, Miguel Ojeda wrote: > On Mon, Oct 28, 2024 at 6:49 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > > > Reviewed-by: Boqun Feng <boqun.feng@gmail.com> > > > > In addition, I've also queued it in my lockdep-for-tip branch: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/boqun/linux.git/log/?h=lockdep-for-tip > > > > as I want to help route Rust lock-related patches into tip, so this is > > a practice round for me ;-) > > > > Miguel, feel free to take it via rust-next with my Reviewed-by, I will > > drop it from my branch once if I see it shows up in v6.13-rc1. > > No, no, it is great if you can take them :) > Thanks! > By "if I see it shows up in v6.13-rc1", do you mean your branch is not > meant for v6.13? > Right, so I'm acting as a sub-subsystem maintainer, and I submit pull requests to the tip tree, and then tip will send pull requests to Linus. So usually (yeah, I'm calling sometimes I've done only twice as "usually"), I submit my PR at rc2 to rc4, and tip will carry that into the next merge window. For example, since we are at v6.12-rc5, my next PR will be at v6.13-rc{2, 3 or 4}. So if this patch is going via me, it has to be in v6.14. I think this patch has no problem to go into v6.13, so again, feel free to do it ;-) > Couple of things I noticed that I would normally double-check/fix in > place: the "// SAFETY: called exactly once" comment could be Got it. This particular one needs to be "// SAFETY: Called exactly once", right? Note that since lockdep-for-tip is not watched by linux-next, I have some flexibilities between queuing a patch and preparing it for the PR (I will need to rebase anyway). This could give contributers an early notice and we both would have less things to watch (contributers can just wait for the patches to show up eventually in Linus' tree instead of checking the list for a reply, and I only need to focus on my branch for improvement) for normal cases. > formatted, and I think the "Link:" tag should be before your SoB > (well, at least b4 does that, which makes sense since you added it, > but I have seen commits done differently too). > For this I'm following the precedents in tip tree: always put the patch links at the last line. See the "Commit notifications" in Documentation/process/maintainer-tip.rst. (And yeah, I have to manually modify this after b4 applies the patches if you have to ask ;-)) Regards, Boqun > Thanks! > > Cheers, > Miguel
On Tue, Oct 29, 2024 at 10:12 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > "usually"), I submit my PR at rc2 to rc4, and tip will carry that into Ah, so you only submit up to -rc4, got it. I didn't know what the cutoff was. > Got it. This particular one needs to be "// SAFETY: Called exactly > once", right? Yeah, with an ending period (pedantic nit, but it is the style we try to follow). > For this I'm following the precedents in tip tree: always put the patch > links at the last line. See the "Commit notifications" in > Documentation/process/maintainer-tip.rst. (And yeah, I have to manually > modify this after b4 applies the patches if you have to ask ;-)) That explains it -- thanks! Sounds good (though perhaps the bot should be amended to agree with `b4`... :) Cheers, Miguel
On Mon, Oct 28, 2024 at 6:49 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > On Wed, Oct 23, 2024 at 01:23:18PM +0000, Alice Ryhl wrote: > > Add support for creating global variables that are wrapped in a mutex or > > spinlock. > > > > The implementation here is intended to replace the global mutex > > workaround found in the Rust Binder RFC [1]. In both cases, the global > > lock must be initialized before first use. The macro is unsafe to use > > for the same reason. > > > > The separate initialization step is required because it is tricky to > > access the value of __ARCH_SPIN_LOCK_UNLOCKED from Rust. Doing so will > > require changes to the C side. That change will happen as a follow-up to > > this patch. > > > > Link: https://lore.kernel.org/rust-for-linux/20231101-rust-binder-v1-2-08ba9197f637@google.com/#Z31drivers:android:context.rs [1] > > Signed-off-by: Alice Ryhl <aliceryhl@google.com> > > Reviewed-by: Boqun Feng <boqun.feng@gmail.com> Thanks! Alice
© 2016 - 2024 Red Hat, Inc.