rust/kernel/sync.rs | 1 + rust/kernel/sync/lock.rs | 31 +++++- rust/kernel/sync/lock/global.rs | 237 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 268 insertions(+), 1 deletion(-)
Add support for creating global variables that are wrapped in a mutex or
spinlock. Optionally, the macro can generate a special LockedBy type
that does not require a runtime check.
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 bindgen refuses to
expose __ARCH_SPIN_LOCK_UNLOCKED to Rust as a compile-time constant. It
just generates an `extern "C"` global reference instead. In the future,
we could expose the value of __ARCH_SPIN_LOCK_UNLOCKED to Rust in a way
that Rust can understand. This would remove the need for a separate
initialization step.
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>
---
I've been having some trouble with the kunit tests. The import of
__static_lock_ty_$name fails when I try with kunit, but copying the same
code into a sample does not reproduce the error. Suggestions would be
appreciated.
---
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 | 31 +++++-
rust/kernel/sync/lock/global.rs | 237 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 268 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
index 0ab20975a3b5..2e97e22715db 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;
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 f6c34ca4d819..3ae7a278016d 100644
--- a/rust/kernel/sync/lock.rs
+++ b/rust/kernel/sync/lock.rs
@@ -7,12 +7,14 @@
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;
pub mod spinlock;
+pub(super) mod global;
+
/// The "backend" of a lock.
///
/// It is the actual implementation of the lock, without the need to repeat patterns used in all
@@ -117,6 +119,33 @@ pub fn new(t: T, name: &'static CStr, key: &'static LockClassKey) -> impl PinIni
}),
})
}
+
+ /// # Safety
+ ///
+ /// Before any other method on this lock is called, `global_lock_helper_init` must be called.
+ #[doc(hidden)]
+ pub const unsafe fn global_lock_helper_new(state: Opaque<B::State>, data: T) -> Self {
+ Self {
+ state,
+ data: UnsafeCell::new(data),
+ _pin: PhantomPinned,
+ }
+ }
+
+ /// # Safety
+ ///
+ /// * This lock must have been created using `global_lock_helper_new`.
+ /// * Must only be called once for each lock.
+ #[doc(hidden)]
+ pub unsafe fn global_lock_helper_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> {
diff --git a/rust/kernel/sync/lock/global.rs b/rust/kernel/sync/lock/global.rs
new file mode 100644
index 000000000000..c1eb25d37abd
--- /dev/null
+++ b/rust/kernel/sync/lock/global.rs
@@ -0,0 +1,237 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! Support for defining statics containing locks.
+
+/// Defines a global lock.
+///
+/// Supports the following options:
+///
+/// * `value` specifies the initial value in the global lock.
+/// * `wrapper` specifies the name of the wrapper struct.
+/// * `guard` specifies the name of the guard type.
+/// * `locked_by` specifies the name of the `LockedBy` type.
+///
+/// # Examples
+///
+/// A global counter.
+///
+/// ```
+/// kernel::sync::global_lock! {
+/// // SAFETY: Initialized in module initializer before first use.
+/// static MY_COUNTER: Mutex<u32> = unsafe { uninit };
+/// value: 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.
+///
+/// ```
+/// kernel::sync::global_lock! {
+/// // SAFETY: Initialized in module initializer before first use.
+/// static MY_MUTEX: Mutex<()> = unsafe { uninit };
+/// value: ();
+/// guard: MyGuard;
+/// locked_by: LockedByMyMutex;
+/// }
+///
+/// /// All instances of this struct are protected by `MY_MUTEX`.
+/// struct MyStruct {
+/// my_counter: LockedByMyMutex<u32>,
+/// }
+///
+/// impl MyStruct {
+/// /// Increment the counter in this instance.
+/// ///
+/// /// The caller must hold the `MY_MUTEX` mutex.
+/// fn increment(&self, guard: &mut MyGuard) -> 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 static $name:ident: $kind:ident<$valuety:ty> = unsafe { uninit };
+ value: $value:expr;
+ wrapper: $wrapper:ident;
+ $( name: $lname:literal; )?
+ $(
+ guard: $guard:ident;
+ locked_by: $locked_by:ident;
+ )?
+ } => {
+ $crate::macros::paste! {
+ type [< __static_lock_ty_ $name >] = $valuety;
+
+ #[allow(unused_pub)]
+ mod [< __static_lock_mod_ $name >] {
+ use super::[< __static_lock_ty_ $name >] as Val;
+ type Backend = $crate::global_lock_inner!(backend $kind);
+ type GuardTyp = $crate::global_lock_inner!(guard $kind, Val $(, $guard)?);
+
+ /// # Safety
+ ///
+ /// Must be used to initialize `super::$name`.
+ pub(super) const unsafe fn new() -> $wrapper {
+ let state = $crate::types::Opaque::uninit();
+ $wrapper {
+ // SAFETY: The user of this macro promises to call `init` before calling
+ // `lock`.
+ inner: unsafe {
+ $crate::sync::lock::Lock::global_lock_helper_new(state, $value)
+ }
+ }
+ }
+
+ /// Wrapper type for a global lock.
+ pub struct $wrapper {
+ inner: $crate::sync::lock::Lock<Val, Backend>,
+ }
+
+ impl $wrapper {
+ /// Initialize the global lock.
+ ///
+ /// # Safety
+ ///
+ /// This method must not be called more than once.
+ pub unsafe fn init(&'static self) {
+ // SAFETY:
+ // * This type can only be created by `new`.
+ // * Caller promises to not call this method more than once.
+ unsafe {
+ $crate::sync::lock::Lock::global_lock_helper_init(
+ ::core::pin::Pin::static_ref(&self.inner),
+ $crate::optional_name!($($lname)?),
+ $crate::static_lock_class!(),
+ );
+ }
+ }
+
+ /// Lock this global lock.
+ pub fn lock(&'static self) -> GuardTyp {
+ $crate::global_lock_inner!(new_guard $($guard)? {
+ self.inner.lock()
+ })
+ }
+ }
+
+ $(
+ pub struct $guard($crate::sync::lock::Guard<'static, Val, Backend>);
+
+ impl ::core::ops::Deref for $guard {
+ type Target = Val;
+ fn deref(&self) -> &Val {
+ &self.0
+ }
+ }
+
+ impl ::core::ops::DerefMut for $guard {
+ fn deref_mut(&mut self) -> &mut Val {
+ &mut self.0
+ }
+ }
+
+ pub struct $locked_by<T: ?Sized>(::core::cell::UnsafeCell<T>);
+
+ impl<T> $locked_by<T> {
+ pub fn new(val: T) -> Self {
+ Self(::core::cell::UnsafeCell::new(val))
+ }
+ }
+
+ impl<T: ?Sized> $locked_by<T> {
+ pub fn as_ref<'a>(&'a self, _guard: &'a $guard) -> &'a T {
+ // SAFETY: The lock is globally unique, so there can only be one guard.
+ unsafe { &*self.0.get() }
+ }
+
+ pub fn as_mut<'a>(&'a self, _guard: &'a mut $guard) -> &'a mut T {
+ // SAFETY: The lock is globally unique, so there can only be one guard.
+ unsafe { &mut *self.0.get() }
+ }
+
+ pub fn get_mut(&mut self) -> &mut T {
+ self.0.get_mut()
+ }
+ }
+ )?
+ }
+
+ use [< __static_lock_mod_ $name >]::$wrapper;
+ $( use [< __static_lock_mod_ $name >]::{$guard, $locked_by}; )?
+
+ $(#[$meta])*
+ $pub static $name: $wrapper = {
+ // SAFETY: We are using this to initialize $name.
+ unsafe { [< __static_lock_mod_ $name >]::new() }
+ };
+ }
+ };
+
+ {
+ $(#[$meta:meta])* $pub:vis static $name:ident: $kind:ident<$valuety:ty> = unsafe { uninit };
+ value: $value:expr;
+ $( name: $lname:literal; )?
+ $(
+ guard: $guard:ident;
+ locked_by: $locked_by:ident;
+ )?
+ } => {
+ $crate::macros::paste! {
+ $crate::global_lock! {
+ $(#[$meta])* $pub static $name: $kind<$valuety> = unsafe { uninit };
+ value: $value;
+ wrapper: [< __static_lock_wrapper_ $name >];
+ $( name: $lname; )?
+ $( guard: $guard; locked_by: $locked_by; )?
+ }
+ }
+ }
+}
+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 };
+ (guard Mutex, $val:ty) => {
+ $crate::sync::lock::Guard<'static, $val, $crate::sync::lock::mutex::MutexBackend>
+ };
+ (guard SpinLock, $val:ty) => {
+ $crate::sync::lock::Guard<'static, $val, $crate::sync::lock::spinlock::SpinLockBackend>
+ };
+ (guard $kind:ident, $val:ty, $name:ident) => { $name };
+ (new_guard { $val:expr }) => { $val };
+ (new_guard $name:ident { $val:expr }) => { $name($val) };
+}
---
base-commit: 93dc3be19450447a3a7090bd1dfb9f3daac3e8d2
change-id: 20240826-static-mutex-a4b228e0e6aa
Best regards,
--
Alice Ryhl <aliceryhl@google.com>On Tue, Sep 10, 2024 at 02:23:34PM +0000, Alice Ryhl wrote:
> Add support for creating global variables that are wrapped in a mutex or
> spinlock. Optionally, the macro can generate a special LockedBy type
> that does not require a runtime check.
>
> 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 bindgen refuses to
> expose __ARCH_SPIN_LOCK_UNLOCKED to Rust as a compile-time constant. It
> just generates an `extern "C"` global reference instead. In the future,
> we could expose the value of __ARCH_SPIN_LOCK_UNLOCKED to Rust in a way
> that Rust can understand. This would remove the need for a separate
> initialization step.
Yeah it's just a raw C struct initializer, I wouldn't even know how to
move that to rust.
An absolute horrible idea, and I didn't try whether it's even possible:
- put all the global locks of a type into a special linker section (we
need a macro anyway for them).
- patch them up with a horrible linker script objtool patching with an
example lock initialized by the C macro.
Even worse idea, on most architectures/config it's all zeros. Iirc the one
I've found that might matter a bit is CONFIG_SMP=n with some lock
debugging enabled. We could make rust support conditional on those, and
then maybe a build-time check that it's actually all set to 0 ...
Requiring the unsafe init just feels a bit disappointing to me, when the C
side (including lockdep annotations) tries really hard to make global
locks a one-liner.
Cheers, Sima
>
> 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>
> ---
> I've been having some trouble with the kunit tests. The import of
> __static_lock_ty_$name fails when I try with kunit, but copying the same
> code into a sample does not reproduce the error. Suggestions would be
> appreciated.
> ---
> 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 | 31 +++++-
> rust/kernel/sync/lock/global.rs | 237 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 268 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> index 0ab20975a3b5..2e97e22715db 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;
> 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 f6c34ca4d819..3ae7a278016d 100644
> --- a/rust/kernel/sync/lock.rs
> +++ b/rust/kernel/sync/lock.rs
> @@ -7,12 +7,14 @@
>
> 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;
> pub mod spinlock;
>
> +pub(super) mod global;
> +
> /// The "backend" of a lock.
> ///
> /// It is the actual implementation of the lock, without the need to repeat patterns used in all
> @@ -117,6 +119,33 @@ pub fn new(t: T, name: &'static CStr, key: &'static LockClassKey) -> impl PinIni
> }),
> })
> }
> +
> + /// # Safety
> + ///
> + /// Before any other method on this lock is called, `global_lock_helper_init` must be called.
> + #[doc(hidden)]
> + pub const unsafe fn global_lock_helper_new(state: Opaque<B::State>, data: T) -> Self {
> + Self {
> + state,
> + data: UnsafeCell::new(data),
> + _pin: PhantomPinned,
> + }
> + }
> +
> + /// # Safety
> + ///
> + /// * This lock must have been created using `global_lock_helper_new`.
> + /// * Must only be called once for each lock.
> + #[doc(hidden)]
> + pub unsafe fn global_lock_helper_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> {
> diff --git a/rust/kernel/sync/lock/global.rs b/rust/kernel/sync/lock/global.rs
> new file mode 100644
> index 000000000000..c1eb25d37abd
> --- /dev/null
> +++ b/rust/kernel/sync/lock/global.rs
> @@ -0,0 +1,237 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +// Copyright (C) 2024 Google LLC.
> +
> +//! Support for defining statics containing locks.
> +
> +/// Defines a global lock.
> +///
> +/// Supports the following options:
> +///
> +/// * `value` specifies the initial value in the global lock.
> +/// * `wrapper` specifies the name of the wrapper struct.
> +/// * `guard` specifies the name of the guard type.
> +/// * `locked_by` specifies the name of the `LockedBy` type.
> +///
> +/// # Examples
> +///
> +/// A global counter.
> +///
> +/// ```
> +/// kernel::sync::global_lock! {
> +/// // SAFETY: Initialized in module initializer before first use.
> +/// static MY_COUNTER: Mutex<u32> = unsafe { uninit };
> +/// value: 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.
> +///
> +/// ```
> +/// kernel::sync::global_lock! {
> +/// // SAFETY: Initialized in module initializer before first use.
> +/// static MY_MUTEX: Mutex<()> = unsafe { uninit };
> +/// value: ();
> +/// guard: MyGuard;
> +/// locked_by: LockedByMyMutex;
> +/// }
> +///
> +/// /// All instances of this struct are protected by `MY_MUTEX`.
> +/// struct MyStruct {
> +/// my_counter: LockedByMyMutex<u32>,
> +/// }
> +///
> +/// impl MyStruct {
> +/// /// Increment the counter in this instance.
> +/// ///
> +/// /// The caller must hold the `MY_MUTEX` mutex.
> +/// fn increment(&self, guard: &mut MyGuard) -> 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 static $name:ident: $kind:ident<$valuety:ty> = unsafe { uninit };
> + value: $value:expr;
> + wrapper: $wrapper:ident;
> + $( name: $lname:literal; )?
> + $(
> + guard: $guard:ident;
> + locked_by: $locked_by:ident;
> + )?
> + } => {
> + $crate::macros::paste! {
> + type [< __static_lock_ty_ $name >] = $valuety;
> +
> + #[allow(unused_pub)]
> + mod [< __static_lock_mod_ $name >] {
> + use super::[< __static_lock_ty_ $name >] as Val;
> + type Backend = $crate::global_lock_inner!(backend $kind);
> + type GuardTyp = $crate::global_lock_inner!(guard $kind, Val $(, $guard)?);
> +
> + /// # Safety
> + ///
> + /// Must be used to initialize `super::$name`.
> + pub(super) const unsafe fn new() -> $wrapper {
> + let state = $crate::types::Opaque::uninit();
> + $wrapper {
> + // SAFETY: The user of this macro promises to call `init` before calling
> + // `lock`.
> + inner: unsafe {
> + $crate::sync::lock::Lock::global_lock_helper_new(state, $value)
> + }
> + }
> + }
> +
> + /// Wrapper type for a global lock.
> + pub struct $wrapper {
> + inner: $crate::sync::lock::Lock<Val, Backend>,
> + }
> +
> + impl $wrapper {
> + /// Initialize the global lock.
> + ///
> + /// # Safety
> + ///
> + /// This method must not be called more than once.
> + pub unsafe fn init(&'static self) {
> + // SAFETY:
> + // * This type can only be created by `new`.
> + // * Caller promises to not call this method more than once.
> + unsafe {
> + $crate::sync::lock::Lock::global_lock_helper_init(
> + ::core::pin::Pin::static_ref(&self.inner),
> + $crate::optional_name!($($lname)?),
> + $crate::static_lock_class!(),
> + );
> + }
> + }
> +
> + /// Lock this global lock.
> + pub fn lock(&'static self) -> GuardTyp {
> + $crate::global_lock_inner!(new_guard $($guard)? {
> + self.inner.lock()
> + })
> + }
> + }
> +
> + $(
> + pub struct $guard($crate::sync::lock::Guard<'static, Val, Backend>);
> +
> + impl ::core::ops::Deref for $guard {
> + type Target = Val;
> + fn deref(&self) -> &Val {
> + &self.0
> + }
> + }
> +
> + impl ::core::ops::DerefMut for $guard {
> + fn deref_mut(&mut self) -> &mut Val {
> + &mut self.0
> + }
> + }
> +
> + pub struct $locked_by<T: ?Sized>(::core::cell::UnsafeCell<T>);
> +
> + impl<T> $locked_by<T> {
> + pub fn new(val: T) -> Self {
> + Self(::core::cell::UnsafeCell::new(val))
> + }
> + }
> +
> + impl<T: ?Sized> $locked_by<T> {
> + pub fn as_ref<'a>(&'a self, _guard: &'a $guard) -> &'a T {
> + // SAFETY: The lock is globally unique, so there can only be one guard.
> + unsafe { &*self.0.get() }
> + }
> +
> + pub fn as_mut<'a>(&'a self, _guard: &'a mut $guard) -> &'a mut T {
> + // SAFETY: The lock is globally unique, so there can only be one guard.
> + unsafe { &mut *self.0.get() }
> + }
> +
> + pub fn get_mut(&mut self) -> &mut T {
> + self.0.get_mut()
> + }
> + }
> + )?
> + }
> +
> + use [< __static_lock_mod_ $name >]::$wrapper;
> + $( use [< __static_lock_mod_ $name >]::{$guard, $locked_by}; )?
> +
> + $(#[$meta])*
> + $pub static $name: $wrapper = {
> + // SAFETY: We are using this to initialize $name.
> + unsafe { [< __static_lock_mod_ $name >]::new() }
> + };
> + }
> + };
> +
> + {
> + $(#[$meta:meta])* $pub:vis static $name:ident: $kind:ident<$valuety:ty> = unsafe { uninit };
> + value: $value:expr;
> + $( name: $lname:literal; )?
> + $(
> + guard: $guard:ident;
> + locked_by: $locked_by:ident;
> + )?
> + } => {
> + $crate::macros::paste! {
> + $crate::global_lock! {
> + $(#[$meta])* $pub static $name: $kind<$valuety> = unsafe { uninit };
> + value: $value;
> + wrapper: [< __static_lock_wrapper_ $name >];
> + $( name: $lname; )?
> + $( guard: $guard; locked_by: $locked_by; )?
> + }
> + }
> + }
> +}
> +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 };
> + (guard Mutex, $val:ty) => {
> + $crate::sync::lock::Guard<'static, $val, $crate::sync::lock::mutex::MutexBackend>
> + };
> + (guard SpinLock, $val:ty) => {
> + $crate::sync::lock::Guard<'static, $val, $crate::sync::lock::spinlock::SpinLockBackend>
> + };
> + (guard $kind:ident, $val:ty, $name:ident) => { $name };
> + (new_guard { $val:expr }) => { $val };
> + (new_guard $name:ident { $val:expr }) => { $name($val) };
> +}
>
> ---
> base-commit: 93dc3be19450447a3a7090bd1dfb9f3daac3e8d2
> change-id: 20240826-static-mutex-a4b228e0e6aa
>
> Best regards,
> --
> Alice Ryhl <aliceryhl@google.com>
>
--
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
On Fri, Sep 13, 2024 at 6:02 PM Simona Vetter <simona.vetter@ffwll.ch> wrote:
>
> On Tue, Sep 10, 2024 at 02:23:34PM +0000, Alice Ryhl wrote:
> > Add support for creating global variables that are wrapped in a mutex or
> > spinlock. Optionally, the macro can generate a special LockedBy type
> > that does not require a runtime check.
> >
> > 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 bindgen refuses to
> > expose __ARCH_SPIN_LOCK_UNLOCKED to Rust as a compile-time constant. It
> > just generates an `extern "C"` global reference instead. In the future,
> > we could expose the value of __ARCH_SPIN_LOCK_UNLOCKED to Rust in a way
> > that Rust can understand. This would remove the need for a separate
> > initialization step.
>
> Yeah it's just a raw C struct initializer, I wouldn't even know how to
> move that to rust.
>
> An absolute horrible idea, and I didn't try whether it's even possible:
> - put all the global locks of a type into a special linker section (we
> need a macro anyway for them).
> - patch them up with a horrible linker script objtool patching with an
> example lock initialized by the C macro.
>
> Even worse idea, on most architectures/config it's all zeros. Iirc the one
> I've found that might matter a bit is CONFIG_SMP=n with some lock
> debugging enabled. We could make rust support conditional on those, and
> then maybe a build-time check that it's actually all set to 0 ...
>
> Requiring the unsafe init just feels a bit disappointing to me, when the C
> side (including lockdep annotations) tries really hard to make global
> locks a one-liner.
I actually have a prototype lying around that gets rid of the
initialization step. The idea is to define some new macros:
#define __ARCH_SPIN_LOCK_UNLOCKED { 0 }
+#define __ARCH_SPIN_LOCK_UNLOCKED_TYP unsigned int
+#define __ARCH_SPIN_LOCK_UNLOCKED_INT 0
Rust then uses the two new #defines to initialize the raw spin lock to
the right bytes. As long as __ARCH_SPIN_LOCK_UNLOCKED and
__ARCH_SPIN_LOCK_UNLOCKED_INT are represented by the same sequence of
bytes, it should work.
Alice
On 13.09.24 18:10, Alice Ryhl wrote:
> On Fri, Sep 13, 2024 at 6:02 PM Simona Vetter <simona.vetter@ffwll.ch> wrote:
>> On Tue, Sep 10, 2024 at 02:23:34PM +0000, Alice Ryhl wrote:
>>> Add support for creating global variables that are wrapped in a mutex or
>>> spinlock. Optionally, the macro can generate a special LockedBy type
>>> that does not require a runtime check.
>>>
>>> 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 bindgen refuses to
>>> expose __ARCH_SPIN_LOCK_UNLOCKED to Rust as a compile-time constant. It
>>> just generates an `extern "C"` global reference instead. In the future,
>>> we could expose the value of __ARCH_SPIN_LOCK_UNLOCKED to Rust in a way
>>> that Rust can understand. This would remove the need for a separate
>>> initialization step.
>>
>> Yeah it's just a raw C struct initializer, I wouldn't even know how to
>> move that to rust.
>>
>> An absolute horrible idea, and I didn't try whether it's even possible:
>> - put all the global locks of a type into a special linker section (we
>> need a macro anyway for them).
>> - patch them up with a horrible linker script objtool patching with an
>> example lock initialized by the C macro.
>>
>> Even worse idea, on most architectures/config it's all zeros. Iirc the one
>> I've found that might matter a bit is CONFIG_SMP=n with some lock
>> debugging enabled. We could make rust support conditional on those, and
>> then maybe a build-time check that it's actually all set to 0 ...
>>
>> Requiring the unsafe init just feels a bit disappointing to me, when the C
>> side (including lockdep annotations) tries really hard to make global
>> locks a one-liner.
>
> I actually have a prototype lying around that gets rid of the
> initialization step. The idea is to define some new macros:
>
> #define __ARCH_SPIN_LOCK_UNLOCKED { 0 }
> +#define __ARCH_SPIN_LOCK_UNLOCKED_TYP unsigned int
> +#define __ARCH_SPIN_LOCK_UNLOCKED_INT 0
>
> Rust then uses the two new #defines to initialize the raw spin lock to
> the right bytes. As long as __ARCH_SPIN_LOCK_UNLOCKED and
> __ARCH_SPIN_LOCK_UNLOCKED_INT are represented by the same sequence of
> bytes, it should work.
I have no idea if this is possible, but maybe one of the C experts can
help with this: could we define `__ARCH_SPIN_LOCK_UNLOCKED` in terms of
`__ARCH_SPIN_LOCK_UNLOCKED_INT`? That way there is only one source of
truth.
---
Cheers,
Benno
On Fri, Sep 13, 2024 at 04:52:02PM +0000, Benno Lossin wrote:
> On 13.09.24 18:10, Alice Ryhl wrote:
> > On Fri, Sep 13, 2024 at 6:02 PM Simona Vetter <simona.vetter@ffwll.ch> wrote:
> >> On Tue, Sep 10, 2024 at 02:23:34PM +0000, Alice Ryhl wrote:
> >>> Add support for creating global variables that are wrapped in a mutex or
> >>> spinlock. Optionally, the macro can generate a special LockedBy type
> >>> that does not require a runtime check.
> >>>
> >>> 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 bindgen refuses to
> >>> expose __ARCH_SPIN_LOCK_UNLOCKED to Rust as a compile-time constant. It
> >>> just generates an `extern "C"` global reference instead. In the future,
> >>> we could expose the value of __ARCH_SPIN_LOCK_UNLOCKED to Rust in a way
> >>> that Rust can understand. This would remove the need for a separate
> >>> initialization step.
> >>
> >> Yeah it's just a raw C struct initializer, I wouldn't even know how to
> >> move that to rust.
> >>
> >> An absolute horrible idea, and I didn't try whether it's even possible:
> >> - put all the global locks of a type into a special linker section (we
> >> need a macro anyway for them).
> >> - patch them up with a horrible linker script objtool patching with an
> >> example lock initialized by the C macro.
> >>
> >> Even worse idea, on most architectures/config it's all zeros. Iirc the one
> >> I've found that might matter a bit is CONFIG_SMP=n with some lock
> >> debugging enabled. We could make rust support conditional on those, and
> >> then maybe a build-time check that it's actually all set to 0 ...
> >>
> >> Requiring the unsafe init just feels a bit disappointing to me, when the C
> >> side (including lockdep annotations) tries really hard to make global
> >> locks a one-liner.
> >
> > I actually have a prototype lying around that gets rid of the
> > initialization step. The idea is to define some new macros:
> >
> > #define __ARCH_SPIN_LOCK_UNLOCKED { 0 }
> > +#define __ARCH_SPIN_LOCK_UNLOCKED_TYP unsigned int
> > +#define __ARCH_SPIN_LOCK_UNLOCKED_INT 0
> >
> > Rust then uses the two new #defines to initialize the raw spin lock to
> > the right bytes. As long as __ARCH_SPIN_LOCK_UNLOCKED and
> > __ARCH_SPIN_LOCK_UNLOCKED_INT are represented by the same sequence of
> > bytes, it should work.
>
> I have no idea if this is possible, but maybe one of the C experts can
> help with this: could we define `__ARCH_SPIN_LOCK_UNLOCKED` in terms of
> `__ARCH_SPIN_LOCK_UNLOCKED_INT`? That way there is only one source of
> truth.
Some arch are {{ 0 }}, but parisc is {{ 0x1a46, 0x1a46, 0x1a46, 0x1a46 }}
Everything else is reasonable though, with either 0 or 1 for unlucked and
one or two structs/unions around it (some arch though have named
initializers for that one field).
-Sima
--
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Hi Alice,
kernel test robot noticed the following build errors:
[auto build test ERROR on 93dc3be19450447a3a7090bd1dfb9f3daac3e8d2]
url: https://github.com/intel-lab-lkp/linux/commits/Alice-Ryhl/rust-add-global-lock-support/20240910-222519
base: 93dc3be19450447a3a7090bd1dfb9f3daac3e8d2
patch link: https://lore.kernel.org/r/20240910-static-mutex-v3-1-5bebd11bdf3b%40google.com
patch subject: [PATCH v3] rust: add global lock support
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20240911/202409112144.dcfxjqII-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240911/202409112144.dcfxjqII-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409112144.dcfxjqII-lkp@intel.com/
All errors (new ones prefixed by >>):
>> error[E0432]: unresolved import `super::__static_lock_ty_MY_COUNTER`
--> rust/doctests_kernel_generated.rs:4818:1
|
4818 | / kernel::sync::global_lock! {
4819 | | // SAFETY: Initialized in module initializer before first use.
4820 | | static MY_COUNTER: Mutex<u32> = unsafe { uninit };
4821 | | value: 0;
4822 | | }
| |_^ no `__static_lock_ty_MY_COUNTER` in the root
|
= note: this error originates in the macro `$crate::global_lock` which comes from the expansion of the macro `kernel::sync::global_lock` (in Nightly builds, run with -Z macro-backtrace for more info)
--
>> error[E0432]: unresolved import `super::__static_lock_ty_MY_MUTEX`
--> rust/doctests_kernel_generated.rs:4886:1
|
4886 | / kernel::sync::global_lock! {
4887 | | // SAFETY: Initialized in module initializer before first use.
4888 | | static MY_MUTEX: Mutex<()> = unsafe { uninit };
4889 | | value: ();
4890 | | guard: MyGuard;
4891 | | locked_by: LockedByMyMutex;
4892 | | }
| |_^ no `__static_lock_ty_MY_MUTEX` in the root
|
= note: this error originates in the macro `$crate::global_lock` which comes from the expansion of the macro `kernel::sync::global_lock` (in Nightly builds, run with -Z macro-backtrace for more info)
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
© 2016 - 2024 Red Hat, Inc.