Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 28 Mar 2023 14:42:31 +0800
From:   Aaron Lu <aaron.lu@intel.com>
To:     Chen Yu <yu.c.chen@intel.com>
CC:     Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Juri Lelli <juri.lelli@redhat.com>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        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>,
        Tim Chen <tim.c.chen@intel.com>,
        Nitin Tekchandani <nitin.tekchandani@intel.com>,
        Waiman Long <longman@redhat.com>,
        <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH] sched/fair: Make tg->load_avg per node
Message-ID: <20230328064231.GA4899@ziqianlu-desk2>
References: <20230327053955.GA570404@ziqianlu-desk2>
 <ZCGsJB36YYbgsgBW@chenyu5-mobl1>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <ZCGsJB36YYbgsgBW@chenyu5-mobl1>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?3cxfUphSIQMM4f0RVoSxav7jKVBktncKCiQTBnLUXp7QWpmT9Fr7DAFSLVUH?=
 =?us-ascii?Q?QJAMxUayyJaM6S7ak7vAySpYDG4T5YimMCBS1bZ0ouwQJjU9D2TzHAUftIfk?=
 =?us-ascii?Q?GyOEGrfc94qa1NbJk7QCJGVmvpjSBZvXxXhTRDRw+KNMt2wNKX0pn6ZthRPd?=
 =?us-ascii?Q?iNeyovxk9ZeBGhzPwmz6Px3AHXlm9COKBIqxlEuXGy5NNsJ3yxGZYKhjYxPa?=
 =?us-ascii?Q?E8avBrP0bBxFqEBY5CYc918TElABzx6c9Dw0SzyYlwm0gsifM2OHrN9SvMuW?=
 =?us-ascii?Q?Wb1gDh+9M0IakuF4CGAFy8p9KW351qxbLa2Fg7QwU1rNES0om/08vvBaA9ao?=
 =?us-ascii?Q?Lw+/cgtmx85sEn73lF/zQOzatxdXwyldXb2GBwqMwaYHK1bksqZTqJtSuMV9?=
 =?us-ascii?Q?UkaOzdciTiAbWSdRvxMoQ51ZLRTD4w+rk9q1e6LoaL2H92p8la2Qtf9+KvhS?=
 =?us-ascii?Q?8k8QVIF4++l6q6bEvSKL7sLLllJZh34lHLvGiWtUJDyItO21SjtY1Gfx5U0K?=
 =?us-ascii?Q?LxD56ru6XKxcY8qLWhiMlAxMFVOQBi3e2Itv7qj8vzLrRkdTBQttFRxAJivt?=
 =?us-ascii?Q?GPIxdhIszNmcmFdp3FP5YEUTUdUdNkjaNqEyKjIax7qp2uxSRJM/7Jw9HWoR?=
 =?us-ascii?Q?3P1blyT/77DrL4mBlT1ilKZ5Vryrm5QsdyywCzoLVqkTSFQWnGjRSyyOjRQ7?=
 =?us-ascii?Q?T/ytbFdeygAuTT3KI80j0JTgY20UY6Cj59atg3CQsWliluCPKZJG92hy6uVS?=
 =?us-ascii?Q?fE+fgQCNsYCe6cDQp/teHjB879OfVid9GvU/HpNOA+kcLym73k9sLGAC5KHv?=
 =?us-ascii?Q?JsxAo1E6+3b/d5y5C3iPb4A5jhOV7wH0ClP8cOo19mj7EShWfHHiHOw6WX/W?=
 =?us-ascii?Q?nH29Nvh8oXkwo52IjsuKSl/3oGDXSgk0eFDA/NzB6b+EVj4SCLGEA0LvWehb?=
 =?us-ascii?Q?juO7QyZuTwxpJr3m9IA5BwfTKqYzRLZ7Qt+P8F24+v4NydcwcEzmkncD1q83?=
 =?us-ascii?Q?a8RVpco7zlxWqJEg2A89S3VNOOesjlhs66Y4SrzO4KPkwZ82Z3XrymcUGHEb?=
 =?us-ascii?Q?2q0HHsj90zJ5Nbfl58UbEzarkwuStShEflGvVlSlgSJ9oqxqhIRVSbhIe0bC?=
 =?us-ascii?Q?VV7zHDVYsSjsnVC02ortzFF51DxAXHxJbpxwO4CXG4ORNxTlrzswizexyqKb?=
 =?us-ascii?Q?aHqz4zlPZYWJX9mcJUx3YyVb0Lvk76djbrzaFb/s5OJBPa2Xxq6EVOiiRJ5+?=
 =?us-ascii?Q?m91CmcVoiQKw1eD29t5LmVLfYjnpRJu+RdTweh0BaPIFLK4ml+5wtE002fXf?=
 =?us-ascii?Q?RrgFKussTXj4+w4/APKcMxqJAURGpVvW8u2HplBah5HnzTOUmOfDVxthsZoP?=
 =?us-ascii?Q?0EZt38CaLtCQWvbY+A7jWaDmBPj0qugSuI+dgKsZeZqiXuYwihbBiS7I20Nc?=
 =?us-ascii?Q?uiJQdMeSTuEoIjDGbOV53+SBcdD9JBnb0qoWXM9Moi5Ph9TzZjOJLHrColqL?=
 =?us-ascii?Q?eUvnSESP83aC+L7DuzT47x1alnzHf8ja+MV/I4oJzC73+POZXbHCi8yPiK3g?=
 =?us-ascii?Q?V8iHZei7OKhWos3d2HjckfBkw+Vxl4r/qZO6S45T?=
X-MS-Exchange-CrossTenant-Network-Message-Id: b74dcce4-3490-4841-057f-08db2f57a1e0
X-MS-Exchange-CrossTenant-AuthSource: BYAPR11MB3062.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Mar 2023 06:42:43.1124
 (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: Wzz/8yGs6Ny5FSbM1OeOM5pkBchoJxF09RSePelGbklBYKGnutRmLE0EhnV1q18s6mOpDcHTjJvwfiP7eIRU5w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR11MB7128
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.5 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE
        autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Yu,

Thanks for taking a look.

On Mon, Mar 27, 2023 at 10:45:56PM +0800, Chen Yu wrote:
> On 2023-03-27 at 13:39:55 +0800, Aaron Lu wrote:
> > When using sysbench to benchmark Postgres in a single docker instance
> > with sysbench's nr_threads set to nr_cpu, it is observed there are times
> > update_cfs_group() and update_load_avg() shows noticeable overhead on
> > cpus of one node of a 2sockets/112core/224cpu Intel Sapphire Rapids:
> > 
> >     10.01%     9.86%  [kernel.vmlinux]        [k] update_cfs_group
> >      7.84%     7.43%  [kernel.vmlinux]        [k] update_load_avg
> > 
> > While cpus of the other node normally sees a lower cycle percent:
> > 
> >      4.46%     4.36%  [kernel.vmlinux]        [k] update_cfs_group
> >      4.02%     3.40%  [kernel.vmlinux]        [k] update_load_avg
> > 
> > Annotate shows the cycles are mostly spent on accessing tg->load_avg
> > with update_load_avg() being the write side and update_cfs_group() being
> > the read side.
> > 
> > The reason why only cpus of one node has bigger overhead is: task_group
> > is allocated on demand from a slab and whichever cpu happens to do the
> > allocation, the allocated tg will be located on that node and accessing
> > to tg->load_avg will have a lower cost for cpus on the same node and
> > a higer cost for cpus of the remote node.
> > 
> > Tim Chen told me that PeterZ once mentioned a way to solve a similar
> > problem by making a counter per node so do the same for tg->load_avg.
> > After this change, the worst number I saw during a 5 minutes run from
> > both nodes are:
> > 
> >      2.77%     2.11%  [kernel.vmlinux]        [k] update_load_avg
> >      2.72%     2.59%  [kernel.vmlinux]        [k] update_cfs_group
> > 
> > Another observation of this workload is: it has a lot of wakeup time
> > task migrations and that is the reason why update_load_avg() and
> > update_cfs_group() shows noticeable cost. Running this workload in N
> > instances setup where N >= 2 with sysbench's nr_threads set to 1/N nr_cpu,
> > task migrations on wake up time are greatly reduced and the overhead from
> > the two above mentioned functions also dropped a lot. It's not clear to
> > me why running in multiple instances can reduce task migrations on
> > wakeup path yet.
> >
> Looks interesting, when the sysbench is 1 instance and nr_threads = nr_cpu,
> and when the launches more than 1 instance of sysbench, while nr_threads set
> to 1/N * nr_cpu, do both cases have similar CPU utilization? Currently the
> task wakeup inhibits migration wakeup if the system is overloaded.

I think this is a good point. I did notice during a run, when CPU util
is up, the migration number will drop. And 4 instances setup generally
has higher CPU util than 1 instance setup.

I should also add that in vanilla kernel, if tg is allocated on node 0
then task migrations happening on remote node is the deciding factor of
an increased cost of update_cfs_group() and update_load_avg() because
remote node has a higher cost of accessing tg->load_avg.

> [...]
> >  struct task_group *sched_create_group(struct task_group *parent)
> >  {
> > +	size_t size = sizeof(struct task_group);
> > +	int __maybe_unused i, nodes;
> >  	struct task_group *tg;
> >  
> > -	tg = kmem_cache_alloc(task_group_cache, GFP_KERNEL | __GFP_ZERO);
> > +#if defined(CONFIG_FAIR_GROUP_SCHED) && defined(CONFIG_SMP)
> > +	nodes = num_possible_nodes();
> > +	size += nodes * sizeof(void *);
> > +	tg = kzalloc(size, GFP_KERNEL);
> > +	if (!tg)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	for_each_node(i) {
> > +		tg->node_info[i] = kzalloc_node(sizeof(struct tg_node_info), GFP_KERNEL, i);
> > +		if (!tg->node_info[i])
> > +			return ERR_PTR(-ENOMEM);
> Do we need to free tg above in case of memory leak?

Good catch, will fix this in next posting, thanks!