Skip to content

Commit 33db8c9

Browse files
committed
rust: Use CpuId in place of raw CPU numbers
Use the newly defined `CpuId` abstraction instead of raw CPU numbers. This also fixes a doctest failure for configurations where `nr_cpu_ids < 4`. The C `cpumask_{set|clear}_cpu()` APIs emit a warning when given an invalid CPU number — but only if `CONFIG_DEBUG_PER_CPU_MAPS=y` is set. Meanwhile, `cpumask_weight()` only considers CPUs up to `nr_cpu_ids`, which can cause inconsistencies: a CPU number greater than `nr_cpu_ids` may be set in the mask, yet the weight calculation won't reflect it. This leads to doctest failures when `nr_cpu_ids < 4`, as the test tries to set CPUs 2 and 3: rust_doctest_kernel_cpumask_rs_0.location: rust/kernel/cpumask.rs:180 rust_doctest_kernel_cpumask_rs_0: ASSERTION FAILED at rust/kernel/cpumask.rs:190 Fixes: 8961b8c ("rust: cpumask: Add initial abstractions") Reported-by: Miguel Ojeda <[email protected]> Closes: https://lore.kernel.org/rust-for-linux/CANiq72k3ozKkLMinTLQwvkyg9K=BeRxs1oYZSKhJHY-veEyZdg@mail.gmail.com/ Reported-by: Andreas Hindborg <[email protected]> Closes: https://lore.kernel.org/all/[email protected]/ Suggested-by: Boqun Feng <[email protected]> Signed-off-by: Viresh Kumar <[email protected]> Reviewed-by: Boqun Feng <[email protected]>
1 parent ebf2e50 commit 33db8c9

File tree

4 files changed

+59
-27
lines changed

4 files changed

+59
-27
lines changed

drivers/cpufreq/rcpufreq_dt.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ fn find_supply_name_exact(dev: &Device, name: &str) -> Option<CString> {
2626
}
2727

2828
/// Finds supply name for the CPU from DT.
29-
fn find_supply_names(dev: &Device, cpu: u32) -> Option<KVec<CString>> {
29+
fn find_supply_names(dev: &Device, cpu: cpu::CpuId) -> Option<KVec<CString>> {
3030
// Try "cpu0" for older DTs, fallback to "cpu".
31-
let name = (cpu == 0)
31+
let name = (cpu.as_u32() == 0)
3232
.then(|| find_supply_name_exact(dev, "cpu0"))
3333
.flatten()
3434
.or_else(|| find_supply_name_exact(dev, "cpu"))?;

rust/kernel/cpu.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,9 +127,9 @@ impl From<CpuId> for i32 {
127127
/// Callers must ensure that the CPU device is not used after it has been unregistered.
128128
/// This can be achieved, for example, by registering a CPU hotplug notifier and removing
129129
/// any references to the CPU device within the notifier's callback.
130-
pub unsafe fn from_cpu(cpu: u32) -> Result<&'static Device> {
130+
pub unsafe fn from_cpu(cpu: CpuId) -> Result<&'static Device> {
131131
// SAFETY: It is safe to call `get_cpu_device()` for any CPU.
132-
let ptr = unsafe { bindings::get_cpu_device(cpu) };
132+
let ptr = unsafe { bindings::get_cpu_device(u32::from(cpu)) };
133133
if ptr.is_null() {
134134
return Err(ENODEV);
135135
}

rust/kernel/cpufreq.rs

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
1111
use crate::{
1212
clk::Hertz,
13+
cpu::CpuId,
1314
cpumask,
1415
device::{Bound, Device},
1516
devres::Devres,
@@ -465,8 +466,9 @@ impl Policy {
465466

466467
/// Returns the primary CPU for the [`Policy`].
467468
#[inline]
468-
pub fn cpu(&self) -> u32 {
469-
self.as_ref().cpu
469+
pub fn cpu(&self) -> CpuId {
470+
// SAFETY: The C API guarantees that `cpu` refers to a valid CPU number.
471+
unsafe { CpuId::from_u32_unchecked(self.as_ref().cpu) }
470472
}
471473

472474
/// Returns the minimum frequency for the [`Policy`].
@@ -525,7 +527,7 @@ impl Policy {
525527
#[inline]
526528
pub fn generic_get(&self) -> Result<u32> {
527529
// SAFETY: By the type invariant, the pointer stored in `self` is valid.
528-
Ok(unsafe { bindings::cpufreq_generic_get(self.cpu()) })
530+
Ok(unsafe { bindings::cpufreq_generic_get(u32::from(self.cpu())) })
529531
}
530532

531533
/// Provides a wrapper to the register with energy model using the OPP core.
@@ -678,9 +680,9 @@ impl Policy {
678680
struct PolicyCpu<'a>(&'a mut Policy);
679681

680682
impl<'a> PolicyCpu<'a> {
681-
fn from_cpu(cpu: u32) -> Result<Self> {
683+
fn from_cpu(cpu: CpuId) -> Result<Self> {
682684
// SAFETY: It is safe to call `cpufreq_cpu_get` for any valid CPU.
683-
let ptr = from_err_ptr(unsafe { bindings::cpufreq_cpu_get(cpu) })?;
685+
let ptr = from_err_ptr(unsafe { bindings::cpufreq_cpu_get(u32::from(cpu)) })?;
684686

685687
Ok(Self(
686688
// SAFETY: The `ptr` is guaranteed to be valid and remains valid for the lifetime of
@@ -1266,7 +1268,10 @@ impl<T: Driver> Registration<T> {
12661268
target_perf: usize,
12671269
capacity: usize,
12681270
) {
1269-
if let Ok(mut policy) = PolicyCpu::from_cpu(cpu) {
1271+
// SAFETY: The C API guarantees that `cpu` refers to a valid CPU number.
1272+
let cpu_id = unsafe { CpuId::from_u32_unchecked(cpu) };
1273+
1274+
if let Ok(mut policy) = PolicyCpu::from_cpu(cpu_id) {
12701275
T::adjust_perf(&mut policy, min_perf, target_perf, capacity);
12711276
}
12721277
}
@@ -1321,7 +1326,10 @@ impl<T: Driver> Registration<T> {
13211326
///
13221327
/// - This function may only be called from the cpufreq C infrastructure.
13231328
unsafe extern "C" fn get_callback(cpu: u32) -> kernel::ffi::c_uint {
1324-
PolicyCpu::from_cpu(cpu).map_or(0, |mut policy| T::get(&mut policy).map_or(0, |f| f))
1329+
// SAFETY: The C API guarantees that `cpu` refers to a valid CPU number.
1330+
let cpu_id = unsafe { CpuId::from_u32_unchecked(cpu) };
1331+
1332+
PolicyCpu::from_cpu(cpu_id).map_or(0, |mut policy| T::get(&mut policy).map_or(0, |f| f))
13251333
}
13261334

13271335
/// Driver's `update_limit` callback.
@@ -1344,8 +1352,11 @@ impl<T: Driver> Registration<T> {
13441352
/// - This function may only be called from the cpufreq C infrastructure.
13451353
/// - The pointer arguments must be valid pointers.
13461354
unsafe extern "C" fn bios_limit_callback(cpu: i32, limit: *mut u32) -> kernel::ffi::c_int {
1355+
// SAFETY: The C API guarantees that `cpu` refers to a valid CPU number.
1356+
let cpu_id = unsafe { CpuId::from_i32_unchecked(cpu) };
1357+
13471358
from_result(|| {
1348-
let mut policy = PolicyCpu::from_cpu(cpu as u32)?;
1359+
let mut policy = PolicyCpu::from_cpu(cpu_id)?;
13491360

13501361
// SAFETY: `limit` is guaranteed by the C code to be valid.
13511362
T::bios_limit(&mut policy, &mut (unsafe { *limit })).map(|()| 0)

rust/kernel/cpumask.rs

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
77
use crate::{
88
alloc::{AllocError, Flags},
9+
cpu::CpuId,
910
prelude::*,
1011
types::Opaque,
1112
};
@@ -35,9 +36,10 @@ use core::ops::{Deref, DerefMut};
3536
///
3637
/// ```
3738
/// use kernel::bindings;
39+
/// use kernel::cpu::CpuId;
3840
/// use kernel::cpumask::Cpumask;
3941
///
40-
/// fn set_clear_cpu(ptr: *mut bindings::cpumask, set_cpu: u32, clear_cpu: i32) {
42+
/// fn set_clear_cpu(ptr: *mut bindings::cpumask, set_cpu: CpuId, clear_cpu: CpuId) {
4143
/// // SAFETY: The `ptr` is valid for writing and remains valid for the lifetime of the
4244
/// // returned reference.
4345
/// let mask = unsafe { Cpumask::as_mut_ref(ptr) };
@@ -90,9 +92,9 @@ impl Cpumask {
9092
/// This mismatches kernel naming convention and corresponds to the C
9193
/// function `__cpumask_set_cpu()`.
9294
#[inline]
93-
pub fn set(&mut self, cpu: u32) {
95+
pub fn set(&mut self, cpu: CpuId) {
9496
// SAFETY: By the type invariant, `self.as_raw` is a valid argument to `__cpumask_set_cpu`.
95-
unsafe { bindings::__cpumask_set_cpu(cpu, self.as_raw()) };
97+
unsafe { bindings::__cpumask_set_cpu(u32::from(cpu), self.as_raw()) };
9698
}
9799

98100
/// Clear `cpu` in the cpumask.
@@ -101,19 +103,19 @@ impl Cpumask {
101103
/// This mismatches kernel naming convention and corresponds to the C
102104
/// function `__cpumask_clear_cpu()`.
103105
#[inline]
104-
pub fn clear(&mut self, cpu: i32) {
106+
pub fn clear(&mut self, cpu: CpuId) {
105107
// SAFETY: By the type invariant, `self.as_raw` is a valid argument to
106108
// `__cpumask_clear_cpu`.
107-
unsafe { bindings::__cpumask_clear_cpu(cpu, self.as_raw()) };
109+
unsafe { bindings::__cpumask_clear_cpu(i32::from(cpu), self.as_raw()) };
108110
}
109111

110112
/// Test `cpu` in the cpumask.
111113
///
112114
/// Equivalent to the kernel's `cpumask_test_cpu` API.
113115
#[inline]
114-
pub fn test(&self, cpu: i32) -> bool {
116+
pub fn test(&self, cpu: CpuId) -> bool {
115117
// SAFETY: By the type invariant, `self.as_raw` is a valid argument to `cpumask_test_cpu`.
116-
unsafe { bindings::cpumask_test_cpu(cpu, self.as_raw()) }
118+
unsafe { bindings::cpumask_test_cpu(i32::from(cpu), self.as_raw()) }
117119
}
118120

119121
/// Set all CPUs in the cpumask.
@@ -178,21 +180,40 @@ impl Cpumask {
178180
/// The following example demonstrates how to create and update a [`CpumaskVar`].
179181
///
180182
/// ```
183+
/// use kernel::cpu::CpuId;
181184
/// use kernel::cpumask::CpumaskVar;
182185
///
183186
/// let mut mask = CpumaskVar::new_zero(GFP_KERNEL).unwrap();
184187
///
185188
/// assert!(mask.empty());
186-
/// mask.set(2);
187-
/// assert!(mask.test(2));
188-
/// mask.set(3);
189-
/// assert!(mask.test(3));
190-
/// assert_eq!(mask.weight(), 2);
189+
/// let mut count = 0;
190+
///
191+
/// let cpu2 = CpuId::from_u32(2);
192+
/// if let Some(cpu) = cpu2 {
193+
/// mask.set(cpu);
194+
/// assert!(mask.test(cpu));
195+
/// count += 1;
196+
/// }
197+
///
198+
/// let cpu3 = CpuId::from_u32(3);
199+
/// if let Some(cpu) = cpu3 {
200+
/// mask.set(cpu);
201+
/// assert!(mask.test(cpu));
202+
/// count += 1;
203+
/// }
204+
///
205+
/// assert_eq!(mask.weight(), count);
191206
///
192207
/// let mask2 = CpumaskVar::try_clone(&mask).unwrap();
193-
/// assert!(mask2.test(2));
194-
/// assert!(mask2.test(3));
195-
/// assert_eq!(mask2.weight(), 2);
208+
///
209+
/// if let Some(cpu) = cpu2 {
210+
/// assert!(mask2.test(cpu));
211+
/// }
212+
///
213+
/// if let Some(cpu) = cpu3 {
214+
/// assert!(mask2.test(cpu));
215+
/// }
216+
/// assert_eq!(mask2.weight(), count);
196217
/// ```
197218
pub struct CpumaskVar {
198219
#[cfg(CONFIG_CPUMASK_OFFSTACK)]

0 commit comments

Comments
 (0)