On 01/02/2024 14:11, Hongyan Xia wrote:
From: Hongyan Xia <Hongyan.Xia2@xxxxxxx>
That commit creates further problems because 0 spare capacity can be
either a real indication that the CPU is maxed out, or the CPU is
UCLAMP_MAX throttled, but we end up giving all of them a chance which
can results in bogus energy calculations. It also tends to schedule
tasks on the same CPU and requires load balancing patches. Sum
aggregation solves these problems and this patch is not needed.
This reverts commit 6b00a40147653c8ea748e8f4396510f252763364.
I assume you did this revert especially for the 'Scenario 5: 8 tasks
with UCLAMP_MAX of 120' testcase?
IMHO, the issue is especially visible in compute_energy()'s busy_time
computation with a valid destination CPU (dst_cpu >= 0). I.e. when we
have to add performance domain (pd) and task busy time.
find_energy_efficient_cpu() (feec())
for each pd
for each cpu in pd
set {prev_,max}_spare_cap
bail if prev_ and max_spare_cap < 0 (was == 0 before )
{base_,prev_,cur_}energy = compute_energy
So with the patch we potentially compute energy for a saturated PD
according:
compute_energy()
if (dst_cpu >= 0)
busy_time = min(eenv->pd_cap, eenv->busy_time + eenv->task_busy_time)
<----(a)---> <--------------(b)------------------->
energy = em_cpu_energy(pd->em_pd, max_util, busy_time, eenv->cpu_cap)
If (b) > (a) then we're saturated and 'energy' is bogus.
The way to fix this is up for discussion:
(1) feec() returning prev_cpu
(2) feec() returning -1 (forcing wakeup into sis() -> sic())
(3) using uclamped values for task and rq utilization
None of those have immediately given the desired task placement on
mainline (2 tasks on each of the 4 little CPUs and no task on the 2 big
CPUs on my [l B B l l l] w/ CPU capacities = [446 1024 1024 446 446 446]
machine) you can achieve with uclamp sum aggregation.
[...]