Re: [RFC PATCH 1/2] thermal/cpufreq_cooling: remove unused cpu_idx in get_load()

[Xuewen Yan]

On Fri, Mar 20, 2026 at 8:32 PM Lukasz Luba <lukasz.luba@xxxxxxx> wrote:
>
> Hi Xuewen,
>
> On 3/20/26 11:31, Xuewen Yan wrote:
> > From: Di Shen <di.shen@xxxxxxxxxx>
> >
> > The cpu_idx variable in the get_load function is now
> > unused and can be safely removed.
> >
> > No code logic is affected.
> >
> > Signed-off-by: Di Shen <di.shen@xxxxxxxxxx>
> > ---
> >   drivers/thermal/cpufreq_cooling.c | 13 +++++--------
> >   1 file changed, 5 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/thermal/cpufreq_cooling.c b/drivers/thermal/cpufreq_cooling.c
> > index 32bf5ab44f4a..d030dbeb2973 100644
> > --- a/drivers/thermal/cpufreq_cooling.c
> > +++ b/drivers/thermal/cpufreq_cooling.c
> > @@ -151,26 +151,23 @@ static u32 cpu_power_to_freq(struct cpufreq_cooling_device *cpufreq_cdev,
> >    * get_load() - get load for a cpu
> >    * @cpufreq_cdev: struct cpufreq_cooling_device for the cpu
> >    * @cpu: cpu number
> > - * @cpu_idx: index of the cpu in time_in_idle array
> >    *
> >    * Return: The average load of cpu @cpu in percentage since this
> >    * function was last called.
> >    */
> >   #ifdef CONFIG_SMP
> > -static u32 get_load(struct cpufreq_cooling_device *cpufreq_cdev, int cpu,
> > -                 int cpu_idx)
> > +static u32 get_load(struct cpufreq_cooling_device *cpufreq_cdev, int cpu)
> >   {
> >       unsigned long util = sched_cpu_util(cpu);
> >
> >       return (util * 100) / arch_scale_cpu_capacity(cpu);
> >   }
> >   #else /* !CONFIG_SMP */
> > -static u32 get_load(struct cpufreq_cooling_device *cpufreq_cdev, int cpu,
> > -                 int cpu_idx)
> > +static u32 get_load(struct cpufreq_cooling_device *cpufreq_cdev, int cpu)
> >   {
> >       u32 load;
> >       u64 now, now_idle, delta_time, delta_idle;
> > -     struct time_in_idle *idle_time = &cpufreq_cdev->idle_time[cpu_idx];
> > +     struct time_in_idle *idle_time = &cpufreq_cdev->idle_time[cpu];
>
> This is a bug. We allocate 'num_cpus' size of array based on
> number of CPU in the cpumask for a given cpufreq policy.
> If there are 4 cpus in the CPU cluster but CPUs have ids:
> CPU4-7 then accessing it with this change would explode.

Sorry, it was our oversight. However, this is fine without the other patch.
For patch-v2, maybe we can add back i++ to work around this issue.

Thanks!

>
> Please re-design this patch set slightly and I will have a look
> on the 2nd version (and particularly the part in current patch 2/2).
>
> Regards,
> Lukasz