Skip to content

Commit c76ea62

Browse files
ojedaojeda
authored
Merge pull request #392 from wedsonaf/init-sync
rust: require mutable references when initialising sync primitives.
2 parents ac48d0a + 8168bc1 commit c76ea62

File tree

12 files changed

+54
-49
lines changed

12 files changed

+54
-49
lines changed

‎drivers/android/context.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ impl Context {
3737
let ctx = Arc::get_mut(&mut ctx_ref).unwrap();
3838

3939
// SAFETY: `manager` is also pinned when `ctx` is.
40-
let manager = unsafe { Pin::new_unchecked(&ctx.manager) };
40+
let manager = unsafe { Pin::new_unchecked(&mut ctx.manager) };
4141
kernel::mutex_init!(manager, "Context::manager");
4242

4343
// SAFETY: `ctx_ref` is pinned behind the `Arc` reference.

‎drivers/android/node.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,9 @@ impl NodeDeath {
9494
}
9595
}
9696

97-
pub(crate) fn init(self: Pin<&Self>) {
97+
pub(crate) fn init(self: Pin<&mut Self>) {
9898
// SAFETY: `inner` is pinned when `self` is.
99-
let inner = unsafe { self.map_unchecked(|s| &s.inner) };
99+
let inner = unsafe { self.map_unchecked_mut(|n| &mut n.inner) };
100100
kernel::spinlock_init!(inner, "NodeDeath::inner");
101101
}
102102

‎drivers/android/process.rs

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -300,12 +300,12 @@ impl Process {
300300
// SAFETY: `node_refs` is initialised in the call to `mutex_init` below.
301301
node_refs: unsafe { Mutex::new(ProcessNodeRefs::new()) },
302302
},
303-
|process| {
304-
// SAFETY: `inner` is pinned behind the `Ref` reference.
305-
let pinned = unsafe { Pin::new_unchecked(&process.inner) };
303+
|mut process| {
304+
// SAFETY: `inner` is pinned when `Process` is.
305+
let pinned = unsafe { process.as_mut().map_unchecked_mut(|p| &mut p.inner) };
306306
kernel::mutex_init!(pinned, "Process::inner");
307-
// SAFETY: `node_refs` is pinned behind the `Ref` reference.
308-
let pinned = unsafe { Pin::new_unchecked(&process.node_refs) };
307+
// SAFETY: `node_refs` is pinned when `Process` is.
308+
let pinned = unsafe { process.as_mut().map_unchecked_mut(|p| &mut p.node_refs) };
309309
kernel::mutex_init!(pinned, "Process::node_refs");
310310
},
311311
)
@@ -720,10 +720,15 @@ impl Process {
720720
}
721721

722722
// SAFETY: `init` is called below.
723-
let death = death
723+
let mut death = death
724724
.commit(unsafe { NodeDeath::new(info.node_ref.node.clone(), self.clone(), cookie) });
725-
// SAFETY: `death` is pinned behind the `Arc` reference.
726-
unsafe { Pin::new_unchecked(death.as_ref()) }.init();
725+
726+
{
727+
let mutable = Arc::get_mut(&mut death).ok_or(Error::EINVAL)?;
728+
// SAFETY: `mutable` is pinned behind the `Arc` reference.
729+
unsafe { Pin::new_unchecked(mutable) }.init();
730+
}
731+
727732
info.death = Some(death.clone());
728733

729734
// Register the death notification.

‎drivers/android/thread.rs

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -247,9 +247,12 @@ impl Thread {
247247
})?;
248248
let thread = Arc::get_mut(&mut arc).unwrap();
249249
// SAFETY: `inner` is pinned behind the `Arc` reference.
250-
let inner = unsafe { Pin::new_unchecked(&thread.inner) };
250+
let inner = unsafe { Pin::new_unchecked(&mut thread.inner) };
251251
kernel::spinlock_init!(inner, "Thread::inner");
252-
kernel::condvar_init!(thread.pinned_condvar(), "Thread::work_condvar");
252+
253+
// SAFETY: `work_condvar` is pinned behind the `Arc` reference.
254+
let condvar = unsafe { Pin::new_unchecked(&mut thread.work_condvar) };
255+
kernel::condvar_init!(condvar, "Thread::work_condvar");
253256
{
254257
let mut inner = arc.inner.lock();
255258
inner.set_reply_work(reply_work);
@@ -258,10 +261,6 @@ impl Thread {
258261
Ok(arc)
259262
}
260263

261-
fn pinned_condvar(&self) -> Pin<&CondVar> {
262-
unsafe { Pin::new_unchecked(&self.work_condvar) }
263-
}
264-
265264
pub(crate) fn set_current_transaction(&self, transaction: Arc<Transaction>) {
266265
self.inner.lock().current_transaction = Some(transaction);
267266
}
@@ -276,15 +275,14 @@ impl Thread {
276275
}
277276

278277
// Loop waiting only on the local queue (i.e., not registering with the process queue).
279-
let cv = self.pinned_condvar();
280278
let mut inner = self.inner.lock();
281279
loop {
282280
if let Some(work) = inner.pop_work() {
283281
return Ok(work);
284282
}
285283

286284
inner.looper_flags |= LOOPER_WAITING;
287-
let signal_pending = cv.wait(&mut inner);
285+
let signal_pending = self.work_condvar.wait(&mut inner);
288286
inner.looper_flags &= !LOOPER_WAITING;
289287

290288
if signal_pending {
@@ -321,15 +319,14 @@ impl Thread {
321319
Either::Right(reg) => reg,
322320
};
323321

324-
let cv = self.pinned_condvar();
325322
let mut inner = self.inner.lock();
326323
loop {
327324
if let Some(work) = inner.pop_work() {
328325
return Ok(work);
329326
}
330327

331328
inner.looper_flags |= LOOPER_WAITING;
332-
let signal_pending = cv.wait(&mut inner);
329+
let signal_pending = self.work_condvar.wait(&mut inner);
333330
inner.looper_flags &= !LOOPER_WAITING;
334331

335332
if signal_pending {
@@ -352,7 +349,7 @@ impl Thread {
352349
}
353350
inner.push_work(work);
354351
}
355-
self.pinned_condvar().notify_one();
352+
self.work_condvar.notify_one();
356353
Ok(())
357354
}
358355

@@ -535,7 +532,7 @@ impl Thread {
535532
}
536533

537534
// Notify the thread now that we've released the inner lock.
538-
self.pinned_condvar().notify_one();
535+
self.work_condvar.notify_one();
539536
false
540537
}
541538

@@ -779,7 +776,7 @@ impl Thread {
779776

780777
// Now that the lock is no longer held, notify the waiters if we have to.
781778
if notify {
782-
self.pinned_condvar().notify_one();
779+
self.work_condvar.notify_one();
783780
}
784781
}
785782

@@ -802,7 +799,7 @@ impl Thread {
802799
// Remove epoll items if polling was ever used on the thread.
803800
let poller = self.inner.lock().looper_flags & LOOPER_POLL != 0;
804801
if poller {
805-
self.pinned_condvar().free_waiters();
802+
self.work_condvar.free_waiters();
806803

807804
unsafe { bindings::synchronize_rcu() };
808805
}

‎drivers/android/transaction.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ impl Transaction {
7878
let mut_tr = Arc::get_mut(&mut tr).ok_or(Error::EINVAL)?;
7979

8080
// SAFETY: `inner` is pinned behind `Arc`.
81-
let pinned = unsafe { Pin::new_unchecked(&mut_tr.inner) };
81+
let pinned = unsafe { Pin::new_unchecked(&mut mut_tr.inner) };
8282
kernel::spinlock_init!(pinned, "Transaction::inner");
8383
Ok(tr)
8484
}
@@ -112,7 +112,7 @@ impl Transaction {
112112
let mut_tr = Arc::get_mut(&mut tr).ok_or(Error::EINVAL)?;
113113

114114
// SAFETY: `inner` is pinned behind `Arc`.
115-
let pinned = unsafe { Pin::new_unchecked(&mut_tr.inner) };
115+
let pinned = unsafe { Pin::new_unchecked(&mut mut_tr.inner) };
116116
kernel::spinlock_init!(pinned, "Transaction::inner");
117117
Ok(tr)
118118
}

‎rust/kernel/sync/condvar.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ impl CondVar {
131131
}
132132

133133
impl NeedsLockClass for CondVar {
134-
unsafe fn init(self: Pin<&Self>, name: &'static CStr, key: *mut bindings::lock_class_key) {
134+
unsafe fn init(self: Pin<&mut Self>, name: &'static CStr, key: *mut bindings::lock_class_key) {
135135
unsafe { bindings::__init_waitqueue_head(self.wait_list.get(), name.as_char_ptr(), key) };
136136
}
137137
}

‎rust/kernel/sync/mod.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,11 @@
1111
//! # use kernel::prelude::*;
1212
//! # use kernel::mutex_init;
1313
//! # use kernel::sync::Mutex;
14+
//! # use alloc::boxed::Box;
15+
//! # use core::pin::Pin;
1416
//! // SAFETY: `init` is called below.
15-
//! let data = alloc::sync::Arc::pin(unsafe { Mutex::new(0) });
16-
//! mutex_init!(data.as_ref(), "test::data");
17+
//! let mut data = Pin::from(Box::new(unsafe { Mutex::new(0) }));
18+
//! mutex_init!(data.as_mut(), "test::data");
1719
//! *data.lock() = 10;
1820
//! pr_info!("{}\n", *data.lock());
1921
//! ```
@@ -73,7 +75,7 @@ pub trait NeedsLockClass {
7375
/// # Safety
7476
///
7577
/// `key` must point to a valid memory location as it will be used by the kernel.
76-
unsafe fn init(self: Pin<&Self>, name: &'static CStr, key: *mut bindings::lock_class_key);
78+
unsafe fn init(self: Pin<&mut Self>, name: &'static CStr, key: *mut bindings::lock_class_key);
7779
}
7880

7981
/// Determines if a signal is pending on the current process.

‎rust/kernel/sync/mutex.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ impl<T: ?Sized> Mutex<T> {
7272
}
7373

7474
impl<T: ?Sized> NeedsLockClass for Mutex<T> {
75-
unsafe fn init(self: Pin<&Self>, name: &'static CStr, key: *mut bindings::lock_class_key) {
75+
unsafe fn init(self: Pin<&mut Self>, name: &'static CStr, key: *mut bindings::lock_class_key) {
7676
unsafe { bindings::__mutex_init(self.mutex.get(), name.as_char_ptr(), key) };
7777
}
7878
}

‎rust/kernel/sync/spinlock.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ impl<T: ?Sized> SpinLock<T> {
8686
}
8787

8888
impl<T: ?Sized> NeedsLockClass for SpinLock<T> {
89-
unsafe fn init(self: Pin<&Self>, name: &'static CStr, key: *mut bindings::lock_class_key) {
89+
unsafe fn init(self: Pin<&mut Self>, name: &'static CStr, key: *mut bindings::lock_class_key) {
9090
unsafe { rust_helper_spin_lock_init(self.spin_lock.get(), name.as_char_ptr(), key) };
9191
}
9292
}

‎samples/rust/rust_miscdev.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,13 @@ impl SharedState {
4646
// SAFETY: `mutex_init!` is called below.
4747
inner: unsafe { Mutex::new(SharedStateInner { token_count: 0 }) },
4848
},
49-
|state| {
49+
|mut state| {
5050
// SAFETY: `state_changed` is pinned when `state` is.
51-
let state_changed = unsafe { Pin::new_unchecked(&state.state_changed) };
52-
kernel::condvar_init!(state_changed, "SharedState::state_changed");
51+
let pinned = unsafe { state.as_mut().map_unchecked_mut(|s| &mut s.state_changed) };
52+
kernel::condvar_init!(pinned, "SharedState::state_changed");
5353
// SAFETY: `inner` is pinned when `state` is.
54-
let inner = unsafe { Pin::new_unchecked(&state.inner) };
55-
kernel::mutex_init!(inner, "SharedState::inner");
54+
let pinned = unsafe { state.as_mut().map_unchecked_mut(|s| &mut s.inner) };
55+
kernel::mutex_init!(pinned, "SharedState::inner");
5656
},
5757
)?))
5858
}

‎samples/rust/rust_semaphore.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -131,13 +131,13 @@ impl KernelModule for RustSemaphore {
131131
})
132132
},
133133
},
134-
|sema| {
134+
|mut sema| {
135135
// SAFETY: `changed` is pinned when `sema` is.
136-
let pinned = unsafe { Pin::new_unchecked(&sema.changed) };
136+
let pinned = unsafe { sema.as_mut().map_unchecked_mut(|s| &mut s.changed) };
137137
condvar_init!(pinned, "Semaphore::changed");
138138

139139
// SAFETY: `inner` is pinned when `sema` is.
140-
let pinned = unsafe { Pin::new_unchecked(&sema.inner) };
140+
let pinned = unsafe { sema.as_mut().map_unchecked_mut(|s| &mut s.inner) };
141141
mutex_init!(pinned, "Semaphore::inner");
142142
},
143143
)?;

‎samples/rust/rust_sync.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,15 @@ impl KernelModule for RustSync {
3030
// Test mutexes.
3131
{
3232
// SAFETY: `init` is called below.
33-
let data = Pin::from(Box::try_new(unsafe { Mutex::new(0) })?);
34-
mutex_init!(data.as_ref(), "RustSync::init::data1");
33+
let mut data = Pin::from(Box::try_new(unsafe { Mutex::new(0) })?);
34+
mutex_init!(data.as_mut(), "RustSync::init::data1");
3535
*data.lock() = 10;
3636
pr_info!("Value: {}\n", *data.lock());
3737

3838
// SAFETY: `init` is called below.
39-
let cv = Pin::from(Box::try_new(unsafe { CondVar::new() })?);
40-
condvar_init!(cv.as_ref(), "RustSync::init::cv1");
39+
let mut cv = Pin::from(Box::try_new(unsafe { CondVar::new() })?);
40+
condvar_init!(cv.as_mut(), "RustSync::init::cv1");
41+
4142
{
4243
let mut guard = data.lock();
4344
while *guard != 10 {
@@ -52,14 +53,14 @@ impl KernelModule for RustSync {
5253
// Test spinlocks.
5354
{
5455
// SAFETY: `init` is called below.
55-
let data = Pin::from(Box::try_new(unsafe { SpinLock::new(0) })?);
56-
spinlock_init!(data.as_ref(), "RustSync::init::data2");
56+
let mut data = Pin::from(Box::try_new(unsafe { SpinLock::new(0) })?);
57+
spinlock_init!(data.as_mut(), "RustSync::init::data2");
5758
*data.lock() = 10;
5859
pr_info!("Value: {}\n", *data.lock());
5960

6061
// SAFETY: `init` is called below.
61-
let cv = Pin::from(Box::try_new(unsafe { CondVar::new() })?);
62-
condvar_init!(cv.as_ref(), "RustSync::init::cv2");
62+
let mut cv = Pin::from(Box::try_new(unsafe { CondVar::new() })?);
63+
condvar_init!(cv.as_mut(), "RustSync::init::cv2");
6364
{
6465
let mut guard = data.lock();
6566
while *guard != 10 {

0 commit comments

Comments
 (0)