Received-SPF: pass (google.com: domain of linux-kernel+bounces-86375-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <26af2033-c5e0-43a1-8953-35435ffbbdd0@arm.com>
Date: Thu, 29 Feb 2024 10:11:09 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 1/2] sched/fair: Add EAS checks before updating
 overutilized
To: Shrikanth Hegde <sshegde@linux.ibm.com>
Cc: yu.c.chen@intel.com, dietmar.eggemann@arm.com,
 linux-kernel@vger.kernel.org, nysal@linux.ibm.com, aboorvad@linux.ibm.com,
 srikar@linux.vnet.ibm.com, vschneid@redhat.com, morten.rasmussen@arm.com,
 qyousef@layalina.io, mingo@kernel.org, peterz@infradead.org,
 vincent.guittot@linaro.org
References: <20240228071621.602596-1-sshegde@linux.ibm.com>
 <20240228071621.602596-2-sshegde@linux.ibm.com>
 <86f000a3-3068-4c57-858d-c00e3eb6e974@arm.com>
 <72c2d279-90ae-4612-9b96-e579333b8088@linux.ibm.com>
Content-Language: en-US
From: Pierre Gondois <pierre.gondois@arm.com>
In-Reply-To: <72c2d279-90ae-4612-9b96-e579333b8088@linux.ibm.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

Hello Shrikanth,

On 2/28/24 18:24, Shrikanth Hegde wrote:
> 
> 
> On 2/28/24 9:28 PM, Pierre Gondois wrote:
> 
> Hi Pierre, Thanks for taking a look.
> 
>> It is nice to avoid calling effective_cpu_util() through the following
>> when EAS is not enabled:
>> I think we are avoiding calling cpu_overutilized except in update_sg_lb_stats.
> I didnt want to put a EAS check in cpu_overutilized as it could be useful
> function in non-EAS cases in future. calling cpu_overutilized alone doesnt
> do any access to root_domain's overutilized field. So we are okay w.r.t to
> cache issues.
> But we will do some extra computation currently and then not use it if it
> Non-EAS case in update_sg_lb_stats
> 
> Would something like this makes sense?
> @@ -9925,7 +9925,7 @@ static inline void update_sg_lb_stats(struct lb_env *env,
>                  if (nr_running > 1)
>                          *sg_status |= SG_OVERLOAD;
>   
> -               if (cpu_overutilized(i))
> +               if (sched_energy_enabled() && cpu_overutilized(i))
>                          *sg_status |= SG_OVERUTILIZED;
>   

Yes right. I think that what Dietmar suggested is also a good idea
which could be used instead.

> 
> 
> 
> I didnt find how would util_fits_cpu ends up calling effective_cpu_util.
> Could you please elaborate?

Sorry I meant this path:
cpu_overutilized()
\-cpu_util_cfs()
   \-cpu_util()

effective_cpu_util() is effectively not involved.

> 
>> cpu_overutilized()
>> \-util_fits_cpu()
>>    \- ...
>>      \-effective_cpu_util()
>>

[snip]

Regards,
Pierre