Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
Date:   Wed, 13 Sep 2023 11:52:14 +0530
From:   "Gautham R. Shenoy" <gautham.shenoy@amd.com>
To:     Chen Yu <yu.c.chen@intel.com>
Cc:     Aaron Lu <aaron.lu@intel.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
        Ingo Molnar <mingo@redhat.com>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Juri Lelli <juri.lelli@redhat.com>,
        Tim Chen <tim.c.chen@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>,
        K Prateek Nayak <kprateek.nayak@amd.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 2/2] sched/fair: skip the cache hot CPU in
 select_idle_cpu()
Message-ID: <ZQFVFv4GcPOU+X7N@BLR-5CG11610CF.amd.com>
References: <cover.1694397335.git.yu.c.chen@intel.com>
 <d49cf5748aa7c6d69580315d2373a9eafa21c21f.1694397335.git.yu.c.chen@intel.com>
 <20230911072629.GA298322@ziqianlu-dell>
 <ZP7SYu+gxlc/YjHu@chenyu5-mobl2>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZP7SYu+gxlc/YjHu@chenyu5-mobl2>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?nI4+jJqGHYtJVEXY4MUNjr01O6TbyumXbYtZhqPfQqTcLZlvTlsfxdzS9eEb?=
 =?us-ascii?Q?BvYi8fK5b93PBZffx+2zL2Zicw9easEi0lUZgzA71LW1MK0C8nzR7OAYh3wm?=
 =?us-ascii?Q?ryYe1lBFCC5GyyFK4aIkkS9PnqWdeEwIh3dq5barxnfRtVdUr6uhHTmlQRGL?=
 =?us-ascii?Q?dIbE6wrhlcgWNgyv2LKtn1799B76nmlzwk3Cnm078J3e4R1/KIJG32Yjkisn?=
 =?us-ascii?Q?EbVvY8HJN0jAV1lFywpAscgFHZ/Oa9ahRgEQrBTTNoxyWRHiDB/w3X6M6EV7?=
 =?us-ascii?Q?k5ALQGxTTWESBSr+RX5wjcXEYKa9/t/qx0vlbk7EB3vU0kQX1R2kEEIRGF7R?=
 =?us-ascii?Q?fx27j6Mfjp/ojIk5HqTPVIFEdqxpDQt/S55lVIViJo9vYr5T5NiSTUrRvWgw?=
 =?us-ascii?Q?ERCPvocfsXaZn1vhep9nhiU6InMrL3C++LdtPCiXqQ5tIq8FY7yOCnbuh6Kr?=
 =?us-ascii?Q?SFNSDGevy/ELU6cX8aV9HNgNIln0b8uJhoMLwKZYJfbWCtI1EvOahvpCwvPD?=
 =?us-ascii?Q?VNIwT9yPxj3htMOFgrDgJwmetwpsqXjIL83bh/EPenoXyokR3DcB9WcnRH7z?=
 =?us-ascii?Q?NEbY02x4YbLb6h3Rljj8hc/b/rYAOApZoAoSrmnf1pPTPVIXkMH1ATwTrING?=
 =?us-ascii?Q?f3zixzmfmz4SEjlsNkx3lROOS29HvnT4Pxic6Fwkha4rhOWJMnXwDW6G9S1L?=
 =?us-ascii?Q?SuE/HVss9Ajui5UwtJ8oxwzRQwNYZEU4wiq6I1V3T3fHkJxl/4xkR5s8bFxb?=
 =?us-ascii?Q?MklxYd3EHGKRLe0m7ewjwl6ChElu2brSNMkmCNTLkhFl3eB45CAv73tkIYYy?=
 =?us-ascii?Q?iPWTP8N5Ho09MN+8O/I7gUfcvBjvhcrLiBt4XXxlMjsH3IDSh+/x5i75AjGr?=
 =?us-ascii?Q?ywhkEDxy+Ur5srWE10X8xeULAWmeCMKCihf+Od6XsyvoFo07fzjMDngCR4fv?=
 =?us-ascii?Q?QlVVZ5YFtzw92rrW9VR3xYtrvyh7Bz7/jtbsplfhh+uhwM8waFnV/pCKduGp?=
 =?us-ascii?Q?/rMVW/Di+gtcn88xHcaZ8MecACBVpU59m5vkUOZJX+sch2ijnoiZ+CqG/tUA?=
 =?us-ascii?Q?EALSBPyxm8V+IOjRnUiniyEwAbZ2Mn9VkyahAbMl6eXb5CxSAU5LZRWZ2oP0?=
 =?us-ascii?Q?eKTwtZt2ryxFlP59GIv53686lsLymROHkL38IIzZZniKYF4PVm2QJu1RdEtr?=
 =?us-ascii?Q?vjkrGUgRmAwvb7lqF/mqMVzYWAzVCzdUIhd8va2yNkWVkcOg/hPlTbiwT4ro?=
 =?us-ascii?Q?fK1hMF3d+oPjeUlCr2mAU1GW7YdOR33KPG1vD2XZuXU4FFxGY1kx7Dz0NWgA?=
 =?us-ascii?Q?8mPwLYgvgHb8Xd9U2QlRM9hz3uVKWdhNOlOPnYEIXbd6qgqZ4JRbulD2RlXS?=
 =?us-ascii?Q?/8tjpWixA+ibaHAPrpaYqiofr02/sjFgKbTOV8ylQAgQep/Jc8GgAZutEcAX?=
 =?us-ascii?Q?v9JQtgyjlTE9MkxqB96WK08ob607uJsnGV7NNg1H3hCKq0TWf38QDjA+NxKY?=
 =?us-ascii?Q?KxSWI7xlQLCkbCdEyA4XitmHhe/mUVDMsUveuKwK3ju0wyioIqRnNuXaQxzs?=
 =?us-ascii?Q?V3dAae/OlFXK+1LaZo1ECrRandioJMZ1UlE5EJQw?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ceadd5ff-3a5d-4513-c617-08dbb421cd65
X-MS-Exchange-CrossTenant-AuthSource: DS7PR12MB8252.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Sep 2023 06:22:27.9948
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: L18u348KgOUukrbnvOfNoEK5NkUAwYqV8tXhiAaJcZUweouDgJuWpAQKqJmxZ0Zirs2ifilNbQ1ggU7axNZS6g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR12MB5677
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]); Tue, 12 Sep 2023 23:22:37 -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

On Mon, Sep 11, 2023 at 04:40:02PM +0800, Chen Yu wrote:
> Hi Aaron,
> 
> thanks for the review,
> 
> On 2023-09-11 at 15:26:29 +0800, Aaron Lu wrote:

[..snip..]

> > > @@ -6982,8 +6997,13 @@ static inline int find_idlest_cpu(struct sched_domain *sd, struct task_struct *p
> > >  static inline int __select_idle_cpu(int cpu, struct task_struct *p)
> > >  {
> > >  	if ((available_idle_cpu(cpu) || sched_idle_cpu(cpu)) &&
> > > -	    sched_cpu_cookie_match(cpu_rq(cpu), p))
> > > +	    sched_cpu_cookie_match(cpu_rq(cpu), p)) {
> > > +		if (sched_feat(SIS_CACHE) &&
> > > +		    sched_clock_cpu(cpu) < cpu_rq(cpu)->cache_hot_timeout)
> > > +			return -1;
> > > +
> > 
> > Maybe introduce a new function that also considers rq->cache_hot_timeout,
> > like available_idle_cpu_migrate() so that above and below logic can be
> > simplified a bit?
> > 
> 
> Yes, that would be simpler, I'll do in next version.
> 
> > I was thinking to simply add that rq->cache_hot_timeout check to
> > available_idle_cpu() but then a long sleeping task could be forced to
> > migrate if its prev_cpu happens to just deschedule a task that sets rq's
> > cache_hot_timeout. I guess that's why you chose to only change the idle
> > semantic in select_idle_cpu() but not in select_idle_sibling()?
> >
> 
> Yes, sort of. And the reason I did not put this cache hot check in available_idle_cpu()
> or idle_cpu() was mainly because these APIs are generic and could be invoked by select_idle_sibling().
> If the task fall asleep and woken up quickly, its previous idle CPU will also be skipped,
> thus no one could use this CPU within the cache hot period, including the cache-hot task
> itself.

This happens even with this patch right? It is possible for a task p1
whose avg sleep time is "t" to go to sleep which causes its CPU to go
idle. When it wakes up after a time t' < t, the logic above skips the
idle CPU because it is still cache-hot, despite the fact that it is
cache hot for p1!

Have you considered recording p1's identity in the
rq->cache_hot_sleeper so that in select_task_rq_fair(), we can simply
return the previous CPU if it is cache hot and the wakee is
rq->cache_hot_sleeper, thus avoiding the whole select_idle_sibling
scan.

> 
> thanks,
> Chenyu

--
Thanks and Regards
gautham.