Skip to content

Commit e7b9b1f

Browse files
Darksonnojeda
authored andcommitted
rust: sync: add CondVar::wait_timeout
Sleep on a condition variable with a timeout. This is used by Rust Binder for process freezing. There, we want to sleep until the freeze operation completes, but we want to be able to abort the process freezing if it doesn't complete within some timeout. Note that it is not enough to avoid jiffies by introducing a variant of `CondVar::wait_timeout` that takes the timeout in msecs because we need to be able to restart the sleep with the remaining sleep duration if it is interrupted, and if the API takes msecs rather than jiffies, then that would require a conversion roundtrip jiffies->msecs->jiffies that is best avoided. Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com> Reviewed-by: Tiago Lam <tiagolam@gmail.com> Reviewed-by: Boqun Feng <boqun.feng@gmail.com> Signed-off-by: Alice Ryhl <aliceryhl@google.com> Reviewed-by: Benno Lossin <benno.lossin@proton.me> Link: https://lore.kernel.org/r/20240108-rb-new-condvar-methods-v4-3-88e0c871cc05@google.com [ Added `CondVarTimeoutResult` re-export and fixed typo. ] Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
1 parent 82e1708 commit e7b9b1f

File tree

4 files changed

+61
-10
lines changed

4 files changed

+61
-10
lines changed

rust/kernel/sync.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ pub mod lock;
1313
mod locked_by;
1414

1515
pub use arc::{Arc, ArcBorrow, UniqueArc};
16-
pub use condvar::CondVar;
16+
pub use condvar::{CondVar, CondVarTimeoutResult};
1717
pub use lock::{mutex::Mutex, spinlock::SpinLock};
1818
pub use locked_by::LockedBy;
1919

rust/kernel/sync/condvar.rs

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,11 @@
66
//! variable.
77
88
use super::{lock::Backend, lock::Guard, LockClassKey};
9-
use crate::{bindings, init::PinInit, pin_init, str::CStr, types::Opaque};
9+
use crate::{
10+
bindings, init::PinInit, pin_init, str::CStr, task::MAX_SCHEDULE_TIMEOUT, time::Jiffies,
11+
types::Opaque,
12+
};
13+
use core::ffi::c_long;
1014
use core::marker::PhantomPinned;
1115
use macros::pin_data;
1216

@@ -102,7 +106,12 @@ impl CondVar {
102106
})
103107
}
104108

105-
fn wait_internal<T: ?Sized, B: Backend>(&self, wait_state: u32, guard: &mut Guard<'_, T, B>) {
109+
fn wait_internal<T: ?Sized, B: Backend>(
110+
&self,
111+
wait_state: u32,
112+
guard: &mut Guard<'_, T, B>,
113+
timeout_in_jiffies: c_long,
114+
) -> c_long {
106115
let wait = Opaque::<bindings::wait_queue_entry>::uninit();
107116

108117
// SAFETY: `wait` points to valid memory.
@@ -117,11 +126,13 @@ impl CondVar {
117126
)
118127
};
119128

120-
// SAFETY: No arguments, switches to another thread.
121-
guard.do_unlocked(|| unsafe { bindings::schedule() });
129+
// SAFETY: Switches to another thread. The timeout can be any number.
130+
let ret = guard.do_unlocked(|| unsafe { bindings::schedule_timeout(timeout_in_jiffies) });
122131

123132
// SAFETY: Both `wait` and `wait_queue_head` point to valid memory.
124133
unsafe { bindings::finish_wait(self.wait_queue_head.get(), wait.get()) };
134+
135+
ret
125136
}
126137

127138
/// Releases the lock and waits for a notification in uninterruptible mode.
@@ -131,7 +142,7 @@ impl CondVar {
131142
/// [`CondVar::notify_one`] or [`CondVar::notify_all`]. Note that it may also wake up
132143
/// spuriously.
133144
pub fn wait<T: ?Sized, B: Backend>(&self, guard: &mut Guard<'_, T, B>) {
134-
self.wait_internal(bindings::TASK_UNINTERRUPTIBLE, guard);
145+
self.wait_internal(bindings::TASK_UNINTERRUPTIBLE, guard, MAX_SCHEDULE_TIMEOUT);
135146
}
136147

137148
/// Releases the lock and waits for a notification in interruptible mode.
@@ -142,10 +153,31 @@ impl CondVar {
142153
/// Returns whether there is a signal pending.
143154
#[must_use = "wait_interruptible returns if a signal is pending, so the caller must check the return value"]
144155
pub fn wait_interruptible<T: ?Sized, B: Backend>(&self, guard: &mut Guard<'_, T, B>) -> bool {
145-
self.wait_internal(bindings::TASK_INTERRUPTIBLE, guard);
156+
self.wait_internal(bindings::TASK_INTERRUPTIBLE, guard, MAX_SCHEDULE_TIMEOUT);
146157
crate::current!().signal_pending()
147158
}
148159

160+
/// Releases the lock and waits for a notification in interruptible mode.
161+
///
162+
/// Atomically releases the given lock (whose ownership is proven by the guard) and puts the
163+
/// thread to sleep. It wakes up when notified by [`CondVar::notify_one`] or
164+
/// [`CondVar::notify_all`], or when a timeout occurs, or when the thread receives a signal.
165+
#[must_use = "wait_interruptible_timeout returns if a signal is pending, so the caller must check the return value"]
166+
pub fn wait_interruptible_timeout<T: ?Sized, B: Backend>(
167+
&self,
168+
guard: &mut Guard<'_, T, B>,
169+
jiffies: Jiffies,
170+
) -> CondVarTimeoutResult {
171+
let jiffies = jiffies.try_into().unwrap_or(MAX_SCHEDULE_TIMEOUT);
172+
let res = self.wait_internal(bindings::TASK_INTERRUPTIBLE, guard, jiffies);
173+
174+
match (res as Jiffies, crate::current!().signal_pending()) {
175+
(jiffies, true) => CondVarTimeoutResult::Signal { jiffies },
176+
(0, false) => CondVarTimeoutResult::Timeout,
177+
(jiffies, false) => CondVarTimeoutResult::Woken { jiffies },
178+
}
179+
}
180+
149181
/// Calls the kernel function to notify the appropriate number of threads with the given flags.
150182
fn notify(&self, count: i32, flags: u32) {
151183
// SAFETY: `wait_queue_head` points to valid memory.
@@ -185,3 +217,19 @@ impl CondVar {
185217
self.notify(0, 0);
186218
}
187219
}
220+
221+
/// The return type of `wait_timeout`.
222+
pub enum CondVarTimeoutResult {
223+
/// The timeout was reached.
224+
Timeout,
225+
/// Somebody woke us up.
226+
Woken {
227+
/// Remaining sleep duration.
228+
jiffies: Jiffies,
229+
},
230+
/// A signal occurred.
231+
Signal {
232+
/// Remaining sleep duration.
233+
jiffies: Jiffies,
234+
},
235+
}

rust/kernel/sync/lock.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,15 +139,15 @@ pub struct Guard<'a, T: ?Sized, B: Backend> {
139139
unsafe impl<T: Sync + ?Sized, B: Backend> Sync for Guard<'_, T, B> {}
140140

141141
impl<T: ?Sized, B: Backend> Guard<'_, T, B> {
142-
pub(crate) fn do_unlocked(&mut self, cb: impl FnOnce()) {
142+
pub(crate) fn do_unlocked<U>(&mut self, cb: impl FnOnce() -> U) -> U {
143143
// SAFETY: The caller owns the lock, so it is safe to unlock it.
144144
unsafe { B::unlock(self.lock.state.get(), &self.state) };
145145

146146
// SAFETY: The lock was just unlocked above and is being relocked now.
147147
let _relock =
148148
ScopeGuard::new(|| unsafe { B::relock(self.lock.state.get(), &mut self.state) });
149149

150-
cb();
150+
cb()
151151
}
152152
}
153153

rust/kernel/task.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@
55
//! C header: [`include/linux/sched.h`](srctree/include/linux/sched.h).
66
77
use crate::{bindings, types::Opaque};
8-
use core::{marker::PhantomData, ops::Deref, ptr};
8+
use core::{ffi::c_long, marker::PhantomData, ops::Deref, ptr};
9+
10+
/// A sentinel value used for infinite timeouts.
11+
pub const MAX_SCHEDULE_TIMEOUT: c_long = c_long::MAX;
912

1013
/// Returns the currently running task.
1114
#[macro_export]

0 commit comments

Comments
 (0)