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: <DM6PR11MB4107F95164772B40B92F66A6DCD99@DM6PR11MB4107.namprd11.prod.outlook.com>
Date:   Thu, 9 Feb 2023 20:00:19 +0000
From:   "Chen, Tim C" <tim.c.chen@...el.com>
To:     "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>,
        Tim Chen <tim.c.chen@...ux.intel.com>,
        "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

>>  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).

							SD_PREFER_SIBLING after init             
							original code	proposed
SD Level		 SD_ASYM_CPUCAPACITY	 
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