linux-kernel - Re: [PATCH 6/8] sched/fair: Do not compute NUMA Balancing stats 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: <CAKfTPtCgkrN3-f86zfZOzDMgWvVHFYdcy84K015c52idqXLioQ@mail.gmail.com>
Date: Thu, 12 Dec 2024 14:28:01 +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 6/8] sched/fair: Do not compute NUMA Balancing stats
 unnecessarily during lb

On Thu, 12 Dec 2024 at 12:44, K Prateek Nayak <kprateek.nayak@....com> wrote:
>
> Hello Vincent,
>
> On 12/12/2024 4:35 PM, Vincent Guittot wrote:
> > On Wed, 11 Dec 2024 at 19:58, K Prateek Nayak <kprateek.nayak@....com> wrote:
> >>
> >> Aggregate nr_numa_running and nr_preferred_running when load balancing
> >> at NUMA domains only. While at it, also move the aggregation below the
> >> idle_cpu() check since an idle CPU cannot have any preferred tasks.
> >>
> >> Signed-off-by: K Prateek Nayak <kprateek.nayak@....com>
> >> ---
> >>   kernel/sched/fair.c | 15 +++++++++------
> >>   1 file changed, 9 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> >> index 2c4ebfc82917..ec2a79c8d0e7 100644
> >> --- a/kernel/sched/fair.c
> >> +++ b/kernel/sched/fair.c
> >> @@ -10340,7 +10340,7 @@ static inline void update_sg_lb_stats(struct lb_env *env,
> >>                                        bool *sg_overloaded,
> >>                                        bool *sg_overutilized)
> >>   {
> >> -       int i, nr_running, local_group;
> >> +       int i, nr_running, local_group, sd_flags = env->sd->flags;
> >>
> >>          memset(sgs, 0, sizeof(*sgs));
> >>
> >> @@ -10364,10 +10364,6 @@ static inline void update_sg_lb_stats(struct lb_env *env,
> >>                  if (cpu_overutilized(i))
> >>                          *sg_overutilized = 1;
> >>
> >> -#ifdef CONFIG_NUMA_BALANCING
> >> -               sgs->nr_numa_running += rq->nr_numa_running;
> >> -               sgs->nr_preferred_running += rq->nr_preferred_running;
> >> -#endif
> >>                  /*
> >>                   * No need to call idle_cpu() if nr_running is not 0
> >>                   */
> >> @@ -10377,10 +10373,17 @@ static inline void update_sg_lb_stats(struct lb_env *env,
> >>                          continue;
> >>                  }
> >>
> >> +#ifdef CONFIG_NUMA_BALANCING
> >> +               /* Only fbq_classify_group() uses this to classify NUMA groups */
> >
> > and fbq_classify_rq() which is also used by non-NUMA groups.
>
> Yup but that just looks at rq's "nr_numa_running" and
> "nr_preferred_running".

Ah yes, I misread and mixed rq and not sgs.  You can forget my comment

Reviewed-by: Vincent Guittot <vincent.guittot@...aro.org>

>
> > AFAICT
> > It doesn't change anything at the end because group type is "all" for
> > non numa groups but we need some explanations why It's ok to skip numa
> > stats and default behavior will remain unchanged
>
> I'll elaborate that comment with complete details:
>
>         /*
>          * Only fbq_classify_group() uses these NUMA stats to derive the
>          * fbq_type of NUMA groups. By default, it is initialized to
>          * "all" - the highest type. sched_balance_find_src_rq() inhibits
>          * load balancing from runqueue whose fbq_type is found to be
>          * higher than the busiest group's fbq_type but since it is
>          * always initialized to the largest value, and remains same for
>          * non-NUMA groups, skip this aggregation when balancing at
>          * non-NUMA domains.
>          */
>
> --
> Thanks and Regards,
> Prateek
>
> >
> >> +               if (sd_flags & SD_NUMA) {
> >> +                       sgs->nr_numa_running += rq->nr_numa_running;
> >> +                       sgs->nr_preferred_running += rq->nr_preferred_running;
> >> +               }
> >> +#endif
> >>                  if (local_group)
> >>                          continue;
> >>
> >> -               if (env->sd->flags & SD_ASYM_CPUCAPACITY) {
> >> +               if (sd_flags & SD_ASYM_CPUCAPACITY) {
> >>                          /* Check for a misfit task on the cpu */
> >>                          if (sgs->group_misfit_task_load < rq->misfit_task_load) {
> >>                                  sgs->group_misfit_task_load = rq->misfit_task_load;
> >> --
> >> 2.34.1
> >>
>
>