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: <CAKfTPtDN2rrKVgdDxtSeFtdQkjLXV7FUx_0JmYc1Fv4FZZaLdw@mail.gmail.com>
Date: Thu, 12 Dec 2024 12:18:38 +0100
From: Vincent Guittot <vincent.guittot@...aro.org>
To: K Prateek Nayak <kprateek.nayak@....com>
Cc: 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>, x86@...nel.org, linux-kernel@...r.kernel.org, 
	"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>
Subject: Re: [PATCH 7/8] sched/fair: Do not compute overloaded status
 unnecessarily during lb

On Thu, 12 Dec 2024 at 12:01, K Prateek Nayak <kprateek.nayak@....com> wrote:
>
> Hello Vincent,
>
> Thank you for reviewing the patch.
>
> On 12/12/2024 3:26 PM, Vincent Guittot wrote:
> > On Wed, 11 Dec 2024 at 19:58, K Prateek Nayak <kprateek.nayak@....com> 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;
> >>
> >> @@ -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)
> >
> > nit: may be worth checking local variable 1st which should be cheaper
> > than env->sd->parent
>
> What are your thoughts on passing NULL for "sg_overloaded" when calling
> update_sg_lb_stats() at non-root domains? Would it be equally cheap to
> do:
>
>         if (sg_overloaded && nr_running > 1)
>                 *sg_overloaded = 1;

you will have to test it twice as it is also set for misfit task

>
> >
> > Reviewed-by: Vincent Guittot <vincent.guittot@...aro.org>
>
> Thank you.
>
> >
> >
> >> +                       *sg_overloaded = 1;
> >> +
> >>   #ifdef CONFIG_NUMA_BALANCING
> >>                  /* Only fbq_classify_group() uses this to classify NUMA groups */
> >>                  if (sd_flags & SD_NUMA) {
> >> --
> >> 2.34.1
> >>
>
> --
> Thanks and Regards,
> Prateek
>