Messages in this thread | | | Date | Sun, 9 Apr 2023 13:49:12 -0300 | From | Martin Rodriguez Reboredo <> | Subject | Re: [PATCH v3 12/13] rust: sync: introduce `CondVar` |
| |
On 4/8/23 04:53, Wedson Almeida Filho wrote: > From: Wedson Almeida Filho <walmeida@microsoft.com> > > This is the traditional condition variable or monitor synchronisation > primitive. It is implemented with C's `wait_queue_head_t`. > > It allows users to release a lock and go to sleep while guaranteeing > that notifications won't be missed. This is achieved by enqueuing a wait > entry before releasing the lock. > > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Will Deacon <will@kernel.org> > Cc: Waiman Long <longman@redhat.com> > Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com> > --- > v1 -> v2: Use the new `current` macro > v2 -> v3: Use new Opaque::ffi_init from Benno's series > > rust/bindings/bindings_helper.h | 1 + > rust/helpers.c | 7 ++ > rust/kernel/sync.rs | 2 + > rust/kernel/sync/condvar.rs | 174 ++++++++++++++++++++++++++++++++ > rust/kernel/sync/lock.rs | 1 - > 5 files changed, 184 insertions(+), 1 deletion(-) > create mode 100644 rust/kernel/sync/condvar.rs > > diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h > index 03656a44a83f..50e7a76d5455 100644 > --- a/rust/bindings/bindings_helper.h > +++ b/rust/bindings/bindings_helper.h > @@ -8,6 +8,7 @@ > > #include <linux/slab.h> > #include <linux/refcount.h> > +#include <linux/wait.h> > #include <linux/sched.h> > > /* `bindgen` gets confused at certain things. */ > diff --git a/rust/helpers.c b/rust/helpers.c > index 96441744030e..8ff2559c1572 100644 > --- a/rust/helpers.c > +++ b/rust/helpers.c > @@ -24,6 +24,7 @@ > #include <linux/mutex.h> > #include <linux/spinlock.h> > #include <linux/sched/signal.h> > +#include <linux/wait.h> > > __noreturn void rust_helper_BUG(void) > { > @@ -76,6 +77,12 @@ void rust_helper_spin_unlock_irqrestore(spinlock_t *lock, unsigned long flags) > } > EXPORT_SYMBOL_GPL(rust_helper_spin_unlock_irqrestore); > > +void rust_helper_init_wait(struct wait_queue_entry *wq_entry) > +{ > + init_wait(wq_entry); > +} > +EXPORT_SYMBOL_GPL(rust_helper_init_wait); > + > int rust_helper_signal_pending(struct task_struct *t) > { > return signal_pending(t); > diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs > index c997ff7e951e..431402180aa8 100644 > --- a/rust/kernel/sync.rs > +++ b/rust/kernel/sync.rs > @@ -8,9 +8,11 @@ > use crate::types::Opaque; > > mod arc; > +mod condvar; > pub mod lock; > > pub use arc::{Arc, ArcBorrow, UniqueArc}; > +pub use condvar::CondVar; > pub use lock::{mutex::Mutex, spinlock::SpinLock}; > > /// Represents a lockdep class. It's a wrapper around C's `lock_class_key`. > diff --git a/rust/kernel/sync/condvar.rs b/rust/kernel/sync/condvar.rs > new file mode 100644 > index 000000000000..ef882e2af746 > --- /dev/null > +++ b/rust/kernel/sync/condvar.rs > @@ -0,0 +1,174 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +//! A condition variable. > +//! > +//! This module allows Rust code to use the kernel's [`struct wait_queue_head`] as a condition > +//! variable. > + > +use super::{lock::Backend, lock::Guard, LockClassKey}; > +use crate::{bindings, init::PinInit, pin_init, str::CStr, types::Opaque}; > +use core::marker::PhantomPinned; > +use macros::pin_data; > + > +/// Creates a [`CondVar`] initialiser with the given name and a newly-created lock class. > +#[macro_export] > +macro_rules! new_condvar { > + ($($name:literal)?) => { > + $crate::sync::CondVar::new($crate::optional_name!($($name)?), $crate::static_lock_class!()) > + }; > +} > + > +/// A conditional variable. > +/// > +/// Exposes the kernel's [`struct wait_queue_head`] as a condition variable. It allows the caller to > +/// atomically release the given lock and go to sleep. It reacquires the lock when it wakes up. And > +/// it wakes up when notified by another thread (via [`CondVar::notify_one`] or > +/// [`CondVar::notify_all`]) or because the thread received a signal. It may also wake up > +/// spuriously. > +/// > +/// Instances of [`CondVar`] need a lock class and to be pinned. The recommended way to create such > +/// instances is with the [`pin_init`](crate::pin_init) and [`new_condvar`] macros. > +/// > +/// # Examples > +/// > +/// The following is an example of using a condvar with a mutex: > +/// > +/// ``` > +/// use kernel::sync::{CondVar, Mutex}; > +/// use kernel::{new_condvar, new_mutex}; > +/// > +/// #[pin_data] > +/// pub struct Example { > +/// #[pin] > +/// value: Mutex<u32>, > +/// > +/// #[pin] > +/// value_changed: CondVar, > +/// } > +/// > +/// /// Waits for `e.value` to become `v`. > +/// fn wait_for_vaue(e: &Example, v: u32) {
Should it be `wait_for_value`?
> +/// let mut guard = e.value.lock(); > +/// while *guard != v { > +/// e.value_changed.wait_uninterruptible(&mut guard); > +/// } > +/// } > +/// > +/// /// Increments `e.value` and notifies all potential waiters. > +/// fn increment(e: &Example) { > +/// *e.value.lock() += 1; > +/// e.value_changed.notify_all(); > +/// } > +/// > +/// /// Allocates a new boxed `Example`. > +/// fn new_example() -> Result<Pin<Box<Example>>> { > +/// Box::pin_init(pin_init!(Example { > +/// value <- new_mutex!(0), > +/// value_changed <- new_condvar!(), > +/// })) > +/// } > +/// ``` > +/// > +/// [`struct wait_queue_head`]: ../../../include/linux/wait.h > +#[pin_data] > +pub struct CondVar { > + #[pin] > + pub(crate) wait_list: Opaque<bindings::wait_queue_head>, > + > + /// A condvar needs to be pinned because it contains a [`struct list_head`] that is > + /// self-referential, so it cannot be safely moved once it is initialised. > + #[pin] > + _pin: PhantomPinned, > +} > + > +// SAFETY: `CondVar` only uses a `struct wait_queue_head`, which is safe to use on any thread. > +#[allow(clippy::non_send_fields_in_send_ty)] > +unsafe impl Send for CondVar {} > + > +// SAFETY: `CondVar` only uses a `struct wait_queue_head`, which is safe to use on multiple threads > +// concurrently. > +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> { > + pin_init!(Self { > + _pin: PhantomPinned, > + // SAFETY: `slot` is valid while the closure is called and both `name` and `key` have > + // static lifetimes so they live indefinitely. > + wait_list <- Opaque::ffi_init(|slot| unsafe { > + bindings::__init_waitqueue_head(slot, name.as_char_ptr(), key.as_ptr()) > + }), > + }) > + } > + > + fn wait_internal<T: ?Sized, B: Backend>(&self, wait_state: u32, guard: &mut Guard<'_, T, B>) { > + let wait = Opaque::<bindings::wait_queue_entry>::uninit(); > + > + // SAFETY: `wait` points to valid memory. > + unsafe { bindings::init_wait(wait.get()) }; > + > + // SAFETY: Both `wait` and `wait_list` point to valid memory. > + unsafe { > + bindings::prepare_to_wait_exclusive(self.wait_list.get(), wait.get(), wait_state as _) > + }; > + > + // SAFETY: No arguments, switches to another thread. > + guard.do_unlocked(|| unsafe { bindings::schedule() }); > + > + // SAFETY: Both `wait` and `wait_list` point to valid memory. > + unsafe { bindings::finish_wait(self.wait_list.get(), wait.get()) }; > + } > + > + /// Releases the lock and waits for a notification in interruptible mode. > + /// > + /// Atomically releases the given lock (whose ownership is proven by the guard) and puts the > + /// thread to sleep, reacquiring the lock on wake up. It wakes up when notified by > + /// [`CondVar::notify_one`] or [`CondVar::notify_all`], or when the thread receives a signal. > + /// It may also wake up spuriously. > + /// > + /// Returns whether there is a signal pending. > + #[must_use = "wait returns if a signal is pending, so the caller must check the return value"] > + pub fn wait<T: ?Sized, B: Backend>(&self, guard: &mut Guard<'_, T, B>) -> bool { > + self.wait_internal(bindings::TASK_INTERRUPTIBLE, guard); > + crate::current!().signal_pending() > + } > + > + /// Releases the lock and waits for a notification in uninterruptible mode. > + /// > + /// Similar to [`CondVar::wait`], except that the wait is not interruptible. That is, the > + /// thread won't wake up due to signals. It may, however, wake up supirously. > + pub fn wait_uninterruptible<T: ?Sized, B: Backend>(&self, guard: &mut Guard<'_, T, B>) { > + self.wait_internal(bindings::TASK_UNINTERRUPTIBLE, guard) > + } > + > + /// Calls the kernel function to notify the appropriate number of threads with the given flags. > + fn notify(&self, count: i32, flags: u32) { > + // SAFETY: `wait_list` points to valid memory. > + unsafe { > + bindings::__wake_up( > + self.wait_list.get(), > + bindings::TASK_NORMAL, > + count, > + flags as _, > + ) > + }; > + } > + > + /// Wakes a single waiter up, if any. > + /// > + /// This is not 'sticky' in the sense that if no thread is waiting, the notification is lost > + /// completely (as opposed to automatically waking up the next waiter). > + pub fn notify_one(&self) { > + self.notify(1, 0); > + } > + > + /// Wakes all waiters up, if any. > + /// > + /// This is not 'sticky' in the sense that if no thread is waiting, the notification is lost > + /// completely (as opposed to automatically waking up the next waiter). > + pub fn notify_all(&self) { > + self.notify(0, 0); > + } > +} > diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs > index a2c654ec5bd3..c365bc909dff 100644 > --- a/rust/kernel/sync/lock.rs > +++ b/rust/kernel/sync/lock.rs > @@ -179,7 +179,6 @@ pub struct Guard<'a, T: ?Sized, B: Backend> { > unsafe impl<T: Sync + ?Sized, B: Backend> Sync for Guard<'_, T, B> {} > > impl<T: ?Sized, B: Backend> Guard<'_, T, B> { > - #[allow(dead_code)] > pub(crate) fn do_unlocked(&mut self, cb: impl FnOnce()) { > // SAFETY: The caller owns the lock, so it is safe to unlock it. > unsafe { B::unlock(self.lock.state.get(), &self.state) };
Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
| |