Re: [PATCH v3 2/3] sched/fair: Take thermal pressure into account while estimating energy

[Lukasz Luba <lukasz.luba@arm.com>]

On 6/14/21 5:03 PM, Vincent Guittot wrote:
> On Thu, 10 Jun 2021 at 17:03, Lukasz Luba <lukasz.luba@arm.com> wrote:

[snip]

>> In the existing code the raw value of arch_scale_cpu_capacity() is used
>> for clamping the returned CPU utilization from effective_cpu_util().
>> This patch fixes issue with too big single CPU utilization, by introducing
>> clamping to the allowed CPU capacity. The allowed CPU capacity is a CPU
>> capacity reduced by thermal pressure signal. We rely on this load avg
> 
> you don't rely on load avg value but on raw thermal pressure value now

Good catch, I'll change that description.

> 
>> geometric series in similar way as other mechanisms in the scheduler.
>>

[snip]

>>
>> +       thermal_pressure = arch_scale_thermal_pressure(cpumask_first(pd_mask));
> 
> Do you really need to use this intermediate variable thermal_pressure
> ? Seems to be used only below

True, it's used only here. I'll remove this variable in the v4.

> 
> With these 2 comments above fixed,
> 
> Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>

Thank you for the review!

Regards,
Lukasz