| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ZxZxzjEaSZ8e_6mn@boqun-archlinux>
Date: Mon, 21 Oct 2024 08:22:54 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: Miguel Ojeda <ojeda@...nel.org>, Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>, Will Deacon <will@...nel.org>,
Waiman Long <longman@...hat.com>, Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <benno.lossin@...ton.me>,
rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org,
Andreas Hindborg <a.hindborg@...nel.org>
Subject: Re: [PATCH v5] rust: add global lock support
On Mon, Oct 21, 2024 at 01:17:23PM +0000, Alice Ryhl wrote:
[...]
> +///
> +/// A global mutex used to protect all instances of a given struct.
> +///
> +/// ```
> +/// # mod ex {
> +/// # use kernel::prelude::*;
> +/// kernel::sync::global_lock! {
> +/// // SAFETY: Initialized in module initializer before first use.
> +/// unsafe(uninit) static MY_MUTEX: Mutex<(), Guard = MyGuard, LockedBy = LockedByMyMutex> = ();
Thanks! This looks much better now ;-)
But I still want to get rid of "LockedBy=", so I've tried and seems it
works, please see the below diff on top of your patch, I think it's
better because:
* Users don't to pick up the names for the locked_by type ;-)
* It moves a significant amount of code out of macros.
* By having:
struct MyStruct {
my_counter: GlobalLockedBy<MyGuard, u32>,
}
, it's much clear for users to see which guard is used to protected
`my_counter`.
I prefer this way. Any concern about doing this?
Regards,
Boqun
> +/// }
> +///
> +/// /// 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 {}
> +/// # }
> +/// ```
[...]
----------------------------------->8
diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
index 9b3b401f3fcc..0d227541faef 100644
--- a/rust/kernel/sync/lock.rs
+++ b/rust/kernel/sync/lock.rs
@@ -15,6 +15,8 @@
pub(super) mod global;
+pub use global::{GlobalGuard, 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
index 803f19db4545..bef188938d5a 100644
--- a/rust/kernel/sync/lock/global.rs
+++ b/rust/kernel/sync/lock/global.rs
@@ -4,6 +4,63 @@
//! Support for defining statics containing locks.
+use core::{cell::UnsafeCell, marker::PhantomData};
+
+/// A marker for the guard type of a global lock.
+///
+/// # Safety
+///
+/// Implementers must guarantee that the type is a guard type of a global lock, that is, the
+/// existence of the object represents a unique global lock is held.
+pub unsafe trait GlobalGuard { }
+
+/// Data protected by a global lock.
+pub struct GlobalLockedBy<G: GlobalGuard, T: ?Sized>(PhantomData<fn(&G)>, UnsafeCell<T>);
+
+impl<G: GlobalGuard, T> GlobalLockedBy<G, T> {
+ /// Creates a new data.
+ pub const fn new(val: T) -> Self {
+ Self(PhantomData, UnsafeCell::new(val))
+ }
+}
+
+impl<G: GlobalGuard, T: ?Sized> GlobalLockedBy<G, T> {
+ /// Returns the immutable reference to the data with the lock held.
+ ///
+ /// With an immutable reference of the [`GlobalGuard`], the function is safe because the
+ /// corresponding global lock is held.
+ pub fn as_ref(&self, _guard: &G) -> &T {
+ // SAFETY: Per the safety requirement of `GlobalGuard`, the lock is held, and with the
+ // shared reference of the guard, it's safe to return an immutable reference to the object.
+ unsafe { &*self.1.get() }
+ }
+
+ /// Returns the mutable reference to the data with the lock held.
+ ///
+ /// With a mutable reference of the [`GlobalGuard`], the function is safe because the
+ /// corresponding global lock is held, and the exclusive reference of the guard guarantees the
+ /// exclusive access of `T`.
+ #[allow(clippy::mut_from_ref)]
+ pub fn as_mut(&self, _guard: &mut G) -> &mut T {
+ // SAFETY: Per the safety requirement of `GlobalGuard`, the lock is held, and with the
+ // exclusive reference of the guard, it's safe to return a mutable reference to the object.
+ unsafe { &mut *self.1.get() }
+ }
+
+ /// Returns the mutable references to the data.
+ pub fn get_mut(&mut self) -> &mut T {
+ self.1.get_mut()
+ }
+}
+
+// SAFETY: `GlobalLockedBy` can be transferred across thread boundaries iff the data it protects
+// can.
+unsafe impl<G: GlobalGuard, T: ?Sized + Send> Send for GlobalLockedBy<G, T> {}
+
+// SAFETY: `GlobalLockedBy` serialises the interior mutability it provides, so it is `Sync` as long
+// as the data it protects is `Send`.
+unsafe impl<G: GlobalGuard, T: ?Sized + Send> Sync for GlobalLockedBy<G, T> {}
+
/// Defines a global lock.
///
/// The global mutex must be initialized before first use. Usually this is done by calling `init`
@@ -44,14 +101,15 @@
/// ```
/// # mod ex {
/// # use kernel::prelude::*;
+/// use kernel::sync::lock::GlobalLockedBy;
/// kernel::sync::global_lock! {
/// // SAFETY: Initialized in module initializer before first use.
-/// unsafe(uninit) static MY_MUTEX: Mutex<(), Guard = MyGuard, LockedBy = LockedByMyMutex> = ();
+/// unsafe(uninit) static MY_MUTEX: Mutex<(), Guard = MyGuard> = ();
/// }
///
/// /// All instances of this struct are protected by `MY_MUTEX`.
/// struct MyStruct {
-/// my_counter: LockedByMyMutex<u32>,
+/// my_counter: GlobalLockedBy<MyGuard, u32>,
/// }
///
/// impl MyStruct {
@@ -81,7 +139,7 @@ macro_rules! global_lock {
{
$(#[$meta:meta])* $pub:vis
unsafe(uninit) static $name:ident: $kind:ident<$valuety:ty
- $(, Guard = $guard:ident $(, LockedBy = $locked_by:ident)?)?
+ $(, Guard = $guard:ident)?
> = $value:expr;
} => {
$crate::macros::paste! {
@@ -167,44 +225,13 @@ fn deref_mut(&mut self) -> &mut Val {
}
}
- $(
- pub struct $locked_by<T: ?Sized>(::core::cell::UnsafeCell<T>);
-
- // SAFETY: `LockedBy` can be transferred across thread boundaries iff the data it
- // protects can.
- unsafe impl<T: ?Sized + Send> Send for $locked_by<T> {}
-
- // SAFETY: `LockedBy` serialises the interior mutability it provides, so it is `Sync` as long as the
- // data it protects is `Send`.
- unsafe impl<T: ?Sized + Send> Sync for $locked_by<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()
- }
- }
- )?)?
+ // SAFETY: `$guard` is a guard type for a unique global lock.
+ unsafe impl $crate::sync::lock::GlobalGuard for $guard {}
+ )?
}
use [< __static_lock_mod_ $name >]::[< __static_lock_wrapper_ $name >];
- $( $pub use [< __static_lock_mod_ $name >]::$guard;
- $( $pub use [< __static_lock_mod_ $name >]::$locked_by; )?)?
+ $( $pub use [< __static_lock_mod_ $name >]::$guard;)?
$(#[$meta])*
#[allow(private_interfaces)]
Powered by blists - more mailing lists