Skip to content

Commit a9f81c2

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 <[email protected]> Reviewed-by: Tiago Lam <[email protected]> Reviewed-by: Boqun Feng <[email protected]> Reviewed-by: Benno Lossin <[email protected]> Signed-off-by: Alice Ryhl <[email protected]> Reviewed-by: Martin Rodriguez Reboredo <[email protected]> Link: https://lore.kernel.org/r/[email protected] [boqun: Fix use core::* in condvar.rs and task.rs]
1 parent 8972853 commit a9f81c2

File tree

2 files changed

+28
-21
lines changed

2 files changed

+28
-21
lines changed

rust/kernel/sync/condvar.rs

Lines changed: 19 additions & 20 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 {
@@ -119,7 +125,7 @@ impl CondVar {
119125

120126
// SAFETY: Both `wait` and `wait_list` point to valid memory.
121127
unsafe {
122-
bindings::prepare_to_wait_exclusive(self.wait_list.get(), wait.get(), wait_state as _)
128+
bindings::prepare_to_wait_exclusive(self.wait_list.get(), wait.get(), wait_state)
123129
};
124130

125131
// SAFETY: Switches to another thread. The timeout can be any number.
@@ -138,7 +144,7 @@ impl CondVar {
138144
/// [`CondVar::notify_one`] or [`CondVar::notify_all`]. Note that it may also wake up
139145
/// spuriously.
140146
pub fn wait<T: ?Sized, B: Backend>(&self, guard: &mut Guard<'_, T, B>) {
141-
self.wait_internal(bindings::TASK_UNINTERRUPTIBLE, guard, MAX_SCHEDULE_TIMEOUT);
147+
self.wait_internal(TASK_UNINTERRUPTIBLE, guard, MAX_SCHEDULE_TIMEOUT);
142148
}
143149

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

@@ -165,7 +171,7 @@ impl CondVar {
165171
jiffies: Jiffies,
166172
) -> CondVarTimeoutResult {
167173
let jiffies = jiffies.try_into().unwrap_or(MAX_SCHEDULE_TIMEOUT);
168-
let res = self.wait_internal(bindings::TASK_INTERRUPTIBLE, guard, jiffies);
174+
let res = self.wait_internal(TASK_INTERRUPTIBLE, guard, jiffies);
169175

170176
match (res as Jiffies, crate::current!().signal_pending()) {
171177
(jiffies, true) => CondVarTimeoutResult::Signal { jiffies },
@@ -174,39 +180,32 @@ impl CondVar {
174180
}
175181
}
176182

177-
/// Calls the kernel function to notify the appropriate number of threads with the given flags.
178-
fn notify(&self, count: i32, flags: u32) {
183+
/// Calls the kernel function to notify the appropriate number of threads.
184+
fn notify(&self, count: c_int) {
179185
// SAFETY: `wait_list` points to valid memory.
180-
unsafe {
181-
bindings::__wake_up(
182-
self.wait_list.get(),
183-
bindings::TASK_NORMAL,
184-
count,
185-
flags as _,
186-
)
187-
};
186+
unsafe { bindings::__wake_up(self.wait_list.get(), TASK_NORMAL, count, ptr::null_mut()) };
188187
}
189188

190189
/// Calls the kernel function to notify one thread synchronously.
191190
pub fn notify_sync(&self) {
192191
// SAFETY: `wait_list` points to valid memory.
193-
unsafe { bindings::__wake_up_sync(self.wait_list.get(), bindings::TASK_NORMAL) };
192+
unsafe { bindings::__wake_up_sync(self.wait_list.get(), TASK_NORMAL) };
194193
}
195194

196195
/// Wakes a single waiter up, if any.
197196
///
198197
/// This is not 'sticky' in the sense that if no thread is waiting, the notification is lost
199198
/// completely (as opposed to automatically waking up the next waiter).
200199
pub fn notify_one(&self) {
201-
self.notify(1, 0);
200+
self.notify(1);
202201
}
203202

204203
/// Wakes all waiters up, if any.
205204
///
206205
/// This is not 'sticky' in the sense that if no thread is waiting, the notification is lost
207206
/// completely (as opposed to automatically waking up the next waiter).
208207
pub fn notify_all(&self) {
209-
self.notify(0, 0);
208+
self.notify(0);
210209
}
211210
}
212211

rust/kernel/task.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,19 @@ use core::{
1212
ptr,
1313
};
1414

15-
use core::ffi::c_long;
15+
use core::ffi::{c_int, c_long, c_uint};
1616

1717
/// A sentinal value used for infinite timeouts.
1818
pub const MAX_SCHEDULE_TIMEOUT: c_long = c_long::MAX;
1919

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

0 commit comments

Comments
 (0)