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

[Viresh Kumar]

On 20-03-26, 12:32, Lukasz Luba 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.

I think following commit introduced a bug by removing `i++`.

commit 3f7ced7ac9af ("drivers/thermal/cpufreq_cooling : Refactor thermal_power_cpu_get_power tracing")

-- 
viresh