linux-kernel - Re: [PATCH 7/8] sched/fair: Do not compute overloaded status unnecessarily during lb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ef5d950e-81bb-49c8-89ab-4f62a9529994@amd.com>
Date: Sat, 14 Dec 2024 01:21:51 +0530
From: K Prateek Nayak <kprateek.nayak@....com>
To: Shrikanth Hegde <sshegde@...ux.ibm.com>
CC: "H. Peter Anvin" <hpa@...or.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>, "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
	Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>, Tim Chen
	<tim.c.chen@...ux.intel.com>, Mario Limonciello <mario.limonciello@....com>,
	Meng Li <li.meng@....com>, Huang Rui <ray.huang@....com>, "Gautham R. Shenoy"
	<gautham.shenoy@....com>, Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar
	<mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, Dave Hansen
	<dave.hansen@...ux.intel.com>, Peter Zijlstra <peterz@...radead.org>, "Juri
 Lelli" <juri.lelli@...hat.com>, Vincent Guittot <vincent.guittot@...aro.org>,
	<x86@...nel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 7/8] sched/fair: Do not compute overloaded status
 unnecessarily during lb

Hello Shrikanth,

Thank you for reviewing the series.

On 12/13/2024 8:27 PM, Shrikanth Hegde wrote:
> 
> 
> On 12/12/24 00:25, K Prateek Nayak wrote:
>> Only set sg_overloaded when computing sg_lb_stats() at the highest sched
>> domain since rd->overloaded status is updated only when load balancing
>> at the highest domain. While at it, move setting of sg_overloaded below
>> idle_cpu() check since an idle CPU can never be overloaded.
>>
>> Signed-off-by: K Prateek Nayak <kprateek.nayak@....com>
>> ---
>>   kernel/sched/fair.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index ec2a79c8d0e7..3f36805ecdca 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -10358,9 +10358,6 @@ static inline void update_sg_lb_stats(struct lb_env *env,
>>           nr_running = rq->nr_running;
>>           sgs->sum_nr_running += nr_running;
>> -        if (nr_running > 1)
>> -            *sg_overloaded = 1;
>> -
>>           if (cpu_overutilized(i))
>>               *sg_overutilized = 1;
> 
> Maybe its worth moving the overutilized too after the idle checks. An idle cpu can't be overutilized right?

Since there are no tasks on the CPU, there are no UCLAMP constraints
which means the cpu_overutilized() boils down to:

	!fits_capacity(cpu_util_cfs(cpu), capacity_of(cpu))

But the averages can capture blocked averages and capacity_of(cpu) can
also change with arch_scale_cpu_capacity() so I cannot say for sure
with 100% confident that an idle CPU cannot appear overutilized.

Vincent, Dietmar, do you think it is possible for an idle CPU to be
overutilized? If not, I'll move this update below idle_cpu() check too.

> 
>> @@ -10373,6 +10370,10 @@ static inline void update_sg_lb_stats(struct lb_env *env,
>>               continue;
>>           }
>> +        /* Overload indicator is only updated at root domain */
>> +        if (!env->sd->parent && nr_running > 1)
>> +            *sg_overloaded = 1;
>> +
>>   #ifdef CONFIG_NUMA_BALANCING
>>           /* Only fbq_classify_group() uses this to classify NUMA groups */
>>           if (sd_flags & SD_NUMA) {
> 
> Other than the point above,
> Reviewed-by: Shrikanth Hegde <sshegde@...ux.ibm.com>

Thank you!

-- 
Thanks and Regards,
Prateek