Skip to content

Commit f73ca66

Browse files
chessturoIngo Molnar
authored andcommitted
rust: lockdep: Use Pin for all LockClassKey usages
Reintroduce dynamically-allocated LockClassKeys such that they are automatically (de)registered. Require that all usages of LockClassKeys ensure that they are Pin'd. Currently, only `'static` LockClassKeys are supported, so Pin is redundant. However, it is intended that dynamically-allocated LockClassKeys will eventually be supported, so using Pin from the outset will make that change simpler. Closes: #1102 Suggested-by: Benno Lossin <benno.lossin@proton.me> Suggested-by: Boqun Feng <boqun.feng@gmail.com> Signed-off-by: Mitchell Levy <levymitchell0@gmail.com> Signed-off-by: Boqun Feng <boqun.feng@gmail.com> Signed-off-by: Ingo Molnar <mingo@kernel.org> Reviewed-by: Benno Lossin <benno.lossin@proton.me> Link: https://lore.kernel.org/r/20250307232717.1759087-12-boqun.feng@gmail.com
1 parent 70b9c85 commit f73ca66

File tree

8 files changed

+77
-12
lines changed

8 files changed

+77
-12
lines changed

rust/helpers/helpers.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "signal.c"
3131
#include "slab.c"
3232
#include "spinlock.c"
33+
#include "sync.c"
3334
#include "task.c"
3435
#include "uaccess.c"
3536
#include "vmalloc.c"

rust/helpers/sync.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// SPDX-License-Identifier: GPL-2.0
2+
3+
#include <linux/lockdep.h>
4+
5+
void rust_helper_lockdep_register_key(struct lock_class_key *k)
6+
{
7+
lockdep_register_key(k);
8+
}
9+
10+
void rust_helper_lockdep_unregister_key(struct lock_class_key *k)
11+
{
12+
lockdep_unregister_key(k);
13+
}

rust/kernel/sync.rs

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
//! This module contains the kernel APIs related to synchronisation that have been ported or
66
//! wrapped for usage by Rust code in the kernel.
77
8+
use crate::pin_init;
9+
use crate::prelude::*;
810
use crate::types::Opaque;
911

1012
mod arc;
@@ -23,15 +25,64 @@ pub use locked_by::LockedBy;
2325

2426
/// Represents a lockdep class. It's a wrapper around C's `lock_class_key`.
2527
#[repr(transparent)]
26-
pub struct LockClassKey(Opaque<bindings::lock_class_key>);
28+
#[pin_data(PinnedDrop)]
29+
pub struct LockClassKey {
30+
#[pin]
31+
inner: Opaque<bindings::lock_class_key>,
32+
}
2733

2834
// SAFETY: `bindings::lock_class_key` is designed to be used concurrently from multiple threads and
2935
// provides its own synchronization.
3036
unsafe impl Sync for LockClassKey {}
3137

3238
impl LockClassKey {
39+
/// Initializes a dynamically allocated lock class key. In the common case of using a
40+
/// statically allocated lock class key, the static_lock_class! macro should be used instead.
41+
///
42+
/// # Example
43+
/// ```
44+
/// # use kernel::{c_str, stack_pin_init};
45+
/// # use kernel::alloc::KBox;
46+
/// # use kernel::types::ForeignOwnable;
47+
/// # use kernel::sync::{LockClassKey, SpinLock};
48+
///
49+
/// let key = KBox::pin_init(LockClassKey::new_dynamic(), GFP_KERNEL)?;
50+
/// let key_ptr = key.into_foreign();
51+
///
52+
/// {
53+
/// stack_pin_init!(let num: SpinLock<u32> = SpinLock::new(
54+
/// 0,
55+
/// c_str!("my_spinlock"),
56+
/// // SAFETY: `key_ptr` is returned by the above `into_foreign()`, whose
57+
/// // `from_foreign()` has not yet been called.
58+
/// unsafe { <Pin<KBox<LockClassKey>> as ForeignOwnable>::borrow(key_ptr) }
59+
/// ));
60+
/// }
61+
///
62+
/// // SAFETY: We dropped `num`, the only use of the key, so the result of the previous
63+
/// // `borrow` has also been dropped. Thus, it's safe to use from_foreign.
64+
/// unsafe { drop(<Pin<KBox<LockClassKey>> as ForeignOwnable>::from_foreign(key_ptr)) };
65+
///
66+
/// # Ok::<(), Error>(())
67+
/// ```
68+
pub fn new_dynamic() -> impl PinInit<Self> {
69+
pin_init!(Self {
70+
// SAFETY: lockdep_register_key expects an uninitialized block of memory
71+
inner <- Opaque::ffi_init(|slot| unsafe { bindings::lockdep_register_key(slot) })
72+
})
73+
}
74+
3375
pub(crate) fn as_ptr(&self) -> *mut bindings::lock_class_key {
34-
self.0.get()
76+
self.inner.get()
77+
}
78+
}
79+
80+
#[pinned_drop]
81+
impl PinnedDrop for LockClassKey {
82+
fn drop(self: Pin<&mut Self>) {
83+
// SAFETY: self.as_ptr was registered with lockdep and self is pinned, so the address
84+
// hasn't changed. Thus, it's safe to pass to unregister.
85+
unsafe { bindings::lockdep_unregister_key(self.as_ptr()) }
3586
}
3687
}
3788

@@ -44,7 +95,7 @@ macro_rules! static_lock_class {
4495
// SAFETY: lockdep expects uninitialized memory when it's handed a statically allocated
4596
// lock_class_key
4697
unsafe { ::core::mem::MaybeUninit::uninit().assume_init() };
47-
&CLASS
98+
$crate::prelude::Pin::static_ref(&CLASS)
4899
}};
49100
}
50101

rust/kernel/sync/condvar.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,7 @@ use crate::{
1717
time::Jiffies,
1818
types::Opaque,
1919
};
20-
use core::marker::PhantomPinned;
21-
use core::ptr;
20+
use core::{marker::PhantomPinned, pin::Pin, ptr};
2221
use macros::pin_data;
2322

2423
/// Creates a [`CondVar`] initialiser with the given name and a newly-created lock class.
@@ -103,7 +102,7 @@ unsafe impl Sync for CondVar {}
103102

104103
impl CondVar {
105104
/// Constructs a new condvar initialiser.
106-
pub fn new(name: &'static CStr, key: &'static LockClassKey) -> impl PinInit<Self> {
105+
pub fn new(name: &'static CStr, key: Pin<&'static LockClassKey>) -> impl PinInit<Self> {
107106
pin_init!(Self {
108107
_pin: PhantomPinned,
109108
// SAFETY: `slot` is valid while the closure is called and both `name` and `key` have

rust/kernel/sync/lock.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use crate::{
1212
str::CStr,
1313
types::{NotThreadSafe, Opaque, ScopeGuard},
1414
};
15-
use core::{cell::UnsafeCell, marker::PhantomPinned};
15+
use core::{cell::UnsafeCell, marker::PhantomPinned, pin::Pin};
1616
use macros::pin_data;
1717

1818
pub mod mutex;
@@ -129,7 +129,7 @@ unsafe impl<T: ?Sized + Send, B: Backend> Sync for Lock<T, B> {}
129129

130130
impl<T, B: Backend> Lock<T, B> {
131131
/// Constructs a new lock initialiser.
132-
pub fn new(t: T, name: &'static CStr, key: &'static LockClassKey) -> impl PinInit<Self> {
132+
pub fn new(t: T, name: &'static CStr, key: Pin<&'static LockClassKey>) -> impl PinInit<Self> {
133133
pin_init!(Self {
134134
data: UnsafeCell::new(t),
135135
_pin: PhantomPinned,

rust/kernel/sync/lock/global.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use crate::{
1313
use core::{
1414
cell::UnsafeCell,
1515
marker::{PhantomData, PhantomPinned},
16+
pin::Pin,
1617
};
1718

1819
/// Trait implemented for marker types for global locks.
@@ -26,7 +27,7 @@ pub trait GlobalLockBackend {
2627
/// The backend used for this global lock.
2728
type Backend: Backend + 'static;
2829
/// The class for this global lock.
29-
fn get_lock_class() -> &'static LockClassKey;
30+
fn get_lock_class() -> Pin<&'static LockClassKey>;
3031
}
3132

3233
/// Type used for global locks.
@@ -270,7 +271,7 @@ macro_rules! global_lock {
270271
type Item = $valuety;
271272
type Backend = $crate::global_lock_inner!(backend $kind);
272273

273-
fn get_lock_class() -> &'static $crate::sync::LockClassKey {
274+
fn get_lock_class() -> Pin<&'static $crate::sync::LockClassKey> {
274275
$crate::static_lock_class!()
275276
}
276277
}

rust/kernel/sync/poll.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ pub struct PollCondVar {
8989

9090
impl PollCondVar {
9191
/// Constructs a new condvar initialiser.
92-
pub fn new(name: &'static CStr, key: &'static LockClassKey) -> impl PinInit<Self> {
92+
pub fn new(name: &'static CStr, key: Pin<&'static LockClassKey>) -> impl PinInit<Self> {
9393
pin_init!(Self {
9494
inner <- CondVar::new(name, key),
9595
})

rust/kernel/workqueue.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,7 @@ unsafe impl<T: ?Sized, const ID: u64> Sync for Work<T, ID> {}
369369
impl<T: ?Sized, const ID: u64> Work<T, ID> {
370370
/// Creates a new instance of [`Work`].
371371
#[inline]
372-
pub fn new(name: &'static CStr, key: &'static LockClassKey) -> impl PinInit<Self>
372+
pub fn new(name: &'static CStr, key: Pin<&'static LockClassKey>) -> impl PinInit<Self>
373373
where
374374
T: WorkItem<ID>,
375375
{

0 commit comments

Comments
 (0)