| Message ID | a7c1fe73-b40e-437c-8ccb-7b3baad04df7@moroto.mountain (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | thermal/cpufreq: increment i in cpufreq_get_requested_power() | expand |
Hi Dan, On 5/4/24 12:25, Dan Carpenter wrote: > We accidentally deleted the "i++" as part of a cleanup. Restore it. > > Fixes: 3f7ced7ac9af ("drivers/thermal/cpufreq_cooling : Refactor thermal_power_cpu_get_power tracing") > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > --- > This is based on static analysis and not tested. Thank you for the patch. I have analyzed the code and why it haven't trigger an issue when I was testing it. I looks like the function get_load() which is called above that 'i++' and takes the 'i' as the last argument is compiled in 2 versions: 1. for SMP system and the last argument 'cpu_idx' is ignored 2. for !SMP where we use the last argument 'cpu_idx' which is 'i' value. Although, for !SMP system we only have 1 cpu, thus the initialized 'int i = 0' at the beginning of that cpufreq_get_requested_power() is used correctly. The loop for !SMP goes only once. > > drivers/thermal/cpufreq_cooling.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/thermal/cpufreq_cooling.c b/drivers/thermal/cpufreq_cooling.c > index 280071be30b1..a074192896de 100644 > --- a/drivers/thermal/cpufreq_cooling.c > +++ b/drivers/thermal/cpufreq_cooling.c > @@ -249,6 +249,7 @@ static int cpufreq_get_requested_power(struct thermal_cooling_device *cdev, > load = 0; > > total_load += load; > + i++; > } > > cpufreq_cdev->last_load = total_load; Would you agree that I will keep you as 'Reported-by' and send a separate patch to change that !SMP code completely in that get_load() function and get rid of the 'cpu_idx' argument? Or I'm happy that you can develop such code and I can review it. It's up to you. Regards, Lukasz
On Mon, May 06, 2024 at 10:41:52AM +0100, Lukasz Luba wrote: > Would you agree that I will keep you as 'Reported-by' and send a > separate patch to change that !SMP code completely in that > get_load() function and get rid of the 'cpu_idx' argument? Yes, please. regards, dan carpenter
diff --git a/drivers/thermal/cpufreq_cooling.c b/drivers/thermal/cpufreq_cooling.c index 280071be30b1..a074192896de 100644 --- a/drivers/thermal/cpufreq_cooling.c +++ b/drivers/thermal/cpufreq_cooling.c @@ -249,6 +249,7 @@ static int cpufreq_get_requested_power(struct thermal_cooling_device *cdev, load = 0; total_load += load; + i++; } cpufreq_cdev->last_load = total_load;
We accidentally deleted the "i++" as part of a cleanup. Restore it. Fixes: 3f7ced7ac9af ("drivers/thermal/cpufreq_cooling : Refactor thermal_power_cpu_get_power tracing") Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> --- This is based on static analysis and not tested. drivers/thermal/cpufreq_cooling.c | 1 + 1 file changed, 1 insertion(+)