From:   "Chen, Tim C" <tim.c.chen@intel.com>
To:     "Chen, Yu C" <yu.c.chen@intel.com>,
        Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
CC:     "Peter Zijlstra (Intel)" <peterz@infradead.org>,
        Juri Lelli <juri.lelli@redhat.com>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        "Neri, Ricardo" <ricardo.neri@intel.com>,
        "Shankar, Ravi V" <ravi.v.shankar@intel.com>,
        Ben Segall <bsegall@google.com>,
        "Daniel Bristot de Oliveira" <bristot@redhat.com>,
        Dietmar Eggemann <dietmar.eggemann@arm.com>,
        "Brown, Len" <len.brown@intel.com>, Mel Gorman <mgorman@suse.de>,
        "Wysocki, Rafael J" <rafael.j.wysocki@intel.com>,
        Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Tim Chen <tim.c.chen@linux.intel.com>,
        "Valentin Schneider" <vschneid@redhat.com>,
        Ionela Voinescu <ionela.voinescu@arm.com>,
        "x86@kernel.org" <x86@kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v3 06/10] sched/fair: Use the prefer_sibling flag of the
 current sched domain
Thread-Topic: [PATCH v3 06/10] sched/fair: Use the prefer_sibling flag of the
 current sched domain
Thread-Index: AQHZOq/DtsWPHSzMLEmo6OdAAD06ha7GnDyAgABmInA=
Date:   Thu, 9 Feb 2023 20:00:19 +0000
Message-ID: <DM6PR11MB4107F95164772B40B92F66A6DCD99@DM6PR11MB4107.namprd11.prod.outlook.com>
References: <20230207045838.11243-1-ricardo.neri-calderon@linux.intel.com>
 <20230207045838.11243-7-ricardo.neri-calderon@linux.intel.com>
 <Y+TyUZg86LYhtWeJ@chenyu5-mobl1>
In-Reply-To: <Y+TyUZg86LYhtWeJ@chenyu5-mobl1>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?XS6q0Bo/IcCWuXuPCkaaK9hLj/GaaGhoGJj0fSQ5YNKibpM2JgskfKL/CUtX?=
 =?us-ascii?Q?MzlcL/Szb089oiUZtrX7VwH1V+NlG41zzAWYE05zmg9qoq7gC9/+GLENZnPl?=
 =?us-ascii?Q?k0XXkmg1wDU0HVhj7+LVE3rEHP6NKOZCXcsfxzenRKIaYxvTuA/1geuIBkqi?=
 =?us-ascii?Q?Y4IMCytZlfDlhCpRC4frYnfNKa7e5D1caiqsvxaslEpVlJt6N+ifSZRnsxps?=
 =?us-ascii?Q?QobGHiGBbVG0Cka0p/17YTmH0dZenf8nH1M9n5qyew/V+NoCjgMYdeQ7i0Cf?=
 =?us-ascii?Q?4fm1qo8LaOCn7lRMwaVymKIaFRIV08aWj5VZCUDdjnSc4LXiMvEbqNBDPOox?=
 =?us-ascii?Q?KYb9jRw8CS6YYTpSxnUjHRhgJD8asXfkQM13fPMjmQ62APuU9o3kVcKzfVTZ?=
 =?us-ascii?Q?PBRmvH05KNm/FqQm3nenJ1cdvvlIwrKCxfSRH2zyiYnUpJ4lUKRSoZ+ODSEc?=
 =?us-ascii?Q?D0tb6rQZpJ4IAIxwSxVdIy/3TaoftiLsDdjSljhPo72Mu2Nph22hVPLtNt/S?=
 =?us-ascii?Q?IXXDliVPFZKV9oQ383AN63/RgUImV8bkJ/MllSKWhveinLbw+88iFfP58mkD?=
 =?us-ascii?Q?cHkEumHg6v4NRfweQCt1w7XQScyG6tkG7Ur/TPMTmR/fhbEzRmgxDNnepIKa?=
 =?us-ascii?Q?PlMPqaOaFOUYAtolz540xxlGAv0Ay+snbs+t0vF3WQFt0uI2NKxUgxhjJ1dP?=
 =?us-ascii?Q?Qt7ixInyNY0R4pcCPefgoWGBltMga7spXw/y93OVKCzl3bOIMckDUyOL/t2s?=
 =?us-ascii?Q?bkP9wfkTxzGRVqtgJlRjcVFNyMzKQUhJLkhYVaNMepYb8QAuz2VjZ+V+kPG3?=
 =?us-ascii?Q?1jAkX2fPSiqh0a0YOjX0S/naLwtWUx67Vv0ElvqJ4+l+37UH0sS0xb4Uptt1?=
 =?us-ascii?Q?EgqwKM6C1f7egEcxVc+HtCnLInB724067vukXcmtvssrSP1cvBwyKAQU9iFk?=
 =?us-ascii?Q?2lxYyDxkhobkv4SWroQk3LKzNDzrJSKrs0OS8GSWqDAEQNtE5e1pGKP8JK3t?=
 =?us-ascii?Q?MGdnnxFWJPWNpbwHDQwfWU2Zde+rNpeKelhrfYS9vxmAU9eJvMltoLkJBrQ1?=
 =?us-ascii?Q?PwfdJbXsyjtSmcWMD/opWoUj8dGapaoeJsdAqWx3T60JdvR1cUvU6JVuBLqo?=
 =?us-ascii?Q?K/FvxwtpRx41NnHHUceE97ZqiZhWRlMF4KovtMKpSTaFwlu2ytgpJnnstPab?=
 =?us-ascii?Q?fZXQv6wD+2TAH0fpTWWjruuQrPK0yINHaR0QKX94u5LwHak/t+4Zx5pKWqHz?=
 =?us-ascii?Q?vuAQgWV9AuRS0I0ztDyXBjBjr3dXz3m6Tw5iP0KmafTJtyaAikwrrT0S2I9i?=
 =?us-ascii?Q?9mT9GedahP5ITBQnQfLzOlkaUtcOsGW9KN7DqcaL1VHNZ4erhHhnEgfIaFuT?=
 =?us-ascii?Q?+YNMl/PHUKgFnu98FYe+glbWssXZIFRynFdWWzk+Enber8upSlmcQi6Cvdpq?=
 =?us-ascii?Q?EjW8uHjV6S3lcVtQ4D8dxQ4C/eY9B5yPfdGsE71Trh46+dOZ2VY2lTUXb6nc?=
 =?us-ascii?Q?r5JuO6eHXnP/dnwaIRNvv1+mmw3MVjq1yvYOpkTti4Z6YuVM1waWaef0NbQE?=
 =?us-ascii?Q?ZsJ8SR/usNepDPrlCu1mhYPxvMdJlSSrdr/u1L6K?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB4107.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c8cf5345-a0a1-4414-1069-08db0ad8456f
X-MS-Exchange-CrossTenant-originalarrivaltime: 09 Feb 2023 20:00:19.6864
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: FYUvfhuZ1+SlmTq+XPTCBg2vJJFPVI5Hp1IRO3Izeq/YoxK7PEv+RmoGvlpJPT6nVkKGBZIPDxoB57hMCdW4tw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB6651
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

>>  static inline void update_sd_lb_stats(struct lb_env *env, struct
>> sd_lb_stats *sds)  {
>> -	struct sched_domain *child =3D env->sd->child;
>>  	struct sched_group *sg =3D env->sd->groups;
>>  	struct sg_lb_stats *local =3D &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 =3D sg->next;
>>  	} while (sg !=3D env->sd->groups);
>>
>> -	/* Tag domain that child domain prefers tasks go to siblings first */
>> -	sds->prefer_sibling =3D child && child->flags & SD_PREFER_SIBLING;
>> -
>> +	/*
>> +	 * Tag domain that @env::sd prefers to spread excess tasks among
>> +	 * sibling sched groups.
>> +	 */
>> +	sds->prefer_sibling =3D env->sd->flags & SD_PREFER_SIBLING;
>>
>This does help fix the issue that non-SMT core fails to pull task from bus=
y 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 &=3D ~SD_PREFER_SIBLING; might be:
>        if ((sd->flags & SD_ASYM_CPUCAPACITY))
>                sd->flags &=3D ~SD_PREFER_SIBLING;
>

Yu,

I think you are talking about the code in sd_init()=20
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 capaci=
ties,
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.=20
This moves the level where spreading happens (SD_PREFER_SIBLING on)=20
up one level which is undesired (see table below).

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

Tim