Skip to content

Commit 9dc576b

Browse files
Darksonnfbq
authored andcommitted
rust: sync: update integer types in CondVar
Reduce the chances of compilation failures due to integer type mismatches in `CondVar`. When an integer is defined using a #define in C, bindgen doesn't know which integer type it is supposed to be, so it will just use `u32` by default (if it fits in an u32). Whenever the right type is something else, we insert a cast in Rust. However, this means that the code has a lot of extra casts, and sometimes the code will be missing casts if u32 happens to be correct on the developer's machine, even though the type might be something else on a different platform. This patch updates all uses of such constants in `rust/kernel/sync/condvar.rs` to use constants defined with the right type. This allows us to remove various unnecessary casts, while also future-proofing for the case where `unsigned int != u32`. I wrote this patch at the suggestion of Benno in [1]. Link: https://lore.kernel.org/all/nAEg-6vbtX72ZY3oirDhrSEf06TBWmMiTt73EklMzEAzN4FD4mF3TPEyAOxBZgZtjzoiaBYtYr3s8sa9wp1uYH9vEWRf2M-Lf4I0BY9rAgk=@proton.me/ [1] Suggested-by: Benno Lossin <benno.lossin@proton.me> Reviewed-by: Tiago Lam <tiagolam@gmail.com> Reviewed-by: Boqun Feng <boqun.feng@gmail.com> Reviewed-by: Benno Lossin <benno.lossin@proton.me> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com> Signed-off-by: Alice Ryhl <aliceryhl@google.com> Link: https://lore.kernel.org/r/20240108-rb-new-condvar-methods-v4-4-88e0c871cc05@google.com [boqun: Resolve conflicts against wait_queue_head renaming]
1 parent 8035ed0 commit 9dc576b

2 files changed

Lines changed: 33 additions & 14 deletions

File tree

rust/kernel/sync/condvar.rs

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,17 @@
77
88
use super::{lock::Backend, lock::Guard, LockClassKey};
99
use crate::{
10-
bindings, init::PinInit, pin_init, str::CStr, task::MAX_SCHEDULE_TIMEOUT, time::Jiffies,
10+
bindings,
11+
init::PinInit,
12+
pin_init,
13+
str::CStr,
14+
task::{MAX_SCHEDULE_TIMEOUT, TASK_INTERRUPTIBLE, TASK_NORMAL, TASK_UNINTERRUPTIBLE},
15+
time::Jiffies,
1116
types::Opaque,
1217
};
13-
use core::ffi::c_long;
18+
use core::ffi::{c_int, c_long};
1419
use core::marker::PhantomPinned;
20+
use core::ptr;
1521
use macros::pin_data;
1622

1723
/// Creates a [`CondVar`] initialiser with the given name and a newly-created lock class.
@@ -108,7 +114,7 @@ impl CondVar {
108114

109115
fn wait_internal<T: ?Sized, B: Backend>(
110116
&self,
111-
wait_state: u32,
117+
wait_state: c_int,
112118
guard: &mut Guard<'_, T, B>,
113119
timeout_in_jiffies: c_long,
114120
) -> c_long {
@@ -122,7 +128,7 @@ impl CondVar {
122128
bindings::prepare_to_wait_exclusive(
123129
self.wait_queue_head.get(),
124130
wait.get(),
125-
wait_state as _,
131+
wait_state,
126132
)
127133
};
128134

@@ -142,7 +148,7 @@ impl CondVar {
142148
/// [`CondVar::notify_one`] or [`CondVar::notify_all`]. Note that it may also wake up
143149
/// spuriously.
144150
pub fn wait<T: ?Sized, B: Backend>(&self, guard: &mut Guard<'_, T, B>) {
145-
self.wait_internal(bindings::TASK_UNINTERRUPTIBLE, guard, MAX_SCHEDULE_TIMEOUT);
151+
self.wait_internal(TASK_UNINTERRUPTIBLE, guard, MAX_SCHEDULE_TIMEOUT);
146152
}
147153

148154
/// Releases the lock and waits for a notification in interruptible mode.
@@ -153,7 +159,7 @@ impl CondVar {
153159
/// Returns whether there is a signal pending.
154160
#[must_use = "wait_interruptible returns if a signal is pending, so the caller must check the return value"]
155161
pub fn wait_interruptible<T: ?Sized, B: Backend>(&self, guard: &mut Guard<'_, T, B>) -> bool {
156-
self.wait_internal(bindings::TASK_INTERRUPTIBLE, guard, MAX_SCHEDULE_TIMEOUT);
162+
self.wait_internal(TASK_INTERRUPTIBLE, guard, MAX_SCHEDULE_TIMEOUT);
157163
crate::current!().signal_pending()
158164
}
159165

@@ -169,7 +175,7 @@ impl CondVar {
169175
jiffies: Jiffies,
170176
) -> CondVarTimeoutResult {
171177
let jiffies = jiffies.try_into().unwrap_or(MAX_SCHEDULE_TIMEOUT);
172-
let res = self.wait_internal(bindings::TASK_INTERRUPTIBLE, guard, jiffies);
178+
let res = self.wait_internal(TASK_INTERRUPTIBLE, guard, jiffies);
173179

174180
match (res as Jiffies, crate::current!().signal_pending()) {
175181
(jiffies, true) => CondVarTimeoutResult::Signal { jiffies },
@@ -178,15 +184,15 @@ impl CondVar {
178184
}
179185
}
180186

181-
/// Calls the kernel function to notify the appropriate number of threads with the given flags.
182-
fn notify(&self, count: i32, flags: u32) {
187+
/// Calls the kernel function to notify the appropriate number of threads.
188+
fn notify(&self, count: c_int) {
183189
// SAFETY: `wait_queue_head` points to valid memory.
184190
unsafe {
185191
bindings::__wake_up(
186192
self.wait_queue_head.get(),
187193
bindings::TASK_NORMAL,
188194
count,
189-
flags as _,
195+
ptr::null_mut(),
190196
)
191197
};
192198
}
@@ -198,23 +204,23 @@ impl CondVar {
198204
/// CPU.
199205
pub fn notify_sync(&self) {
200206
// SAFETY: `wait_queue_head` points to valid memory.
201-
unsafe { bindings::__wake_up_sync(self.wait_queue_head.get(), bindings::TASK_NORMAL) };
207+
unsafe { bindings::__wake_up_sync(self.wait_queue_head.get(), TASK_NORMAL) };
202208
}
203209

204210
/// Wakes a single waiter up, if any.
205211
///
206212
/// This is not 'sticky' in the sense that if no thread is waiting, the notification is lost
207213
/// completely (as opposed to automatically waking up the next waiter).
208214
pub fn notify_one(&self) {
209-
self.notify(1, 0);
215+
self.notify(1);
210216
}
211217

212218
/// Wakes all waiters up, if any.
213219
///
214220
/// This is not 'sticky' in the sense that if no thread is waiting, the notification is lost
215221
/// completely (as opposed to automatically waking up the next waiter).
216222
pub fn notify_all(&self) {
217-
self.notify(0, 0);
223+
self.notify(0);
218224
}
219225
}
220226

rust/kernel/task.rs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,24 @@
55
//! C header: [`include/linux/sched.h`](srctree/include/linux/sched.h).
66
77
use crate::{bindings, types::Opaque};
8-
use core::{ffi::c_long, marker::PhantomData, ops::Deref, ptr};
8+
use core::{
9+
ffi::{c_int, c_long, c_uint},
10+
marker::PhantomData,
11+
ops::Deref,
12+
ptr,
13+
};
914

1015
/// A sentinal value used for infinite timeouts.
1116
pub const MAX_SCHEDULE_TIMEOUT: c_long = c_long::MAX;
1217

18+
/// Bitmask for tasks that are sleeping in an interruptible state.
19+
pub const TASK_INTERRUPTIBLE: c_int = bindings::TASK_INTERRUPTIBLE as c_int;
20+
/// Bitmask for tasks that are sleeping in an uninterruptible state.
21+
pub const TASK_UNINTERRUPTIBLE: c_int = bindings::TASK_UNINTERRUPTIBLE as c_int;
22+
/// Convenience constant for waking up tasks regardless of whether they are in interruptible or
23+
/// uninterruptible sleep.
24+
pub const TASK_NORMAL: c_uint = bindings::TASK_NORMAL as c_uint;
25+
1326
/// Returns the currently running task.
1427
#[macro_export]
1528
macro_rules! current {

0 commit comments

Comments
 (0)