Skip to content

Commit 6e4c596

Browse files
nbdd0121nbdd0121
committed
WIP: guard type split
1 parent 3900b07 commit 6e4c596

3 files changed

Lines changed: 91 additions & 86 deletions

File tree

‎rust/kernel/sync/lock.rs‎

Lines changed: 42 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -13,37 +13,13 @@ use macros::pin_data;
1313
pub mod mutex;
1414
pub mod spinlock;
1515

16-
/// The "backend" of a lock.
17-
///
18-
/// It is the actual implementation of the lock, without the need to repeat patterns used in all
19-
/// locks.
20-
///
21-
/// # Safety
22-
///
23-
/// - Implementers must ensure that only one thread/CPU may access the protected data once the lock
24-
/// is owned, that is, between calls to `lock` and `unlock`.
25-
/// - Implementers must also ensure that `relock` uses the same locking method as the original
26-
/// lock operation. For example, it should disable interrupts if [`IrqSaveBackend::lock_irqsave`]
27-
/// is used.
28-
pub unsafe trait Backend {
16+
pub unsafe trait EffectiveBackend {
2917
/// The state required by the lock.
3018
type State;
3119

3220
/// The state required to be kept between lock and unlock.
3321
type GuardState;
3422

35-
/// Initialises the lock.
36-
///
37-
/// # Safety
38-
///
39-
/// `ptr` must be valid for write for the duration of the call, while `name` and `key` must
40-
/// remain valid for read indefinitely.
41-
unsafe fn init(
42-
ptr: *mut Self::State,
43-
name: *const core::ffi::c_char,
44-
key: *mut bindings::lock_class_key,
45-
);
46-
4723
/// Acquires the lock, making the caller its owner.
4824
///
4925
/// # Safety
@@ -71,6 +47,32 @@ pub unsafe trait Backend {
7147
}
7248
}
7349

50+
/// The "backend" of a lock.
51+
///
52+
/// It is the actual implementation of the lock, without the need to repeat patterns used in all
53+
/// locks.
54+
///
55+
/// # Safety
56+
///
57+
/// - Implementers must ensure that only one thread/CPU may access the protected data once the lock
58+
/// is owned, that is, between calls to `lock` and `unlock`.
59+
/// - Implementers must also ensure that `relock` uses the same locking method as the original
60+
/// lock operation. For example, it should disable interrupts if [`IrqSaveBackend::lock_irqsave`]
61+
/// is used.
62+
pub unsafe trait Backend: EffectiveBackend {
63+
/// Initialises the lock.
64+
///
65+
/// # Safety
66+
///
67+
/// `ptr` must be valid for write for the duration of the call, while `name` and `key` must
68+
/// remain valid for read indefinitely.
69+
unsafe fn init(
70+
ptr: *mut Self::State,
71+
name: *const core::ffi::c_char,
72+
key: *mut bindings::lock_class_key,
73+
);
74+
}
75+
7476
/// The "backend" of a lock that supports the irq-save variant.
7577
///
7678
/// # Safety
@@ -82,16 +84,7 @@ pub unsafe trait Backend {
8284
/// must disable interrupts on lock, and restore interrupt state on unlock. Implementers may use
8385
/// [`Backend::GuardState`] to store state needed to keep track of the interrupt state.
8486
pub unsafe trait IrqSaveBackend: Backend {
85-
/// Acquires the lock, making the caller its owner.
86-
///
87-
/// Before acquiring the lock, it disables interrupts, and returns the previous interrupt state
88-
/// as its guard state so that the guard can restore it when it is dropped.
89-
///
90-
/// # Safety
91-
///
92-
/// Callers must ensure that [`Backend::init`] has been previously called.
93-
#[must_use]
94-
unsafe fn lock_irqsave(ptr: *mut Self::State) -> Self::GuardState;
87+
type IrqSaveBackend: EffectiveBackend<State = Self::State>;
9588
}
9689

9790
/// A mutual exclusion primitive.
@@ -154,10 +147,10 @@ impl<T: ?Sized, B: IrqSaveBackend> Lock<T, B> {
154147
/// Before acquiring the lock, it disables interrupts. When the guard is dropped, the interrupt
155148
/// state (either enabled or disabled) is restored to its state before
156149
/// [`lock_irqsave`](Self::lock_irqsave) was called.
157-
pub fn lock_irqsave(&self) -> Guard<'_, T, B> {
150+
pub fn lock_irqsave(&self) -> Guard<'_, T, B, B::IrqSaveBackend> {
158151
// SAFETY: The constructor of the type calls `init`, so the existence of the object proves
159152
// that `init` was called.
160-
let state = unsafe { B::lock_irqsave(self.state.get()) };
153+
let state = unsafe { B::IrqSaveBackend::lock(self.state.get()) };
161154
// SAFETY: The lock was just acquired.
162155
unsafe { Guard::new(self, state) }
163156
}
@@ -169,29 +162,29 @@ impl<T: ?Sized, B: IrqSaveBackend> Lock<T, B> {
169162
/// when a guard goes out of scope. It also provides a safe and convenient way to access the data
170163
/// protected by the lock.
171164
#[must_use = "the lock unlocks immediately when the guard is unused"]
172-
pub struct Guard<'a, T: ?Sized, B: Backend> {
165+
pub struct Guard<'a, T: ?Sized, B: Backend, E: EffectiveBackend<State = B::State> = B> {
173166
pub(crate) lock: &'a Lock<T, B>,
174-
pub(crate) state: B::GuardState,
167+
pub(crate) state: E::GuardState,
175168
_not_send: PhantomData<*mut ()>,
176169
}
177170

178171
// SAFETY: `Guard` is sync when the data protected by the lock is also sync.
179-
unsafe impl<T: Sync + ?Sized, B: Backend> Sync for Guard<'_, T, B> {}
172+
unsafe impl<T: Sync + ?Sized, B: Backend, E: EffectiveBackend<State = B::State>> Sync for Guard<'_, T, B, E> {}
180173

181-
impl<T: ?Sized, B: Backend> Guard<'_, T, B> {
174+
impl<T: ?Sized, B: Backend, E: EffectiveBackend<State = B::State>> Guard<'_, T, B, E> {
182175
pub(crate) fn do_unlocked(&mut self, cb: impl FnOnce()) {
183176
// SAFETY: The caller owns the lock, so it is safe to unlock it.
184-
unsafe { B::unlock(self.lock.state.get(), &self.state) };
177+
unsafe { E::unlock(self.lock.state.get(), &self.state) };
185178

186179
// SAFETY: The lock was just unlocked above and is being relocked now.
187180
let _relock =
188-
ScopeGuard::new(|| unsafe { B::relock(self.lock.state.get(), &mut self.state) });
181+
ScopeGuard::new(|| unsafe { E::relock(self.lock.state.get(), &mut self.state) });
189182

190183
cb();
191184
}
192185
}
193186

194-
impl<T: ?Sized, B: Backend> core::ops::Deref for Guard<'_, T, B> {
187+
impl<T: ?Sized, B: Backend, E: EffectiveBackend<State = B::State>> core::ops::Deref for Guard<'_, T, B, E> {
195188
type Target = T;
196189

197190
fn deref(&self) -> &Self::Target {
@@ -200,27 +193,27 @@ impl<T: ?Sized, B: Backend> core::ops::Deref for Guard<'_, T, B> {
200193
}
201194
}
202195

203-
impl<T: ?Sized, B: Backend> core::ops::DerefMut for Guard<'_, T, B> {
196+
impl<T: ?Sized, B: Backend, E: EffectiveBackend<State = B::State>> core::ops::DerefMut for Guard<'_, T, B, E> {
204197
fn deref_mut(&mut self) -> &mut Self::Target {
205198
// SAFETY: The caller owns the lock, so it is safe to deref the protected data.
206199
unsafe { &mut *self.lock.data.get() }
207200
}
208201
}
209202

210-
impl<T: ?Sized, B: Backend> Drop for Guard<'_, T, B> {
203+
impl<T: ?Sized, B: Backend, E: EffectiveBackend<State = B::State>> Drop for Guard<'_, T, B, E> {
211204
fn drop(&mut self) {
212205
// SAFETY: The caller owns the lock, so it is safe to unlock it.
213-
unsafe { B::unlock(self.lock.state.get(), &self.state) };
206+
unsafe { E::unlock(self.lock.state.get(), &self.state) };
214207
}
215208
}
216209

217-
impl<'a, T: ?Sized, B: Backend> Guard<'a, T, B> {
210+
impl<'a, T: ?Sized, B: Backend, E: EffectiveBackend<State = B::State>> Guard<'a, T, B, E> {
218211
/// Constructs a new immutable lock guard.
219212
///
220213
/// # Safety
221214
///
222215
/// The caller must ensure that it owns the lock.
223-
pub(crate) unsafe fn new(lock: &'a Lock<T, B>, state: B::GuardState) -> Self {
216+
pub(crate) unsafe fn new(lock: &'a Lock<T, B>, state: E::GuardState) -> Self {
224217
Self {
225218
lock,
226219
state,

‎rust/kernel/sync/lock/mutex.rs‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -90,20 +90,10 @@ pub type Mutex<T> = super::Lock<T, MutexBackend>;
9090
pub struct MutexBackend;
9191

9292
// SAFETY: The underlying kernel `struct mutex` object ensures mutual exclusion.
93-
unsafe impl super::Backend for MutexBackend {
93+
unsafe impl super::EffectiveBackend for MutexBackend {
9494
type State = bindings::mutex;
9595
type GuardState = ();
9696

97-
unsafe fn init(
98-
ptr: *mut Self::State,
99-
name: *const core::ffi::c_char,
100-
key: *mut bindings::lock_class_key,
101-
) {
102-
// SAFETY: The safety requirements ensure that `ptr` is valid for writes, and `name` and
103-
// `key` are valid for read indefinitely.
104-
unsafe { bindings::__mutex_init(ptr, name, key) }
105-
}
106-
10797
unsafe fn lock(ptr: *mut Self::State) -> Self::GuardState {
10898
// SAFETY: The safety requirements of this function ensure that `ptr` points to valid
10999
// memory, and that it has been initialised before.
@@ -116,3 +106,16 @@ unsafe impl super::Backend for MutexBackend {
116106
unsafe { bindings::mutex_unlock(ptr) };
117107
}
118108
}
109+
110+
// SAFETY: The underlying kernel `struct mutex` object ensures mutual exclusion.
111+
unsafe impl super::Backend for MutexBackend {
112+
unsafe fn init(
113+
ptr: *mut Self::State,
114+
name: *const core::ffi::c_char,
115+
key: *mut bindings::lock_class_key,
116+
) {
117+
// SAFETY: The safety requirements ensure that `ptr` is valid for writes, and `name` and
118+
// `key` are valid for read indefinitely.
119+
unsafe { bindings::__mutex_init(ptr, name, key) }
120+
}
121+
}

‎rust/kernel/sync/lock/spinlock.rs‎

Lines changed: 35 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -96,12 +96,31 @@ pub type SpinLock<T> = super::Lock<T, SpinLockBackend>;
9696
/// A kernel `spinlock_t` lock backend.
9797
pub struct SpinLockBackend;
9898

99+
pub struct SpinLockIrqSaveBackend;
100+
99101
// SAFETY: The underlying kernel `spinlock_t` object ensures mutual exclusion. `relock` uses the
100102
// same scheme as `unlock` to figure out which locking method was used originally.
101-
unsafe impl super::Backend for SpinLockBackend {
103+
unsafe impl super::EffectiveBackend for SpinLockBackend {
102104
type State = bindings::spinlock_t;
103-
type GuardState = Option<core::ffi::c_ulong>;
105+
type GuardState = ();
104106

107+
unsafe fn lock(ptr: *mut Self::State) -> Self::GuardState {
108+
// SAFETY: The safety requirements of this function ensure that `ptr` points to valid
109+
// memory, and that it has been initialised before.
110+
unsafe { bindings::spin_lock(ptr) };
111+
()
112+
}
113+
114+
unsafe fn unlock(ptr: *mut Self::State, _: &Self::GuardState) {
115+
unsafe { bindings::spin_unlock(ptr) }
116+
}
117+
118+
unsafe fn relock(ptr: *mut Self::State, _: &mut Self::GuardState) {
119+
unsafe { Self::lock(ptr) };
120+
}
121+
}
122+
123+
unsafe impl super::Backend for SpinLockBackend {
105124
unsafe fn init(
106125
ptr: *mut Self::State,
107126
name: *const core::ffi::c_char,
@@ -111,34 +130,28 @@ unsafe impl super::Backend for SpinLockBackend {
111130
// `key` are valid for read indefinitely.
112131
unsafe { bindings::__spin_lock_init(ptr, name, key) }
113132
}
133+
}
134+
135+
unsafe impl super::EffectiveBackend for SpinLockIrqSaveBackend {
136+
type State = bindings::spinlock_t;
137+
type GuardState = core::ffi::c_ulong;
114138

115139
unsafe fn lock(ptr: *mut Self::State) -> Self::GuardState {
116140
// SAFETY: The safety requirements of this function ensure that `ptr` points to valid
117141
// memory, and that it has been initialised before.
118-
unsafe { bindings::spin_lock(ptr) };
119-
None
142+
unsafe { bindings::spin_lock_irqsave(ptr) }
120143
}
121144

122145
unsafe fn unlock(ptr: *mut Self::State, guard_state: &Self::GuardState) {
123-
match guard_state {
124-
// SAFETY: The safety requirements of this function ensure that `ptr` is valid and that
125-
// the caller is the owner of the mutex.
126-
Some(flags) => unsafe { bindings::spin_unlock_irqrestore(ptr, *flags) },
127-
// SAFETY: The safety requirements of this function ensure that `ptr` is valid and that
128-
// the caller is the owner of the mutex.
129-
None => unsafe { bindings::spin_unlock(ptr) },
130-
}
146+
// SAFETY: The safety requirements of this function ensure that `ptr` is valid and that
147+
// the caller is the owner of the mutex.
148+
unsafe { bindings::spin_unlock_irqrestore(ptr, *guard_state) }
131149
}
132150

133-
unsafe fn relock(ptr: *mut Self::State, guard_state: &mut Self::GuardState) {
134-
let _ = match guard_state {
135-
// SAFETY: The safety requiments of this function ensure that `ptr` has been
136-
// initialised.
137-
None => unsafe { Self::lock(ptr) },
138-
// SAFETY: The safety requiments of this function ensure that `ptr` has been
139-
// initialised.
140-
Some(_) => unsafe { Self::lock_irqsave(ptr) },
141-
};
151+
unsafe fn relock(ptr: *mut Self::State, _guard_state: &mut Self::GuardState) {
152+
// SAFETY: The safety requiments of this function ensure that `ptr` has been
153+
// initialised.
154+
unsafe { bindings::spin_lock_irqsave(ptr); }
142155
}
143156
}
144157

@@ -147,9 +160,5 @@ unsafe impl super::Backend for SpinLockBackend {
147160
// interrupt state, and the `irqrestore` variant of the lock release functions to restore the state
148161
// in `unlock` -- we use the guard context to determine which method was used to acquire the lock.
149162
unsafe impl IrqSaveBackend for SpinLockBackend {
150-
unsafe fn lock_irqsave(ptr: *mut Self::State) -> Self::GuardState {
151-
// SAFETY: The safety requirements of this function ensure that `ptr` points to valid
152-
// memory, and that it has been initialised before.
153-
Some(unsafe { bindings::spin_lock_irqsave(ptr) })
154-
}
163+
type IrqSaveBackend = SpinLockIrqSaveBackend;
155164
}

0 commit comments

Comments
 (0)