Date:   Thu, 9 Feb 2023 21:17:05 +0800
From:   Chen Yu <yu.c.chen@intel.com>
To:     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>,
        Ricardo Neri <ricardo.neri@intel.com>,
        "Ravi V. Shankar" <ravi.v.shankar@intel.com>,
        Ben Segall <bsegall@google.com>,
        "Daniel Bristot de Oliveira" <bristot@redhat.com>,
        Dietmar Eggemann <dietmar.eggemann@arm.com>,
        Len Brown <len.brown@intel.com>, Mel Gorman <mgorman@suse.de>,
        "Rafael J. Wysocki" <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>,
        <linux-kernel@vger.kernel.org>,
        "Tim C . Chen" <tim.c.chen@intel.com>
Subject: Re: [PATCH v3 06/10] sched/fair: Use the prefer_sibling flag of the
 current sched domain
Message-ID: <Y+TyUZg86LYhtWeJ@chenyu5-mobl1>
References: <20230207045838.11243-1-ricardo.neri-calderon@linux.intel.com>
 <20230207045838.11243-7-ricardo.neri-calderon@linux.intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230207045838.11243-7-ricardo.neri-calderon@linux.intel.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?2c1ginA8ZI8P/wof/ewa8h6SBBWBBgXW4+FZTSc8bXAaIAFH9fVVXAuMXRK/?=
 =?us-ascii?Q?sHc4WucT2weFzCYZ8eYXCr3IBSJUuWKp0fnCgnPnTzn4r4ku0bY39omxdvv5?=
 =?us-ascii?Q?C0rejmrNBpt09Ww7UO49036EtoQEVKMwIhf6wT2LzQOsSh863LMw9jm4Apho?=
 =?us-ascii?Q?dbPUPi11P35Cm1vVcX1kxn7UWkNCsUXKfmPsV3t4Zxqy02t1pWZfHSymddav?=
 =?us-ascii?Q?adNDoVoCSQfkbTpi6LoGLiONd3AKckk6Ya3LJEXTaAeZovIL3UoGxOMCqr9l?=
 =?us-ascii?Q?rESxoeQWPenHo9k3h8iCnL/cChnfTx05wmD/aluFlbgENaP+UtEStfjA1jru?=
 =?us-ascii?Q?gh7DlFqMIg3O0RMcZMmASNq3PS51rXa7gTWGilFLBDEZVa2yD0pECc4tS6wV?=
 =?us-ascii?Q?mZ8tB3kAn9FDLipAaHJL9KP7/YaSAz/z6BpAivCC677ZyN+GM4m9dsYFhsAO?=
 =?us-ascii?Q?hKqVwL2jDHxM+PuL1B/snji6dd2WiDR+7b6LchvrrvFtnoY5jb1LZ2wxslKe?=
 =?us-ascii?Q?ez1DvZJoGyz5OZEiojLYGSJPZA3MqvA/V+QiGFzWTiv++iCbBEjQvx6hUVT5?=
 =?us-ascii?Q?XjnV1HApNuxig7Tkl1VlhP+5GvuR2miBNoa790S+WyNTFhc6bMk/W1rTQnf/?=
 =?us-ascii?Q?me2LziYVhtuom/3A6PLj1G1ZhTOK9FKpruA3udJxBm/eWjWQqPMofXf2ee0O?=
 =?us-ascii?Q?BW+p+ur+qdul39WwswZeEZsosKXN8ZsPdNjHSJjUYc6kiGCU3oNBIjtjEHUB?=
 =?us-ascii?Q?cefMf5SaG916ekTYBTTMZ+L8StPHC8K6JMw+4n4tUd0CXfe/E5IBSR7NbFqM?=
 =?us-ascii?Q?TSxMCh2c36UmciyvqQjj2j0AwSkL3h4iUUS6ZtR6771vjMBubyqoa6bqTNBc?=
 =?us-ascii?Q?ctANIF8t9bCShsJvnmBFxOdBlZ/wwWY5z3J3SpC98IWYT9hpkVfMhsLtls6c?=
 =?us-ascii?Q?XLB8WXgWG3nSD4hKJINIs0+WRlxadP4389QUWm6rCdTc0UI1PEgEHEMtzO77?=
 =?us-ascii?Q?X4ixA9K56b6/vjei4Ckc5PUZVZBMelwdxqcGyzBKGA1rgI7ovmHe3HnP6k93?=
 =?us-ascii?Q?Ezv5j3ysXdAPzdXvMQZIeJRqa2hCjFDmVIxzpjdRzOQgjkHzjfQZOzbXR7tk?=
 =?us-ascii?Q?cjuq13GbrCnydGnEXtgbXw3tsOQZVVtWrmSR1Fg3n+kACsA6wGyc1pC/MWu0?=
 =?us-ascii?Q?6QyikV8jT01coQD1wyXkZJ9dwD9npenm1/TO28KA8C5TCnHb4Si9fMwMsrZ1?=
 =?us-ascii?Q?MfJ8VAnSSdiX+JsLd6ThOFNeuwPzv4jAWGsNkdDEaIAVOdJgCErEnw6g6JCY?=
 =?us-ascii?Q?r1JlwUjGN5L2R/uWbKwk2ScOE3sotGlhmV1ZCvPTtDCUTjMnYHkWBP1iMqKM?=
 =?us-ascii?Q?xB5HbqBmLcUtY1e9tdqwkO21l6+liWp6DBIOKRS5gaBouQmF8ES3A/qQWaI2?=
 =?us-ascii?Q?VovvfAm8w6vYhOJlh6n1kaqiNevagB6FTSeZJoicSj9Pih6yNJre6TNa+Ch7?=
 =?us-ascii?Q?vO3lp9B4h9JpdRZFWSNRwOulrjAxmfaaTVDzGaCTpNYEGRKsapKDS8k8LXw8?=
 =?us-ascii?Q?jORlceAQcd2nZDN0oh2y0G06emYZSW+MNvoAH3nm?=
X-MS-Exchange-CrossTenant-Network-Message-Id: eb5fa6c0-bdad-4c94-d959-08db0a9ffc2a
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6206.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Feb 2023 13:17:25.1988
 (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: TxD0OuveyPihvrOI6e5FjILbcQ3z8D2+6oI9nUetfMNGm9oSeAVldNOEgUSGswKh0FYL5whCdXKeRoR/sE82rQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR11MB6062
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2023-02-06 at 20:58:34 -0800, Ricardo Neri wrote:
> SD_PREFER_SIBLING is set from the SMT scheduling domain up to the first
> non-NUMA domain (the exception is systems with SD_ASYM_CPUCAPACITY).
> 
> Above the SMT sched domain, all domains have a child. The SD_PREFER_
> SIBLING is honored always regardless of the scheduling domain at which the
> load balance takes place.
> 
> There are cases, however, in which the busiest CPU's sched domain has
> child but the destination CPU's does not. Consider, for instance a non-SMT
> core (or an SMT core with only one online sibling) doing load balance with
> an SMT core at the MC level. SD_PREFER_SIBLING will not be honored. We are
> left with a fully busy SMT core and an idle non-SMT core.
> 
> Avoid inconsistent behavior. Use the prefer_sibling behavior at the current
> scheduling domain, not its child.
> 
> The NUMA sched domain does not have the SD_PREFER_SIBLING flag. Thus, we
> will not spread load among NUMA sched groups, as desired.
> 
> Cc: Ben Segall <bsegall@google.com>
> Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
> Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
> Cc: Len Brown <len.brown@intel.com>
> Cc: Mel Gorman <mgorman@suse.de>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Tim C. Chen <tim.c.chen@intel.com>
> Cc: Valentin Schneider <vschneid@redhat.com>
> Cc: x86@kernel.org
> Cc: linux-kernel@vger.kernel.org
> Suggested-by: Valentin Schneider <vschneid@redhat.com>
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> ---
> Changes since v2:
>  * Introduced this patch.
> 
> Changes since v1:
>  * N/A
> ---
>  kernel/sched/fair.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index df7bcbf634a8..a37ad59f20ea 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -10004,7 +10004,6 @@ static void update_idle_cpu_scan(struct lb_env *env,
>  
>  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;

thanks,
Chenyu

>