Message-ID: <3c21871b-4936-4143-bc78-38495a7995a6@arm.com>
Date: Thu, 27 Mar 2025 10:35:35 +0100
From: Pierre Gondois <pierre.gondois@....com>
To: Vincent Guittot <vincent.guittot@...aro.org>
Cc: linux-kernel@...r.kernel.org, Chritian Loehle <christian.loehle@....com>,
 Hongyan Xia <hongyan.xia2@....com>, Ingo Molnar <mingo@...hat.com>,
 Peter Zijlstra <peterz@...radead.org>, Juri Lelli <juri.lelli@...hat.com>,
 Dietmar Eggemann <dietmar.eggemann@....com>,
 Steven Rostedt <rostedt@...dmis.org>, Ben Segall <bsegall@...gle.com>,
 Mel Gorman <mgorman@...e.de>, Valentin Schneider <vschneid@...hat.com>
Subject: Re: [PATCH] sched/fair: Allow decaying util_est when util_avg > CPU
 capa



On 3/26/25 18:25, Vincent Guittot wrote:
> On Tue, 25 Mar 2025 at 16:06, Pierre Gondois <pierre.gondois@....com> wrote:
>>
>> commit 10a35e6812aa ("sched/pelt: Skip updating util_est when
>> utilization is higher than CPU's capacity")
>> prevents util_est from being updated if util_avg is higher than the
>> underlying CPU capacity to avoid overestimating the task when the CPU
>> is capped (due to thermal issue for instance). In this scenario, the
>> task will miss its deadlines and start overlapping its wake-up events
>> for instance. The task will appear as always running when the CPU is
>> just not powerful enough to allow having a good estimation of the
>> task.
>>
>> commit b8c96361402a ("sched/fair/util_est: Implement faster ramp-up
>> EWMA on utilization increases")
>> sets ewma to util_avg when ewma > util_avg, allowing ewma to quickly
>> grow instead of slowly converge to the new util_avg value when a task
>> profile changes from small to big.
>>
>> However, the 2 conditions:
>> - Check util_avg against max CPU capacity
>> - Check whether util_est > util_avg
>> are placed in an order such as it is possible to set util_est to a
>> value higher than the CPU capacity if util_est > util_avg, but
>> util_est is prevented to decay as long as:
>> CPU capacity < util_avg < util_est.
>>
>> Just remove the check as either:
>> 1.
>> There is idle time on the CPU. In that case the util_avg value of the
>> task is actually correct. It is possible that the task missed a
>> deadline and appears bigger, but this is also the case when the
>> util_avg of the task is lower than the maximum CPU capacity.
>> 2.
>> There is no idle time. In that case, the util_avg value might aswell
>> be an under estimation of the size of the task.
>> It is possible that undesired frequency spikes will appear when the
>> task is later enqueued with an inflated util_est value, but the
>> frequency spike might aswell be deserved. The absence of idle time
>> prevents from drawing any conclusion.
>>
>> Signed-off-by: Pierre Gondois <pierre.gondois@....com>
> 
> This change looks reasonable to me. Did you face problems related to
> this in a particular use case ?

I think it was more related to the fact util_est is not decayed when:
(runnable - util_avg) > margin

This patch slightly helps to decay, but not that much.

> 
> Reviewed-by: Vincent Guittot <vincent.guittot@...aro.rog>

Thanks!
> 
> 
>> ---
>>   kernel/sched/fair.c | 7 -------
>>   1 file changed, 7 deletions(-)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index c798d2795243..de7687e579c2 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -4918,13 +4918,6 @@ static inline void util_est_update(struct cfs_rq *cfs_rq,
>>          if (last_ewma_diff < UTIL_EST_MARGIN)
>>                  goto done;
>>
>> -       /*
>> -        * To avoid overestimation of actual task utilization, skip updates if
>> -        * we cannot grant there is idle time in this CPU.
>> -        */
>> -       if (dequeued > arch_scale_cpu_capacity(cpu_of(rq_of(cfs_rq))))
>> -               return;
>> -
>>          /*
>>           * To avoid underestimate of task utilization, skip updates of EWMA if
>>           * we cannot grant that thread got all CPU time it wanted.
>> --
>> 2.25.1
>>

Powered by blists - more mailing lists