Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Date:   Wed, 13 Sep 2023 10:57:20 +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>,
        Ingo Molnar <mingo@redhat.com>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Juri Lelli <juri.lelli@redhat.com>
Subject: Re: [RFC PATCH 2/2] sched/fair: skip the cache hot CPU in
 select_idle_cpu()
Message-ID: <ZQElEM5GAn/Vq0tM@chenyu5-mobl2.ccr.corp.intel.com>
References: <cover.1694397335.git.yu.c.chen@intel.com>
 <d49cf5748aa7c6d69580315d2373a9eafa21c21f.1694397335.git.yu.c.chen@intel.com>
 <31c910c2-717d-b86c-4e08-5c94383682e8@amd.com>
 <ZP7ptt70uF10wxlg@chenyu5-mobl2>
 <5b35ae2b-a401-2489-4b3a-a03e8284d138@amd.com>
 <ZQBaWRCmMOvIGmem@chenyu5-mobl2.ccr.corp.intel.com>
 <229069c1-3d61-53bb-fff7-691942c48d21@amd.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <229069c1-3d61-53bb-fff7-691942c48d21@amd.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?TibLvM6S/ZgmT7tp7j7BgH4DCkYgSnex2fYQj2ouEs1c2MwB4hLDc4Vv9UMA?=
 =?us-ascii?Q?b1+5KT/fINniEmtWFam7fxU9N0YBq3OuJ+R/QNLdWNV8Qh4Le7VsJZaAJaIw?=
 =?us-ascii?Q?Vd/jROO1hG9/RkC9GTGVpHyZOzcUQVHq15iJywWFobesKJU8C1iWQm2FEEAb?=
 =?us-ascii?Q?Cl/DKzi6j6os1fkNP+mfD873rb7R/XUtW3IfZgRm1Ql1/JjQhlvSV0yAcPkz?=
 =?us-ascii?Q?L6IjjuBGcK0GyEhoB0yWT9822LY6AdqPe63mOJGwMQ9Sm5Ekkh+HiRPsEMWD?=
 =?us-ascii?Q?gGy1cMQauGYey7tE3YPQFQE70IIEkgWoiXS8J/zVxFfYOV4VTu+psajAz2AB?=
 =?us-ascii?Q?enUGQoi8nxeLhXWqoW1hCyCSPmW8LvArZ5q7+pAUisQxAy07x8nxv6Zj0oZm?=
 =?us-ascii?Q?SwAjpcCiNgL+ORUW2TUEmaNRIuDPNCfE8vMYQG1es6gIRexwO5/OOLMo/A5y?=
 =?us-ascii?Q?nI+zofEtkstnkfgJZXD2AiRYUbtzV6aU8+4Y+8ya5oTdZK9c6YYaZvMDAGQX?=
 =?us-ascii?Q?1mcqAb78fI7vGVFR9iDq0HT65BnmzYX8QurnEk5/DzNplT7PZZkCCl9fQnn9?=
 =?us-ascii?Q?Ybp7s7sjeoKy9fjSXMiXNSwmX8NnuYmSC2/XGDm6RKxCulTCKv4krOV/KQ+T?=
 =?us-ascii?Q?wU67vEF6lomzhtJNeZc3KhghHkgru8LrcqROx0wj+k54MZszl984q8LctDAc?=
 =?us-ascii?Q?dReqRU6UQLBui1/g/NpX/4CCe07MalLto0SwrRTdILoGy2MPoS4UYuNfJGjp?=
 =?us-ascii?Q?L/374t8dvCWEVMjcSrP1MCS0/L5s6lWIETxb5BwED94pM5CYcAKSTz9ZrAze?=
 =?us-ascii?Q?jB78LCBAUo4pCx9JHVkR+txNf5NOvtVaeWdZ8LnzsPlnUJOoW8J2vqtX8JIn?=
 =?us-ascii?Q?aiAzMbs8ofZWQY2LY2/qB8yZMw/v3NJBujZ+T1RZuQaWk9fIaFkecrmr23xU?=
 =?us-ascii?Q?+bkBCoEpMmJ8jBSp2dPZwdJJBad4kX5k0idyDI0yM4i4w3PEW6LdWfW7Wg4c?=
 =?us-ascii?Q?c16qosh97Jevi6cJnW0DvL/CYRCKpZUg4hewFKVdboEalJwH3eIYhqeHWrwE?=
 =?us-ascii?Q?/L6jK2DGlWi82dVqShkPo2CFmrantlRWaAYqampuH9wFezsD/dQMjCR671Pu?=
 =?us-ascii?Q?vo+nt/7wRv0WtonLWePnQVghtTVMxwv7zhBT2abAryVtL3hVZmgDBTgVCJIF?=
 =?us-ascii?Q?ZhPXgfOXDMA5hHMxGk5V3Gd6YTfraF7n8i7ze545mgh+Flhp2cei57PAEExM?=
 =?us-ascii?Q?OEvKotgqE4NHv+17Y7fN2ZoWLK2AZHsuWxmoDoSp+3NLeAUGkRLbTSbZG/5t?=
 =?us-ascii?Q?NtkI9Pajv9gS7UVImJXY7AogUDbahB+7bD/JsmGaNReaMfDNmQRSXGrYSNLh?=
 =?us-ascii?Q?q1KcpP9ONQigYppYuhowoeqkH4D4v0TjWZIZP25nkHlOlUiHWCiD4xH03xVV?=
 =?us-ascii?Q?5hegplCdhT5+9HXP1p55Kh4a75UMOOv4yaXaijPEjC8vdRKjgcoBLELZ4Gem?=
 =?us-ascii?Q?iIRHcJnADDL59qgWiERjAU//TJbRTP3AUr5B4i0/YS/R0fGZ77l5fokD6bDf?=
 =?us-ascii?Q?xR7WdtTjpBK98Vd3XZtc9aRjAVDuFOxvCvKhyUYi?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 7e169e91-7f37-4533-eaf6-08dbb4052cf1
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6206.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Sep 2023 02:57:33.0258
 (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: qOZQwUcGKETWpCwLWEJRvAf+q7uBY2eca9BaO1YX9bWRsNLKc2eHyK4KkRQ6izt5OYje3YFFWOOc7NE98O832A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN0PR11MB6301
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 (lipwig.vger.email [0.0.0.0]); Tue, 12 Sep 2023 19:57:52 -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 lipwig.vger.email

On 2023-09-12 at 19:56:37 +0530, K Prateek Nayak wrote:
> Hello Chenyu,
> 
> On 9/12/2023 6:02 PM, Chen Yu wrote:
> > [..snip..]
> >
> >>> If I understand correctly, WF_SYNC is to let the wakee to woken up
> >>> on the waker's CPU, rather than the wakee's previous CPU, because
> >>> the waker goes to sleep after wakeup. SIS_CACHE mainly cares about
> >>> wakee's previous CPU. We can only restrict that other wakee does not
> >>> occupy the previous CPU, but do not enhance the possibility that
> >>> wake_affine_idle() chooses the previous CPU.
> >>
> >> Correct me if I'm wrong here,
> >>
> >> Say a short sleeper, is always woken up using WF_SYNC flag. When the
> >> task is dequeued, we mark the previous  CPU where it ran as "cache-hot"
> >> and restrict any wakeup happening until the "cache_hot_timeout" is
> >> crossed. Let us assume a perfect world where the task wakes up before
> >> the "cache_hot_timeout" expires. Logically this CPU was reserved all
> >> this while for the short sleeper but since the wakeup bears WF_SYNC
> >> flag, the whole reservation is ignored and waker's LLC is explored.
> >>
> > 
> > Ah, I see your point. Do you mean, because the waker has a WF_SYNC, wake_affine_idle()
> > forces the short sleeping wakee to be woken up on waker's CPU rather the
> > wakee's previous CPU, but wakee's previous has been marked as cache hot
> > for nothing?
> 
> Precisely :)
> 
> > 
> >> Should the timeout be cleared if the wakeup decides to not target the
> >> previous CPU? (The default "sysctl_sched_migration_cost" is probably
> >> small enough to curb any side effect that could possibly show here but
> >> if a genuine use-case warrants setting "sysctl_sched_migration_cost" to
> >> a larger value, the wakeup path might be affected where lot of idle
> >> targets are overlooked since the CPUs are marked cache-hot forr longer
> >> duration)
> >>
> >> Let me know what you think.
> >>
> > 
> > This makes sense. In theory the above logic can be added in
> > select_idle_sibling(), if target CPU is chosen rather than
> > the previous CPU, the previous CPU's cache hot flag should be
> > cleared.
> > 
> > But this might bring overhead. Because we need to grab the rq
> > lock and write to other CPU's rq, which could be costly. It
> > seems to be a trade-off of current implementation.
> 
> I agree, it will not be pretty. Maybe the other way is to have a
> history of the type of wakeup the task experiences (similar to
> wakee_flips but for sync and non-syn wakeups) and only reserve
> the CPU if the task wakes up more via non-sync wakeups? Thinking
> out loud here.
>

This looks good to consider the task's attribute, or maybe something
like this:

new_cpu = select_idle_sibling(p, prev_cpu, new_cpu);
if (new_cpu != prev_cpu)
	p->burst_sleep_avg >>= 1;
So the duration of reservation could be shrinked.

> > On the other
> > hand, if the user sets the sysctl_sched_migration_cost to a quite
> > large value:
> > 1. Without SIS_CACHE, there is no task migration.
> 
> But that is in the load balancing path. I think the wakeup path will
> still migrate the task.

OK, I see.

> But I believe there might be very few cases
> where all CPUs are marked cache-hot and the SIS_UTIL will not bail
> out straight away as a result of high utilization. Probably a rare
> scenario.
>

Agree.

thanks,
Chenyu