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

[Lukasz Luba]

On 6/14/21 5:03 PM, Vincent Guittot wrote:
> On Thu, 10 Jun 2021 at 17:03, Lukasz Luba <lukasz.luba@xxxxxxx> 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@xxxxxxxxxx>

Thank you for the review!

Regards,
Lukasz