Re: [PATCH V3 2/3] rust: Use CpuId in place of raw CPU numbers

[Boqun Feng <boqun.feng@gmail.com>]

On Tue, Jun 10, 2025 at 06:51:57PM +0530, Viresh Kumar wrote:
> 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: 8961b8cb3099 ("rust: cpumask: Add initial abstractions")
> Reported-by: Miguel Ojeda <ojeda@kernel.org>
> Closes: https://lore.kernel.org/rust-for-linux/CANiq72k3ozKkLMinTLQwvkyg9K=BeRxs1oYZSKhJHY-veEyZdg@mail.gmail.com/
> Reported-by: Andreas Hindborg <a.hindborg@kernel.org>
> Closes: https://lore.kernel.org/all/87qzzy3ric.fsf@kernel.org/
> Suggested-by: Boqun Feng <boqun.feng@gmail.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

Reviewed-by: Boqun Feng <boqun.feng@gmail.com>

One nit below..

> ---
>  drivers/cpufreq/rcpufreq_dt.rs |  4 +--
>  rust/kernel/cpu.rs             |  4 +--
>  rust/kernel/cpufreq.rs         | 27 ++++++++++++------
>  rust/kernel/cpumask.rs         | 51 ++++++++++++++++++++++++----------
>  4 files changed, 59 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/cpufreq/rcpufreq_dt.rs b/drivers/cpufreq/rcpufreq_dt.rs
> index 94ed81644fe1..43c87d0259b6 100644
> --- a/drivers/cpufreq/rcpufreq_dt.rs
> +++ b/drivers/cpufreq/rcpufreq_dt.rs
> @@ -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"))?;
> diff --git a/rust/kernel/cpu.rs b/rust/kernel/cpu.rs
> index 6a3aecb12468..7549594fad7f 100644
> --- a/rust/kernel/cpu.rs
> +++ b/rust/kernel/cpu.rs
> @@ -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(cpu.into()) };


I generally found that `u32::from(cpu)` is more clear than `cpu.into()`,
but it's up to you. Same for the rest of `cpu.into()` cases.

Regards,
Boqun

>      if ptr.is_null() {
>          return Err(ENODEV);
>      }
[...]