linux-kernel - Re: [PATCH] sched/fair: Add SMT4 group_smt

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]

Date:   Wed, 06 Sep 2023 08:42:44 -0700
From:   Tim Chen <tim.c.chen@...ux.intel.com>
To:     Shrikanth Hegde <sshegde@...ux.vnet.ibm.com>
Cc:     bristot@...hat.com, bsegall@...gle.com, dietmar.eggemann@....com,
        hdanton@...a.com, ionela.voinescu@....com, juri.lelli@...hat.com,
        len.brown@...el.com, linux-kernel@...r.kernel.org, mgorman@...e.de,
        naveen.n.rao@...ux.vnet.ibm.com, rafael.j.wysocki@...el.com,
        ravi.v.shankar@...el.com, ricardo.neri@...el.com,
        rostedt@...dmis.org, srikar@...ux.vnet.ibm.com,
        srinivas.pandruvada@...ux.intel.com, v-songbaohua@...o.com,
        vincent.guittot@...aro.org, vschneid@...hat.com, x86@...nel.org,
        yangyicong@...ilicon.com, yu.c.chen@...el.com, peterz@...radead.org
Subject: Re: [PATCH] sched/fair: Add SMT4 group_smt_balance handling

On Wed, 2023-09-06 at 14:59 +0530, Shrikanth Hegde wrote:
> > 
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 6e7ee2efc1ba..48e9ab7f8a87 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -9764,16 +9764,9 @@ static bool update_sd_pick_busiest(struct lb_env *env,
> >  
> >         case group_smt_balance:
> >                 /* no idle cpus on both groups handled by group_fully_busy below */
> 
> Please add a comment here explaining the fall-through and spare logic.
> 

Sure.


> > -               if (sgs->idle_cpus != 0 || busiest->idle_cpus != 0) {
> > -                       if (sgs->idle_cpus > busiest->idle_cpus)
> > -                               return false;
> > -                       if (sgs->idle_cpus < busiest->idle_cpus)
> > -                               return true;
> > -                       if (sgs->sum_nr_running <= busiest->sum_nr_running)
> > -                               return false;
> > -                       else
> > -                               return true;
> > -               }
> > +               if (sgs->idle_cpus != 0 || busiest->idle_cpus != 0)
> > +                       goto has_spare;
> > +
> >                 goto fully_busy;
> 
> This can fall through without the additional goto statement no?
> 

There is an unconditional goto fully_busy so won't fall through and
compiler won't complain.

> >  
> >         case group_fully_busy:
> > @@ -9809,6 +9802,7 @@ static bool update_sd_pick_busiest(struct lb_env *env,
> >                  * as we do not want to pull task off SMT core with one task
> >                  * and make the core idle.
> >                  */
> > +has_spare:
> >                 if (smt_vs_nonsmt_groups(sds->busiest, sg)) {
> >                         if (sg->flags & SD_SHARE_CPUCAPACITY && sgs->sum_h_nr_running <= 1)
> >                                 return false;
> > 
> > 
> > 
Tim