Date:   Mon, 20 Feb 2023 12:55:13 +0800
From:   Chen Yu <yu.c.chen@intel.com>
To:     Abel Wu <wuyun.abel@bytedance.com>
CC:     Peter Zijlstra <peterz@infradead.org>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Ingo Molnar <mingo@redhat.com>,
        Juri Lelli <juri.lelli@redhat.com>,
        Mel Gorman <mgorman@techsingularity.net>,
        Tim Chen <tim.c.chen@intel.com>,
        Dietmar Eggemann <dietmar.eggemann@arm.com>,
        "Steven Rostedt" <rostedt@goodmis.org>,
        Ben Segall <bsegall@google.com>,
        "K Prateek Nayak" <kprateek.nayak@amd.com>,
        Yicong Yang <yangyicong@hisilicon.com>,
        "Gautham R . Shenoy" <gautham.shenoy@amd.com>,
        Honglei Wang <wanghonglei@didichuxing.com>,
        Len Brown <len.brown@intel.com>,
        Chen Yu <yu.chen.surf@gmail.com>,
        Tianchen Ding <dtcccc@linux.alibaba.com>,
        "Joel Fernandes" <joel@joelfernandes.org>,
        Josh Don <joshdon@google.com>,
        "Hillf Danton" <hdanton@sina.com>, <linux-kernel@vger.kernel.org>,
        kernel test robot <yujie.liu@intel.com>
Subject: Re: [PATCH v5 2/2] sched/fair: Introduce SIS_SHORT to wake up short
 task on current CPU
Message-ID: <Y/L9MajLI3mqchQr@chenyu5-mobl1>
References: <cover.1675361144.git.yu.c.chen@intel.com>
 <1b8af8d99da99a20449288ab4fbba64dc05057ce.1675361144.git.yu.c.chen@intel.com>
 <36ba3b68-5b73-9db0-2247-061627b0d95a@bytedance.com>
 <Y+5Kwxnrj16xRklf@chenyu5-mobl1>
 <3a3220f7-e35b-ee77-649c-8c75dcb26b6c@bytedance.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <3a3220f7-e35b-ee77-649c-8c75dcb26b6c@bytedance.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?QQE0cHcc3CjhO3xQB/vqviajSQATZXmzwTncanDHrmoz+92zcSsoc4dV2Ru4?=
 =?us-ascii?Q?qJ7KtC7er1qTXZk1NLbc8p7G3dyoGmzaWedoKvgCmdG9OaptNPsmmafXOvLO?=
 =?us-ascii?Q?CVhx/HfJf8yn4IwknzE5g9q8ZW6IP489etZpZ6PETqJzisGVvwtH/+dcwLpq?=
 =?us-ascii?Q?nK05Xqz8PnzZgzboXYE0iRvcGyD51OvX52z2xarovZfPC8P6gu9XB8XM7WBQ?=
 =?us-ascii?Q?7/79cdDHawcXGFaQVVhueQ8cgJl0BVQAYTq68CprMIuG3xasHqsYp4o8nkva?=
 =?us-ascii?Q?Q9FBGace6js40WO0cAe9V312qmGRNId4LOQkdgBgwetslGazRPpxM5DGZafY?=
 =?us-ascii?Q?GVB4PNVqxjDjIs59qy1PIC9oow3mODNIqgE5V7lpzWuAKIKEOBe7UkZIS371?=
 =?us-ascii?Q?Pxgw0kw9HBIQ4n42nQqkLe6CkXgq/OFzxVOkqk3MqwuaX4ZAxKWxYQ0h6YD4?=
 =?us-ascii?Q?SullXQU50hvSPmh1pKZQQt/L0+XQU8WiE17SLIqErDo5hc4IgLgqs2geF0Dd?=
 =?us-ascii?Q?C+vy+pvG4yDTZkqDmwCVxTlVSV94eXaUVCTRNQ+6N3CCyHw7IZVNHgCz+1F0?=
 =?us-ascii?Q?talKtVLuI93ySKhraLKsH4PLOjdA7/qb4hgPT5Nuw9qExyvtzCL30FmNk9iA?=
 =?us-ascii?Q?cHLtuZNi+Lt8oxtek3ZEzQpskfBLKImLW9nOmlLG+0ohTSE+5ibYG4EQYEic?=
 =?us-ascii?Q?aUGc0LOyj7xla8Tf3dPs1jhtVj+fcm2aEMAfXHv8AeGTqrv8/xvFi1n6M8Pm?=
 =?us-ascii?Q?9b2pOk4bKhtJR7fHmHae0T6dHM0UfKmgpKrtr+HeZvQCtnH9geCBp+Qbc7OJ?=
 =?us-ascii?Q?90FiRjDpFQVrtAzc6G/Bw+/kawZBRoQgbDtcf643x4IilYtKW96odMfxdhuP?=
 =?us-ascii?Q?uJeV2BcyqdnwtoUlFDgil0n7Lb6y0E9peJoLulgZ/Q7KaUHSyVEJUm1L08au?=
 =?us-ascii?Q?kL3+HIiIQGbqghOA737Msz5j8JQcCbdmcdgXmLtK52PFkal9ZOjmNY9U9ze+?=
 =?us-ascii?Q?mDtnRw3OyeR7ObQtfPb7aZxJcy2/O9crFGw5K61yT8YspQlslDBKpDBHoToT?=
 =?us-ascii?Q?2c1UPdToLNsRmaEtvSFrvlaEvCwxZamppJXy8QvQs1VlfYpVkxUWt50cHMUw?=
 =?us-ascii?Q?Ap5lWxDn6ogDVm7kvYUQ1XVN8OQYlsjqhWmpE2L70rQWdoTpT9cjjnN9ME9q?=
 =?us-ascii?Q?pQdsTP6XuFjkW+//9ueVETg6P6g1utd6vrIi1kvQdO5Id5cEO44NrNaYnS4G?=
 =?us-ascii?Q?RKFgsvMaWMONCZRmea6zDjUpbZ+t/luVIxRFBok9WdPTRtbEzH7XLhReLV/y?=
 =?us-ascii?Q?2hzvPS1u+PkH2bWBeISUkliH3jRWSI7jckpSXf8zqyaAtN8yBH4B5tSyMXO5?=
 =?us-ascii?Q?uqqzXjU2PRzzX4v2HM0y6A0ym3JNeYVzX8ThVGmGa6vmjGddwef7FUxCksa2?=
 =?us-ascii?Q?Y4oiOWv8ArS/7FaxjldNMb9qkzftP1T6rrxF9evLXaoUcsRD1zRGDlXxb7lF?=
 =?us-ascii?Q?LRUTnBUaGrVeq0IzeEVm2G+1ysuR0dQoylECQxDpkIliOdxXvrRR4YKHCVja?=
 =?us-ascii?Q?zt0x4RTnBeDJOUjKkRnbo1xldFk9Gs2s8ZFZoULE?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 3c45c72b-044e-48df-c7d3-08db12feb398
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6206.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Feb 2023 04:55:34.9997
 (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: IG3MEGcOb7bPPYGTMqKQOm+RbbNHzycmXBFuH74mnq4xWGwU4q8ovZ5M1vpAbWyCdKtrxhaieJrmxIxPg8hb9Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL1PR11MB5512
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2023-02-17 at 10:44:51 +0800, Abel Wu wrote:
> On 2/16/23 11:24 PM, Chen Yu wrote:
> > > The following change greatly reduced the p99lat of Redis service
> > > from 150ms to 0.9ms, at exactly the same throughput (QPS).
> > > 
> > > @@ -5763,6 +5787,9 @@ wake_affine_weight(struct sched_domain *sd, struct
> > > task_struct *p,
> > > 	s64 this_eff_load, prev_eff_load;
> > > 	unsigned long task_load;
> > > 
> > > +	if (is_short_task(p))
> > > +		return nr_cpumask_bits;
> > > +
> > So above change wants to wake up the short task on its previous
> > CPU if I understand correctly.
> 
> Yes.
> 
> > > 	this_eff_load = cpu_load(cpu_rq(this_cpu));
> > > 
> > > 	if (sync) {
> > > 
> > > I know that 'short' tasks are not necessarily 'small' tasks, e.g.
> > > sleeping duration is small or have large weights, but this works
> > > really well for this case. This is partly because delivering data
> > > is memory bandwidth intensive hence prefer cache hot cpus. And I
> > > think this is also applicable to the general purposes: do NOT let
> > > the short running tasks suffering from cache misses caused by
> > > migration.
> > > 
> > I see. My original thought was to mitigate short task migration
> > as much as possible. Either waking up the task on current CPU or previous
> > CPU should both achieve the goal in theory. Could you please describe
> > a little more about how Redis proxy server was tested? Was it tested
> > locally or using multiple machines? I asked this because for network
> > benchmarks, it might be better to wake the task close to the waker(maybe
> > the NIC interrupt) due to hot network buffer. Anyway I will test
> > your change slightly changed to see the impact, and also Redis. But it
> > would be even better if you could provide some simple test steps I can
> > try locally : )
> 
> Sorry for missing the info. The test was done in production environment,
> and what I have done is only updating the kernel in several machines
> which are highly loaded, that is over 85% cpu util observed by mpstat.
> Please let me know if you want any specific info.
>
I've modified the code a little bit, which was inpired by your statement
"'small' tasks can be easily stacked on this cpu when wake up several tasks
at one time if this cpu is unloaded". I added extra check on wakee_flips, so
that tasks waking up too many different tasks will not be treated as 'small'
ones. That is to say, only tasks waking up each other exclusively will be
put together. So far this change has verified to keep the improvement for
netperf/will-it-scale, and I just launched the redis test to see what will
happen.

thanks,
Chenyu