diff mbox series

[1/2] sched/fair: Take thermal pressure into account while estimating energy

Message ID 20210602135609.10867-2-lukasz.luba@arm.com
State New
Headers show
Series [1/2] sched/fair: Take thermal pressure into account while estimating energy | expand

Commit Message

Lukasz Luba June 2, 2021, 1:56 p.m. UTC
Energy Aware Scheduling (EAS) needs to be able to predict the frequency
requests made by the SchedUtil governor to properly estimate energy used
in the future. It has to take into account CPUs utilization and forecast
Performance Domain (PD) frequency. There is a corner case when the max
allowed frequency might be reduced due to thermal. SchedUtil is aware of
that reduced frequency, so it should be taken into account also in EAS
estimations.

SchedUtil, as a CPUFreq governor, knows the maximum allowed frequency of
a CPU, thanks to cpufreq_driver_resolve_freq() and internal clamping
to 'policy::max'. SchedUtil is responsible to respect that upper limit
while setting the frequency through CPUFreq drivers. This effective
frequency is stored internally in 'sugov_policy::next_freq' and EAS has
to predict that value.

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
geometric series in similar way as other mechanisms in the scheduler.

Thanks to knowledge about allowed CPU capacity, we don't get too big value
for a single CPU utilization, which is then added to the util sum. The
util sum is used as a source of information for estimating whole PD energy.
To avoid wrong energy estimation in EAS (due to capped frequency), make
sure that the calculation of util sum is aware of allowed CPU capacity.

Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
---
 kernel/sched/fair.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Quentin Perret June 2, 2021, 3 p.m. UTC | #1
Hi Lukasz,

On Wednesday 02 Jun 2021 at 14:56:08 (+0100), Lukasz Luba wrote:
>  compute_energy(struct task_struct *p, int dst_cpu, struct perf_domain *pd)
>  {
>  	struct cpumask *pd_mask = perf_domain_span(pd);
> -	unsigned long cpu_cap = arch_scale_cpu_capacity(cpumask_first(pd_mask));
> +	unsigned long _cpu_cap = arch_scale_cpu_capacity(cpumask_first(pd_mask));
>  	unsigned long max_util = 0, sum_util = 0;
> +	unsigned long cpu_cap = _cpu_cap;
>  	int cpu;
>  
>  	/*
> @@ -6558,6 +6559,14 @@ compute_energy(struct task_struct *p, int dst_cpu, struct perf_domain *pd)
>  				cpu_util_next(cpu, p, -1) + task_util_est(p);
>  		}
>  
> +		/*
> +		 * Take the thermal pressure from non-idle CPUs. They have
> +		 * most up-to-date information. For idle CPUs thermal pressure
> +		 * signal is not updated so often.
> +		 */
> +		if (!idle_cpu(cpu))
> +			cpu_cap = _cpu_cap - thermal_load_avg(cpu_rq(cpu));

This messes up the irq time scaling no? Maybe move the capping in this
function instead of relying on effective_cpu_util() to do it for you?

>  		/*
>  		 * Busy time computation: utilization clamping is not
>  		 * required since the ratio (sum_util / cpu_capacity)
> -- 
> 2.17.1
>
Lukasz Luba June 2, 2021, 3:35 p.m. UTC | #2
Hi Quentin,

On 6/2/21 4:00 PM, Quentin Perret wrote:
> Hi Lukasz,
> 
> On Wednesday 02 Jun 2021 at 14:56:08 (+0100), Lukasz Luba wrote:
>>   compute_energy(struct task_struct *p, int dst_cpu, struct perf_domain *pd)
>>   {
>>   	struct cpumask *pd_mask = perf_domain_span(pd);
>> -	unsigned long cpu_cap = arch_scale_cpu_capacity(cpumask_first(pd_mask));
>> +	unsigned long _cpu_cap = arch_scale_cpu_capacity(cpumask_first(pd_mask));
>>   	unsigned long max_util = 0, sum_util = 0;
>> +	unsigned long cpu_cap = _cpu_cap;
>>   	int cpu;
>>   
>>   	/*
>> @@ -6558,6 +6559,14 @@ compute_energy(struct task_struct *p, int dst_cpu, struct perf_domain *pd)
>>   				cpu_util_next(cpu, p, -1) + task_util_est(p);
>>   		}
>>   
>> +		/*
>> +		 * Take the thermal pressure from non-idle CPUs. They have
>> +		 * most up-to-date information. For idle CPUs thermal pressure
>> +		 * signal is not updated so often.
>> +		 */
>> +		if (!idle_cpu(cpu))
>> +			cpu_cap = _cpu_cap - thermal_load_avg(cpu_rq(cpu));
> 
> This messes up the irq time scaling no? Maybe move the capping in this

You are talking about scale_irq_capacity() which shrinks the util by
some percentage of irq time. It might be different, by some fraction
(e.g. 8/9 vs 9/10) compared to SchedUtil view, which passes 'raw' arch
capacity. It then adds the irq part, but still to this slightly
different base util.

> function instead of relying on effective_cpu_util() to do it for you?

Agree, since it would be more 'aligned' with how SchedUtil calls
effective_cpu_util(). I will clamp the returned value.

Thanks for pointing this out.

Regards,
Lukasz
diff mbox series

Patch

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 161b92aa1c79..ca0a6f1408da 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6525,8 +6525,9 @@  static long
 compute_energy(struct task_struct *p, int dst_cpu, struct perf_domain *pd)
 {
 	struct cpumask *pd_mask = perf_domain_span(pd);
-	unsigned long cpu_cap = arch_scale_cpu_capacity(cpumask_first(pd_mask));
+	unsigned long _cpu_cap = arch_scale_cpu_capacity(cpumask_first(pd_mask));
 	unsigned long max_util = 0, sum_util = 0;
+	unsigned long cpu_cap = _cpu_cap;
 	int cpu;
 
 	/*
@@ -6558,6 +6559,14 @@  compute_energy(struct task_struct *p, int dst_cpu, struct perf_domain *pd)
 				cpu_util_next(cpu, p, -1) + task_util_est(p);
 		}
 
+		/*
+		 * Take the thermal pressure from non-idle CPUs. They have
+		 * most up-to-date information. For idle CPUs thermal pressure
+		 * signal is not updated so often.
+		 */
+		if (!idle_cpu(cpu))
+			cpu_cap = _cpu_cap - thermal_load_avg(cpu_rq(cpu));
+
 		/*
 		 * Busy time computation: utilization clamping is not
 		 * required since the ratio (sum_util / cpu_capacity)