Received-SPF: pass (google.com: domain of linux-kernel+bounces-84034-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Tue, 27 Feb 2024 16:24:20 -0500
From: Daniel Jordan <daniel.m.jordan@oracle.com>
To: Gang Li <gang.li@linux.dev>
Cc: Andrew Morton <akpm@linux-foundation.org>,
        David Hildenbrand <david@redhat.com>,
        David Rientjes <rientjes@google.com>,
        Muchun Song <muchun.song@linux.dev>,
        Tim Chen <tim.c.chen@linux.intel.com>,
        Steffen Klassert <steffen.klassert@secunet.com>,
        Jane Chu <jane.chu@oracle.com>,
        "Paul E . McKenney" <paulmck@kernel.org>,
        Randy Dunlap <rdunlap@infradead.org>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, ligang.bdlg@bytedance.com
Subject: Re: [PATCH v6 4/8] padata: dispatch works on different nodes
Message-ID: <7g53p42favkoibzg4w3ly3ypdjdy6oymhj74ekwk62bwx4rlaj@seoavjygfadq>
References: <20240222140422.393911-1-gang.li@linux.dev>
 <20240222140422.393911-5-gang.li@linux.dev>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240222140422.393911-5-gang.li@linux.dev>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?us-ascii?Q?0pXcfA5CGZo8qvJV+kRrSQP+dSgnnGH0oJSxevBBPiW7saCmE/ILZOq5JhRX?=
 =?us-ascii?Q?d6uvGMQfoXOvuHVEYI9u8gQIXxSea72DEouux8pI0K+XPPeS/hp6TlMpcgO9?=
 =?us-ascii?Q?QRgebX0E5JeSX2xZ1C8o4Ifv4gGkKNgYgYZsCVUeFY75/zVJdHO90pxPnTtO?=
 =?us-ascii?Q?9BmeQ634Tt4QD5WwqY2q5p3uPoGpSNUGScWx9JHvNBU9LNENkgHT+YLRySVT?=
 =?us-ascii?Q?7TExH68RfFo1bpjDhI2DdLD0p0zUyZZ5OH78AilzS36555hms+81DBmPsBob?=
 =?us-ascii?Q?RZPlZDQ48JWZoCM69Xm7pHotl7FJU3u/I7dYJL4hVdQZdAIG0SP8FkPnzVVn?=
 =?us-ascii?Q?T+2Bz3HGurilBDa1bOyRczZfemVqOmh5WjqJN935FbSVo8dl8WlJFN1l346E?=
 =?us-ascii?Q?tntzYL3zlkwM1vM/R4BOu/Y3BaId1zHer8EMmz3aa0p+XwKOQpRtS/w99f3N?=
 =?us-ascii?Q?+0OLGrsmqEY8DhOTYcr0FK+PqXJml+CnZlPNBA+UKM9aoEnHOVXmJiY2zhrb?=
 =?us-ascii?Q?S/885KYl0P9pXqLkCPr1sfBUq28rx5JHM+rod6fg98nJjhzpmU7duzkfSQfr?=
 =?us-ascii?Q?j+IKU97Ys69wOq4eNQNklmkMQdfdqPBGKyme9+TKGPp6dU2aTazLqZgnoK5v?=
 =?us-ascii?Q?0ccNYifZpsDhOdh4FKm64FkPYaDmfo6usLdRKak9QZa/YgGHaAE4FBKWbqEP?=
 =?us-ascii?Q?q4t+ekj1tSMjilpFv9Wk3H2yL0z87ruSHQcy2djuG9ByqdHXJtcTYuDwIPqj?=
 =?us-ascii?Q?5OuGxqmOgecd6WJ5YdJCpbpkdLSaXT5wT+F2kVJg8q+y47UeAXKVjLToZ1+3?=
 =?us-ascii?Q?DMBYjSTuFGlWB2gYXnmUygfpL7E2gKwBOjd0WsJcFBIoitNza6EONjM8Hsc4?=
 =?us-ascii?Q?pJ4wj1mZYck61eNctqXTDZc1BH+MO9UPVJ7ntsUro+VFslROse0S1S2yWtZj?=
 =?us-ascii?Q?7Z/WtabdMCBllZLYs8I37t21/qEHx+SVNN3bDRgUzlA4tLoLNUapH4KIndrd?=
 =?us-ascii?Q?c9Qb+CZg/n/yR1BRzKIQTCKm6Box+DbmILhLk7+/I+v6CQtRnPCaeVw74Eb+?=
 =?us-ascii?Q?kW0jbCMMQrnbSnr4GPyD92+Z6IEOCjlVYJdaWuL70Ne+UY3QXR1hhe00PwAM?=
 =?us-ascii?Q?1ZHo342gdeOo1wrwyxPEyE/XeGaAQAwCgJCKlPwGCIkJ0yexeH5a35LECDnf?=
 =?us-ascii?Q?SoiuWVk7IpjAVO5orrdbIJcOzFChfKTvPIjXDIAHTQQOvrhCXnaKgxjDpZL+?=
 =?us-ascii?Q?Aon1i5DWx9LrPPWcqBkpj/c0pq6AdJ0u/xF46hK4NedYdBHEmEMHG+kOuuPK?=
 =?us-ascii?Q?YCA5NbUSDpfILh33ue7JPs74nmpBk7HevBjZmue/z+PgAQ0tyxPbRsmMpoWI?=
 =?us-ascii?Q?m+Xhv8493MF0lHrecYyefJ10rO++I8fhdba0KPP+UXARwz/sbS3r/dsGB0RM?=
 =?us-ascii?Q?Q4GeporwXo8p+8HtbV79V1XlalLhYiI90juLFpSuFHAZCb+rEthFAHq3I6Ed?=
 =?us-ascii?Q?XAarnWsnGhaP1PG9V0o1uPiH1LOFqovQAOra/LWMOYoyRhSWjhSZzEicQISZ?=
 =?us-ascii?Q?WzfExt5HErMQcDGYW7i9EZLAs/1V75jMAIBwUwUxfXsb3+UbiXj8SwamRVCB?=
 =?us-ascii?Q?wQ=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	vYK6lnrp8CoIYjbmlaWVM5UDNF4lOyFoM2ScZfnfrU8LILfWAq4BY1FM8dtq2RseRVEwmSJbsWC7CK7fURlE0Baixd6zv/3JJkKnri0VWwdbRnwvNrrA+oKb6pO6mv8WfHNZmOhssRMjHhPGRv4Q4n8GdwWHXZTVt4+zDAL3E+f8i6/6wlOkSYQ9lAS76ALG7tTUS92hUwQOOaznHhCUh1o9qhrJ26Q6yR+sosJyQV+81oqUkotvgEkJR1E0Ay7xT4lJnHp4jVBDvbw6/NS3//uFBH1pbT6XafM8yLRPyg9rl84+RguWaA/fOInoxUZOsWaPb6r1JQfmIv/9b3J6nimKJ2x8PecLAQ9sCGHd2u4GFNuvEBvOlelhAjOo3Gww5jb6BG3h0PmW8YIK6KDvGkMCir0io8jq1neMcFvUtcDVQntwHUWfUASUFxIL8XgElGKcPJWGIYpXjVhCP6gpT8fetxQ1/S5R+gg5zLIi7uxsUJ49zhB0lmsYb5M+IKv5ICW1bt7IUhlKxXwYcp+m5WSY53vdJkCH9SZ6M9wA84BXKNpXhDvS+glq1dOmqD3yoWmO2hmmwGDR8XrcdIB5Gejc1N2XAnRy4AKxapPtRZ0=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 94f65d76-fe32-491e-30d6-08dc37da77f5
X-MS-Exchange-CrossTenant-AuthSource: SA1PR10MB5711.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Feb 2024 21:24:23.6578
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: EIvWdLyXzhh6BWHRWGGlL2EXMImISA7OU2Fzy8U+Rx8yI63kS93Q514IDKmZMMtS/VUCUmF9cmgnKuQwybv9Rwe2W+07gd/p6f4XQCfMExk=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR10MB7154
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-02-27_09,2024-02-27_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 malwarescore=0 bulkscore=0
 spamscore=0 mlxlogscore=999 phishscore=0 suspectscore=0 mlxscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311290000
 definitions=main-2402270164
X-Proofpoint-ORIG-GUID: E5Ph5OSqncxYLzrjfaPNkF1rHwWDodiL
X-Proofpoint-GUID: E5Ph5OSqncxYLzrjfaPNkF1rHwWDodiL

Hi,

On Thu, Feb 22, 2024 at 10:04:17PM +0800, Gang Li wrote:
> When a group of tasks that access different nodes are scheduled on the
> same node, they may encounter bandwidth bottlenecks and access latency.
> 
> Thus, numa_aware flag is introduced here, allowing tasks to be
> distributed across different nodes to fully utilize the advantage of
> multi-node systems.
> 
> Signed-off-by: Gang Li <ligang.bdlg@bytedance.com>
> Tested-by: David Rientjes <rientjes@google.com>
> Reviewed-by: Muchun Song <muchun.song@linux.dev>
> Reviewed-by: Tim Chen <tim.c.chen@linux.intel.com>
> ---
>  include/linux/padata.h |  2 ++
>  kernel/padata.c        | 14 ++++++++++++--
>  mm/mm_init.c           |  1 +
>  3 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/padata.h b/include/linux/padata.h
> index 495b16b6b4d72..8f418711351bc 100644
> --- a/include/linux/padata.h
> +++ b/include/linux/padata.h
> @@ -137,6 +137,7 @@ struct padata_shell {
>   *             appropriate for one worker thread to do at once.
>   * @max_threads: Max threads to use for the job, actual number may be less
>   *               depending on task size and minimum chunk size.
> + * @numa_aware: Distribute jobs to different nodes with CPU in a round robin fashion.

numa_interleave seems more descriptive.

>   */
>  struct padata_mt_job {
>  	void (*thread_fn)(unsigned long start, unsigned long end, void *arg);
> @@ -146,6 +147,7 @@ struct padata_mt_job {
>  	unsigned long		align;
>  	unsigned long		min_chunk;
>  	int			max_threads;
> +	bool			numa_aware;
>  };
>  
>  /**
> diff --git a/kernel/padata.c b/kernel/padata.c
> index 179fb1518070c..e3f639ff16707 100644
> --- a/kernel/padata.c
> +++ b/kernel/padata.c
> @@ -485,7 +485,8 @@ void __init padata_do_multithreaded(struct padata_mt_job *job)
>  	struct padata_work my_work, *pw;
>  	struct padata_mt_job_state ps;
>  	LIST_HEAD(works);
> -	int nworks;
> +	int nworks, nid;
> +	static atomic_t last_used_nid __initdata;

nit, move last_used_nid up so it's below load_balance_factor to keep
that nice tree shape

>  
>  	if (job->size == 0)
>  		return;
> @@ -517,7 +518,16 @@ void __init padata_do_multithreaded(struct padata_mt_job *job)
>  	ps.chunk_size = roundup(ps.chunk_size, job->align);
>  
>  	list_for_each_entry(pw, &works, pw_list)
> -		queue_work(system_unbound_wq, &pw->pw_work);
> +		if (job->numa_aware) {
> +			int old_node = atomic_read(&last_used_nid);
> +
> +			do {
> +				nid = next_node_in(old_node, node_states[N_CPU]);
> +			} while (!atomic_try_cmpxchg(&last_used_nid, &old_node, nid));

There aren't concurrent NUMA-aware _do_multithreaded calls now, so an
atomic per work seems like an unnecessary expense for guarding against
possible uneven thread distribution in the future.  Non-atomic access
instead?

> +			queue_work_node(nid, system_unbound_wq, &pw->pw_work);
> +		} else {
> +			queue_work(system_unbound_wq, &pw->pw_work);
> +		}
>  
>  	/* Use the current thread, which saves starting a workqueue worker. */
>  	padata_work_init(&my_work, padata_mt_helper, &ps, PADATA_WORK_ONSTACK);
> diff --git a/mm/mm_init.c b/mm/mm_init.c
> index 2c19f5515e36c..549e76af8f82a 100644
> --- a/mm/mm_init.c
> +++ b/mm/mm_init.c
> @@ -2231,6 +2231,7 @@ static int __init deferred_init_memmap(void *data)
>  			.align       = PAGES_PER_SECTION,
>  			.min_chunk   = PAGES_PER_SECTION,
>  			.max_threads = max_threads,
> +			.numa_aware  = false,
>  		};
>  
>  		padata_do_multithreaded(&job);
> -- 
> 2.20.1
>