Received-SPF: pass (google.com: domain of linux-kernel+bounces-107723-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <37be0494-7e38-4275-b6eb-62a2eb2f6d46@arm.com>
Date: Tue, 19 Mar 2024 16:34:16 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [RFC PATCH v2 1/7] Revert "sched/uclamp: Set max_spare_cap_cpu
 even if max_spare_cap is 0"
Content-Language: en-US
To: Hongyan Xia <hongyan.xia2@arm.com>, Ingo Molnar <mingo@redhat.com>,
 Peter Zijlstra <peterz@infradead.org>,
 Vincent Guittot <vincent.guittot@linaro.org>,
 Juri Lelli <juri.lelli@redhat.com>, Steven Rostedt <rostedt@goodmis.org>,
 Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
 Daniel Bristot de Oliveira <bristot@redhat.com>,
 Valentin Schneider <vschneid@redhat.com>
Cc: Qais Yousef <qyousef@layalina.io>,
 Morten Rasmussen <morten.rasmussen@arm.com>,
 Lukasz Luba <lukasz.luba@arm.com>,
 Christian Loehle <christian.loehle@arm.com>, linux-kernel@vger.kernel.org,
 David Dai <davidai@google.com>, Saravana Kannan <saravanak@google.com>
References: <cover.1706792708.git.hongyan.xia2@arm.com>
 <b29e7df921ce07c2c2dbbde390e234d162756c42.1706792708.git.hongyan.xia2@arm.com>
From: Dietmar Eggemann <dietmar.eggemann@arm.com>
In-Reply-To: <b29e7df921ce07c2c2dbbde390e234d162756c42.1706792708.git.hongyan.xia2@arm.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

On 01/02/2024 14:11, Hongyan Xia wrote:
> From: Hongyan Xia <Hongyan.Xia2@arm.com>
> 
> 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.

[...]