linux-kernel - Re: [PATCH v3 06/10] sched/fair: Use the prefer

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: <fb63369b2fc686730adefb6800eae4877e62e3b6.camel@linux.intel.com>
Date:   Thu, 09 Feb 2023 15:05:03 -0800
From:   Tim Chen <tim.c.chen@...ux.intel.com>
To:     "Chen, Tim C" <tim.c.chen@...el.com>,
        "Chen, Yu C" <yu.c.chen@...el.com>,
        Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>
Cc:     "Peter Zijlstra (Intel)" <peterz@...radead.org>,
        Juri Lelli <juri.lelli@...hat.com>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        "Neri, Ricardo" <ricardo.neri@...el.com>,
        "Shankar, Ravi V" <ravi.v.shankar@...el.com>,
        Ben Segall <bsegall@...gle.com>,
        Daniel Bristot de Oliveira <bristot@...hat.com>,
        Dietmar Eggemann <dietmar.eggemann@....com>,
        "Brown, Len" <len.brown@...el.com>, Mel Gorman <mgorman@...e.de>,
        "Wysocki, Rafael J" <rafael.j.wysocki@...el.com>,
        Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Valentin Schneider <vschneid@...hat.com>,
        Ionela Voinescu <ionela.voinescu@....com>,
        "x86@...nel.org" <x86@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 06/10] sched/fair: Use the prefer_sibling flag of the
 current sched domain

On Thu, 2023-02-09 at 20:00 +0000, Chen, Tim C wrote:
> > >  static inline void update_sd_lb_stats(struct lb_env *env, struct
> > > sd_lb_stats *sds)  {
> > > -       struct sched_domain *child = env->sd->child;
> > >         struct sched_group *sg = env->sd->groups;
> > >         struct sg_lb_stats *local = &sds->local_stat;
> > >         struct sg_lb_stats tmp_sgs;
> > > @@ -10045,9 +10044,11 @@ static inline void
> > > update_sd_lb_stats(struct
> > lb_env *env, struct sd_lb_stats *sd
> > >                 sg = sg->next;
> > >         } while (sg != env->sd->groups);
> > > 
> > > -       /* Tag domain that child domain prefers tasks go to
> > > siblings first */
> > > -       sds->prefer_sibling = child && child->flags &
> > > SD_PREFER_SIBLING;
> > > -
> > > +       /*
> > > +        * Tag domain that @env::sd prefers to spread excess
> > > tasks among
> > > +        * sibling sched groups.
> > > +        */
> > > +       sds->prefer_sibling = env->sd->flags & SD_PREFER_SIBLING;
> > > 
> > This does help fix the issue that non-SMT core fails to pull task
> > from busy SMT-
> > cores.
> > And it also semantically changes the definination of prefer
> > sibling. Do we also
> > need to change this:
> >        if ((sd->flags & SD_ASYM_CPUCAPACITY) && sd->child)
> >                sd->child->flags &= ~SD_PREFER_SIBLING; might be:
> >        if ((sd->flags & SD_ASYM_CPUCAPACITY))
> >                sd->flags &= ~SD_PREFER_SIBLING;
> > 
> 
> Yu,
> 
> I think you are talking about the code in sd_init() 
> where SD_PREFER_SIBLING is first set
> to "ON" and updated depending on SD_ASYM_CPUCAPACITY.  The intention
> of the code
> is if there are cpus in the scheduler domain that have differing cpu
> capacities,
> we do not want to do spreading among the child groups in the sched
> domain.
> So the flag is turned off in the child group level and not the parent
> level. But with your above
> change, the parent's flag is turned off, leaving the child level flag
> on. 
> This moves the level where spreading happens (SD_PREFER_SIBLING on) 
> up one level which is undesired (see table below).
> 
> 
Sorry got a bad mail client messing up the table format.  Updated below

			SD_ASYM_CPUCAPACITY	SD_PREFER_SIBLING after init             
						original code 	proposed
SD Level		 	 
root			ON			ON		OFF      (note: SD_PREFER_SIBLING unused at this level)				
first level             ON			OFF		OFF
second level		OFF			OFF		ON
third level             OFF			ON		ON

Tim