[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20230714-classless_lockdep-v1-6-229b9671ce31@asahilina.net>
Date: Fri, 14 Jul 2023 18:13:58 +0900
From: Asahi Lina <lina@...hilina.net>
To: Miguel Ojeda <ojeda@...nel.org>,
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>,
Masahiro Yamada <masahiroy@...nel.org>,
Nathan Chancellor <nathan@...nel.org>,
Nick Desaulniers <ndesaulniers@...gle.com>,
Nicolas Schier <nicolas@...sle.eu>, Tom Rix <trix@...hat.com>,
Daniel Vetter <daniel@...ll.ch>
Cc: Hector Martin <marcan@...can.st>, Sven Peter <sven@...npeter.dev>,
Alyssa Rosenzweig <alyssa@...enzweig.io>,
asahi@...ts.linux.dev, rust-for-linux@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-kbuild@...r.kernel.org,
llvm@...ts.linux.dev, Asahi Lina <lina@...hilina.net>
Subject: [PATCH RFC 06/11] rust: sync: Replace static LockClassKey refs
with a pointer wrapper
We want to be able to handle dynamic lock class creation and using
pointers to things that aren't a real lock_class_key as lock classes.
Doing this by casting around Rust references is difficult without
accidentally invoking UB.
Instead, switch LockClassKey to being a raw pointer wrapper around a
lock_class_key, which means there is no UB possible on the Rust side
just by creating and consuming these objects. The C code also should
never actually dereference lock classes, only use their address
(possibly with an offset).
We still provide a dummy ZST version of this wrapper, to be used when
lockdep is disabled.
Signed-off-by: Asahi Lina <lina@...hilina.net>
---
rust/kernel/sync.rs | 6 +++---
rust/kernel/sync/condvar.rs | 2 +-
rust/kernel/sync/lock.rs | 4 ++--
rust/kernel/sync/lockdep.rs | 27 ++++++++++++++++++++++-----
rust/kernel/sync/no_lockdep.rs | 15 +++++++++++++--
rust/kernel/types.rs | 2 +-
6 files changed, 42 insertions(+), 14 deletions(-)
diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
index 352472c6b77a..49286c3e0ff3 100644
--- a/rust/kernel/sync.rs
+++ b/rust/kernel/sync.rs
@@ -20,7 +20,7 @@
pub use arc::{Arc, ArcBorrow, UniqueArc};
pub use condvar::CondVar;
pub use lock::{mutex::Mutex, spinlock::SpinLock};
-pub use lockdep::LockClassKey;
+pub use lockdep::{LockClassKey, StaticLockClassKey};
pub use locked_by::LockedBy;
/// Defines a new static lock class and returns a pointer to it.
@@ -28,8 +28,8 @@
#[macro_export]
macro_rules! static_lock_class {
() => {{
- static CLASS: $crate::sync::LockClassKey = $crate::sync::LockClassKey::new();
- &CLASS
+ static CLASS: $crate::sync::StaticLockClassKey = $crate::sync::StaticLockClassKey::new();
+ CLASS.key()
}};
}
diff --git a/rust/kernel/sync/condvar.rs b/rust/kernel/sync/condvar.rs
index ed353399c4e5..3bccb2c6ef84 100644
--- a/rust/kernel/sync/condvar.rs
+++ b/rust/kernel/sync/condvar.rs
@@ -92,7 +92,7 @@ unsafe impl Sync for CondVar {}
impl CondVar {
/// Constructs a new condvar initialiser.
#[allow(clippy::new_ret_no_self)]
- pub fn new(name: &'static CStr, key: &'static LockClassKey) -> impl PinInit<Self> {
+ pub fn new(name: &'static CStr, key: LockClassKey) -> impl PinInit<Self> {
pin_init!(Self {
_pin: PhantomPinned,
// SAFETY: `slot` is valid while the closure is called and both `name` and `key` have
diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
index d493c5d19104..8e71e7aa2cc1 100644
--- a/rust/kernel/sync/lock.rs
+++ b/rust/kernel/sync/lock.rs
@@ -103,7 +103,7 @@ unsafe impl<T: ?Sized + Send, B: Backend> Sync for Lock<T, B> {}
impl<T, B: Backend> Lock<T, B> {
/// Constructs a new lock initialiser.
#[allow(clippy::new_ret_no_self)]
- pub fn new(t: T, name: &'static CStr, key: &'static LockClassKey) -> impl PinInit<Self> {
+ pub fn new(t: T, name: &'static CStr, key: LockClassKey) -> impl PinInit<Self> {
pin_init!(Self {
data: UnsafeCell::new(t),
_pin: PhantomPinned,
@@ -119,7 +119,7 @@ pub fn new(t: T, name: &'static CStr, key: &'static LockClassKey) -> impl PinIni
pub fn pin_init<E>(
t: impl PinInit<T, E>,
name: &'static CStr,
- key: &'static LockClassKey,
+ key: LockClassKey,
) -> impl PinInit<Self, E>
where
E: core::convert::From<core::convert::Infallible>,
diff --git a/rust/kernel/sync/lockdep.rs b/rust/kernel/sync/lockdep.rs
index cb68b18dc0ad..d8328f4275fb 100644
--- a/rust/kernel/sync/lockdep.rs
+++ b/rust/kernel/sync/lockdep.rs
@@ -9,19 +9,36 @@
/// Represents a lockdep class. It's a wrapper around C's `lock_class_key`.
#[repr(transparent)]
-pub struct LockClassKey(Opaque<bindings::lock_class_key>);
+pub struct StaticLockClassKey(Opaque<bindings::lock_class_key>);
-impl LockClassKey {
+impl StaticLockClassKey {
/// Creates a new lock class key.
pub const fn new() -> Self {
Self(Opaque::uninit())
}
+ /// Returns the lock class key reference for this static lock class.
+ pub const fn key(&self) -> LockClassKey {
+ LockClassKey(self.0.get())
+ }
+}
+
+// SAFETY: `bindings::lock_class_key` just represents an opaque memory location, and is never
+// actually dereferenced.
+unsafe impl Sync for StaticLockClassKey {}
+
+/// A reference to a lock class key. This is a raw pointer to a lock_class_key,
+/// which is required to have a static lifetime.
+#[derive(Copy, Clone)]
+pub struct LockClassKey(*mut bindings::lock_class_key);
+
+impl LockClassKey {
pub(crate) fn as_ptr(&self) -> *mut bindings::lock_class_key {
- self.0.get()
+ self.0
}
}
-// SAFETY: `bindings::lock_class_key` is designed to be used concurrently from multiple threads and
-// provides its own synchronization.
+// SAFETY: `bindings::lock_class_key` just represents an opaque memory location, and is never
+// actually dereferenced.
+unsafe impl Send for LockClassKey {}
unsafe impl Sync for LockClassKey {}
diff --git a/rust/kernel/sync/no_lockdep.rs b/rust/kernel/sync/no_lockdep.rs
index 69d42e8d9801..518ec0bf9a7d 100644
--- a/rust/kernel/sync/no_lockdep.rs
+++ b/rust/kernel/sync/no_lockdep.rs
@@ -5,14 +5,25 @@
//! Takes the place of the `lockdep` module when lockdep is disabled.
/// A dummy, zero-sized lock class.
-pub struct LockClassKey();
+pub struct StaticLockClassKey();
-impl LockClassKey {
+impl StaticLockClassKey {
/// Creates a new dummy lock class key.
pub const fn new() -> Self {
Self()
}
+ /// Returns the lock class key reference for this static lock class.
+ pub const fn key(&self) -> LockClassKey {
+ LockClassKey()
+ }
+}
+
+/// A dummy reference to a lock class key.
+#[derive(Copy, Clone)]
+pub struct LockClassKey();
+
+impl LockClassKey {
pub(crate) fn as_ptr(&self) -> *mut bindings::lock_class_key {
core::ptr::null_mut()
}
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 185d3493857e..91739bf71cc3 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -262,7 +262,7 @@ pub fn ffi_init(init_func: impl FnOnce(*mut T)) -> impl PinInit<Self> {
}
/// Returns a raw pointer to the opaque data.
- pub fn get(&self) -> *mut T {
+ pub const fn get(&self) -> *mut T {
UnsafeCell::raw_get(self.0.as_ptr())
}
--
2.40.1
Powered by blists - more mailing lists