Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
Date:   Thu, 14 Sep 2023 18:43:01 +0800
From:   Chen Yu <yu.c.chen@intel.com>
To:     K Prateek Nayak <kprateek.nayak@amd.com>
CC:     Tim Chen <tim.c.chen@intel.com>, Aaron Lu <aaron.lu@intel.com>,
        "Dietmar Eggemann" <dietmar.eggemann@arm.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
        "Daniel Bristot de Oliveira" <bristot@redhat.com>,
        Valentin Schneider <vschneid@redhat.com>,
        "Gautham R . Shenoy" <gautham.shenoy@amd.com>,
        <linux-kernel@vger.kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Juri Lelli <juri.lelli@redhat.com>,
        Ingo Molnar <mingo@redhat.com>
Subject: Re: [RFC PATCH 2/2] sched/fair: skip the cache hot CPU in
 select_idle_cpu()
Message-ID: <ZQLjtV9TbM2KFGsF@chenyu5-mobl2.ccr.corp.intel.com>
References: <cover.1694397335.git.yu.c.chen@intel.com>
 <d49cf5748aa7c6d69580315d2373a9eafa21c21f.1694397335.git.yu.c.chen@intel.com>
 <09a7a5cb-dde5-122e-a086-5802df993433@amd.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <09a7a5cb-dde5-122e-a086-5802df993433@amd.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?C84dSigUo8u7iGnTmP4cFbbrSQKjIlvYrvLDAHWQdCqHoAAB+yksE3Sl8vjF?=
 =?us-ascii?Q?0BzvH5YOB1dfmS+d+2UnjUseIjgABIlC3JInYGpE9ijlb6wI7+Y/owrLZr4W?=
 =?us-ascii?Q?39wagUrvpheN0snVkelBRsNbutTCvMCmqkXBwi60uzFCvDytEYRW3g9fpUgj?=
 =?us-ascii?Q?/koYIsqq4RAan+BIfPf9R5GzQbCGMIbCk5XXMVY5WQz1t7q/6jUsQ10cBItc?=
 =?us-ascii?Q?AQvKsopJ4i1j7+7RAN+G/MetBnIhJ/bbRblKivq7ULE8VgHKvtChlVw0gKaR?=
 =?us-ascii?Q?IfOZCDS3y8nTrZfmE0KyDABZURHAAaMDFYVlpHxGTr7XU+Hy1xFHQBMYEzVK?=
 =?us-ascii?Q?Vf2DzMN1Hg3pdLXQoLMCkOUxFIiFSDfuXpDJ9ckV1dXqbsxxRVEc0Uwptyw+?=
 =?us-ascii?Q?jqCiN4dbJmjFi9zbCD6c2ploBP4GKh8SW51aMhSpizSbr/c1xgbqiLZAtKZ9?=
 =?us-ascii?Q?RoUg89h847JpfuB/mFrdnTlnVZhsmSMA1lfdKktwcWdanBzK3/Ke9bM34Vu9?=
 =?us-ascii?Q?kAwJEbs9kYw+4JA1A1DYK1LcymlxHSTESBIc+lTXW0U/bHxlCd8L4xhyePgD?=
 =?us-ascii?Q?/I4INl4K5iPEDOiFDmUGTgoiyz2TYDC3ZNl+FrINsN10yZXJvOKUsxD3q4bs?=
 =?us-ascii?Q?twhVFXiMfdawmH/YzZSbapHLOLKjB49LKfYAwosEB4o/Lq+XfdApUByfM90/?=
 =?us-ascii?Q?WUBHZKvrPSd0vK1y41nWfRMwuXCwgFJkRUQJgs8Gckk6Kwajsqm13vLETZYv?=
 =?us-ascii?Q?m15wmJNZ4LVwUNDFM+rSx/Qw69KOUqsZsUr1yQRQMQgbcJHfZMBLAh2RjBw5?=
 =?us-ascii?Q?w98DvWwrOG0Pza1GY7XXLyd+NyWvO0szZ4yahfHX87x95z6LbdO0Rx7u0GA4?=
 =?us-ascii?Q?5Hb3i+WpUWSQE5dkGNzYDz7kRroSjgm+dC1gN9yYNV90UmBXvFItLSs0Q2sp?=
 =?us-ascii?Q?x/ftC//QJ1yoPUu4SO6+TcOPfdi+ZWFcfZZdU9XYQnzc4sljNlVlcz1BYOKz?=
 =?us-ascii?Q?L4JvvJYhtyJhiuGsUORJ7AHH6elamLz5c4+GeoZLVART/1Xvde2vQEsnsrWe?=
 =?us-ascii?Q?cwz2jGafan9G7KRirEkBngpNR3+c0drc6vrIivAtqookF+jc0PGnA7Dl+EJ2?=
 =?us-ascii?Q?m7ceN/K1f1Z3rkvN03zSxD+kxuMVwJahdZT3pU9Q6oEU7E1kLw1/nED9BqXs?=
 =?us-ascii?Q?5dqMssOt49U2xXseqUhkjG8MyBDyOrBS8upumethK+2lkKqvN9pweUY4/1JQ?=
 =?us-ascii?Q?zdPYHHt+zjqNVNQGbBp22Gcmj16JBDSmRaMz7lQkuZb53PhAXZJIi4il0qSz?=
 =?us-ascii?Q?HQc93OJR+Q+tILbXg4kizlRV03I9xMl/KCU5ZE2d2XvQDTM5AdrcgJUd2k+O?=
 =?us-ascii?Q?YvNBUMWt2IVTrrWFM4BMbsCYXJ2j/hDp+HZEAxquGonMI4yYfQK0Ssyg8/Ru?=
 =?us-ascii?Q?S6B3y47zpbcsfeTxeHX+qscoFfw6F1PspHZ375vXYiq3PCjWoNhbOi6pw6aa?=
 =?us-ascii?Q?ZArCDxhPlHdRsoDZjcvhL70w7CW1lyxEvZqIDUN8XRtziWm37XwMuux5+1l2?=
 =?us-ascii?Q?A1CaNq5K1IuJBY06C9Db4g/rG90aCdhcHqnMVKZb?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 52ae9ff5-9b34-414a-b4fc-08dbb50f6873
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6206.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Sep 2023 10:43:18.5934
 (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: 5NTcUxdajJhbOSUqrNkvokZB3R+XN5wEhaBv2N3Vcw53uwpM+daEfYJD8+2nna7bryU+zCbFOFrWXpS4b3bx8g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR11MB5910
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Thu, 14 Sep 2023 03:43:43 -0700 (PDT)
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email

Hi Prateek,

On 2023-09-14 at 11:00:02 +0530, K Prateek Nayak wrote:
> Hello Chenyu,
> 
> One question ...
> 
> On 9/11/2023 8:20 AM, Chen Yu wrote:
> > [..snip..]
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index e20f50726ab8..fe3b760c9654 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > [..more snip..]
> > @@ -7052,10 +7072,14 @@ static int select_idle_core(struct task_struct *p, int core, struct cpumask *cpu
> >  	int cpu;
> >  
> >  	for_each_cpu(cpu, cpu_smt_mask(core)) {
> > -		if (!available_idle_cpu(cpu)) {
> > +		bool cache_hot = sched_feat(SIS_CACHE) ?
> > +			sched_clock_cpu(cpu) < cpu_rq(cpu)->cache_hot_timeout : false;
> > +
> > +		if (!available_idle_cpu(cpu) || cache_hot) {
> >  			idle = false;
> >  			if (*idle_cpu == -1) {
> > -				if (sched_idle_cpu(cpu) && cpumask_test_cpu(cpu, p->cpus_ptr)) {
> > +				if (sched_idle_cpu(cpu) && cpumask_test_cpu(cpu, p->cpus_ptr) &&
> > +				    !cache_hot) {
> 
> Here, the CPU is running a SCHED_IDLE task ...
> 
> >  					*idle_cpu = cpu;
> >  					break;
> >  				}
> 
> ... but just below this, there are following lines to cache the idle_cpu:
> 
> 		}
> 		if (*idle_cpu == -1 && cpumask_test_cpu(cpu, p->cpus_ptr))
> 			*idle_cpu = cpu;
> 
> Would it make sense to also add the same "cache_hot" check here when we
> come across an idle CPU during the search for an idle core? Something
> like:
> 
> -		if (*idle_cpu == -1 && cpumask_test_cpu(cpu, p->cpus_ptr))

When we reached above code, the following condition should be true:
 (available_idle_cpu(cpu) && !cache_hot)
because the previous 'if' statement is false. So I guess we already
has !cache_hot ?

> +		if (*idle_cpu == -1 && !cache_hot && cpumask_test_cpu(cpu, p->cpus_ptr))
> 			*idle_cpu = cpu;
> 
> Implications with the above change:
> 
> If the entire core is idle, "select_idle_core()" will return the core
> and the search will bail out in "select_idle_cpu()". Otherwise, the
> cache-hot idle CPUs encountered during the search for idle core will be
> ignored now and if "idle_cpu" is not -1, it contains an idle CPU that is
> not cache-hot.
> 
> Thoughts?
> 

Yes, agree, we want to skip the cache-hot idle CPU if that entire core is not idle
in your case.

thanks,
Chenyu