Commit 33db8c97 authored by Viresh Kumar's avatar Viresh Kumar
Browse files

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: 8961b8cb ("rust: cpumask: Add initial abstractions")
Reported-by: default avatarMiguel Ojeda <ojeda@kernel.org>
Closes: https://lore.kernel.org/rust-for-linux/CANiq72k3ozKkLMinTLQwvkyg9K=BeRxs1oYZSKhJHY-veEyZdg@mail.gmail.com/


Reported-by: default avatarAndreas Hindborg <a.hindborg@kernel.org>
Closes: https://lore.kernel.org/all/87qzzy3ric.fsf@kernel.org/


Suggested-by: default avatarBoqun Feng <boqun.feng@gmail.com>
Signed-off-by: default avatarViresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: default avatarBoqun Feng <boqun.feng@gmail.com>
parent ebf2e500

drivers/cpufreq/rcpufreq_dt.rs

+2 −2
Original line number Diff line number Diff line
@@ -26,9 +26,9 @@ fn find_supply_name_exact(dev: &Device, name: &str) -> Option<CString> { 
}



/// Finds supply name for the CPU from DT.

fn find_supply_names(dev: &Device, cpu: u32) -> Option<KVec<CString>> {

fn find_supply_names(dev: &Device, cpu: cpu::CpuId) -> Option<KVec<CString>> {

    // Try "cpu0" for older DTs, fallback to "cpu".

    let name = (cpu == 0)

    let name = (cpu.as_u32() == 0)

        .then(|| find_supply_name_exact(dev, "cpu0"))

        .flatten()

        .or_else(|| find_supply_name_exact(dev, "cpu"))?;

rust/kernel/cpu.rs

+2 −2
Original line number Diff line number Diff line
@@ -127,9 +127,9 @@ fn from(id: CpuId) -> Self { 
/// Callers must ensure that the CPU device is not used after it has been unregistered.

/// This can be achieved, for example, by registering a CPU hotplug notifier and removing

/// any references to the CPU device within the notifier's callback.

pub unsafe fn from_cpu(cpu: u32) -> Result<&'static Device> {

pub unsafe fn from_cpu(cpu: CpuId) -> Result<&'static Device> {

    // SAFETY: It is safe to call `get_cpu_device()` for any CPU.

    let ptr = unsafe { bindings::get_cpu_device(cpu) };

    let ptr = unsafe { bindings::get_cpu_device(u32::from(cpu)) };

    if ptr.is_null() {

        return Err(ENODEV);

    }

rust/kernel/cpufreq.rs

+19 −8
Original line number Diff line number Diff line
@@ -10,6 +10,7 @@ 


use crate::{

    clk::Hertz,

    cpu::CpuId,

    cpumask,

    device::{Bound, Device},

    devres::Devres,
@@ -465,8 +466,9 @@ fn as_mut_ref(&mut self) -> &mut bindings::cpufreq_policy { 


    /// Returns the primary CPU for the [`Policy`].

    #[inline]

    pub fn cpu(&self) -> u32 {

        self.as_ref().cpu

    pub fn cpu(&self) -> CpuId {

        // SAFETY: The C API guarantees that `cpu` refers to a valid CPU number.

        unsafe { CpuId::from_u32_unchecked(self.as_ref().cpu) }

    }



    /// Returns the minimum frequency for the [`Policy`].
@@ -525,7 +527,7 @@ pub fn generic_suspend(&mut self) -> Result { 
    #[inline]

    pub fn generic_get(&self) -> Result<u32> {

        // SAFETY: By the type invariant, the pointer stored in `self` is valid.

        Ok(unsafe { bindings::cpufreq_generic_get(self.cpu()) })

        Ok(unsafe { bindings::cpufreq_generic_get(u32::from(self.cpu())) })

    }



    /// Provides a wrapper to the register with energy model using the OPP core.
@@ -678,9 +680,9 @@ fn clear_data<T: ForeignOwnable>(&mut self) -> Option<T> { 
struct PolicyCpu<'a>(&'a mut Policy);



impl<'a> PolicyCpu<'a> {

    fn from_cpu(cpu: u32) -> Result<Self> {

    fn from_cpu(cpu: CpuId) -> Result<Self> {

        // SAFETY: It is safe to call `cpufreq_cpu_get` for any valid CPU.

        let ptr = from_err_ptr(unsafe { bindings::cpufreq_cpu_get(cpu) })?;

        let ptr = from_err_ptr(unsafe { bindings::cpufreq_cpu_get(u32::from(cpu)) })?;



        Ok(Self(

            // SAFETY: The `ptr` is guaranteed to be valid and remains valid for the lifetime of
@@ -1266,7 +1268,10 @@ impl<T: Driver> Registration<T> { 
        target_perf: usize,

        capacity: usize,

    ) {

        if let Ok(mut policy) = PolicyCpu::from_cpu(cpu) {

        // SAFETY: The C API guarantees that `cpu` refers to a valid CPU number.

        let cpu_id = unsafe { CpuId::from_u32_unchecked(cpu) };



        if let Ok(mut policy) = PolicyCpu::from_cpu(cpu_id) {

            T::adjust_perf(&mut policy, min_perf, target_perf, capacity);

        }

    }
@@ -1321,7 +1326,10 @@ impl<T: Driver> Registration<T> { 
    ///

    /// - This function may only be called from the cpufreq C infrastructure.

    unsafe extern "C" fn get_callback(cpu: u32) -> kernel::ffi::c_uint {

        PolicyCpu::from_cpu(cpu).map_or(0, |mut policy| T::get(&mut policy).map_or(0, |f| f))

        // SAFETY: The C API guarantees that `cpu` refers to a valid CPU number.

        let cpu_id = unsafe { CpuId::from_u32_unchecked(cpu) };



        PolicyCpu::from_cpu(cpu_id).map_or(0, |mut policy| T::get(&mut policy).map_or(0, |f| f))

    }



    /// Driver's `update_limit` callback.
@@ -1344,8 +1352,11 @@ impl<T: Driver> Registration<T> { 
    /// - This function may only be called from the cpufreq C infrastructure.

    /// - The pointer arguments must be valid pointers.

    unsafe extern "C" fn bios_limit_callback(cpu: i32, limit: *mut u32) -> kernel::ffi::c_int {

        // SAFETY: The C API guarantees that `cpu` refers to a valid CPU number.

        let cpu_id = unsafe { CpuId::from_i32_unchecked(cpu) };



        from_result(|| {

            let mut policy = PolicyCpu::from_cpu(cpu as u32)?;

            let mut policy = PolicyCpu::from_cpu(cpu_id)?;



            // SAFETY: `limit` is guaranteed by the C code to be valid.

            T::bios_limit(&mut policy, &mut (unsafe { *limit })).map(|()| 0)

rust/kernel/cpumask.rs

+36 −15
Original line number Diff line number Diff line
@@ -6,6 +6,7 @@ 


use crate::{

    alloc::{AllocError, Flags},

    cpu::CpuId,

    prelude::*,

    types::Opaque,

};
@@ -35,9 +36,10 @@ 
///

/// ```

/// use kernel::bindings;

/// use kernel::cpu::CpuId;

/// use kernel::cpumask::Cpumask;

///

/// fn set_clear_cpu(ptr: *mut bindings::cpumask, set_cpu: u32, clear_cpu: i32) {

/// fn set_clear_cpu(ptr: *mut bindings::cpumask, set_cpu: CpuId, clear_cpu: CpuId) {

///     // SAFETY: The `ptr` is valid for writing and remains valid for the lifetime of the

///     // returned reference.

///     let mask = unsafe { Cpumask::as_mut_ref(ptr) };
@@ -90,9 +92,9 @@ pub fn as_raw(&self) -> *mut bindings::cpumask { 
    /// This mismatches kernel naming convention and corresponds to the C

    /// function `__cpumask_set_cpu()`.

    #[inline]

    pub fn set(&mut self, cpu: u32) {

    pub fn set(&mut self, cpu: CpuId) {

        // SAFETY: By the type invariant, `self.as_raw` is a valid argument to `__cpumask_set_cpu`.

        unsafe { bindings::__cpumask_set_cpu(cpu, self.as_raw()) };

        unsafe { bindings::__cpumask_set_cpu(u32::from(cpu), self.as_raw()) };

    }



    /// Clear `cpu` in the cpumask.
@@ -101,19 +103,19 @@ pub fn set(&mut self, cpu: u32) { 
    /// This mismatches kernel naming convention and corresponds to the C

    /// function `__cpumask_clear_cpu()`.

    #[inline]

    pub fn clear(&mut self, cpu: i32) {

    pub fn clear(&mut self, cpu: CpuId) {

        // SAFETY: By the type invariant, `self.as_raw` is a valid argument to

        // `__cpumask_clear_cpu`.

        unsafe { bindings::__cpumask_clear_cpu(cpu, self.as_raw()) };

        unsafe { bindings::__cpumask_clear_cpu(i32::from(cpu), self.as_raw()) };

    }



    /// Test `cpu` in the cpumask.

    ///

    /// Equivalent to the kernel's `cpumask_test_cpu` API.

    #[inline]

    pub fn test(&self, cpu: i32) -> bool {

    pub fn test(&self, cpu: CpuId) -> bool {

        // SAFETY: By the type invariant, `self.as_raw` is a valid argument to `cpumask_test_cpu`.

        unsafe { bindings::cpumask_test_cpu(cpu, self.as_raw()) }

        unsafe { bindings::cpumask_test_cpu(i32::from(cpu), self.as_raw()) }

    }



    /// Set all CPUs in the cpumask.
@@ -178,21 +180,40 @@ pub fn copy(&self, dstp: &mut Self) { 
/// The following example demonstrates how to create and update a [`CpumaskVar`].

///

/// ```

/// use kernel::cpu::CpuId;

/// use kernel::cpumask::CpumaskVar;

///

/// let mut mask = CpumaskVar::new_zero(GFP_KERNEL).unwrap();

///

/// assert!(mask.empty());

/// mask.set(2);

/// assert!(mask.test(2));

/// mask.set(3);

/// assert!(mask.test(3));

/// assert_eq!(mask.weight(), 2);

/// let mut count = 0;

///

/// let cpu2 = CpuId::from_u32(2);

/// if let Some(cpu) = cpu2 {

///     mask.set(cpu);

///     assert!(mask.test(cpu));

///     count += 1;

/// }

///

/// let cpu3 = CpuId::from_u32(3);

/// if let Some(cpu) = cpu3 {

///     mask.set(cpu);

///     assert!(mask.test(cpu));

///     count += 1;

/// }

///

/// assert_eq!(mask.weight(), count);

///

/// let mask2 = CpumaskVar::try_clone(&mask).unwrap();

/// assert!(mask2.test(2));

/// assert!(mask2.test(3));

/// assert_eq!(mask2.weight(), 2);

///

/// if let Some(cpu) = cpu2 {

///     assert!(mask2.test(cpu));

/// }

///

/// if let Some(cpu) = cpu3 {

///     assert!(mask2.test(cpu));

/// }

/// assert_eq!(mask2.weight(), count);

/// ```

pub struct CpumaskVar {

    #[cfg(CONFIG_CPUMASK_OFFSTACK)]