Re: [PATCH 13/19] mm: Create/affine kcompactd to its preferred node

[Michal Hocko <mhocko@suse.com>]

On Tue 17-09-24 00:49:17, Frederic Weisbecker wrote:
> Kcompactd is dedicated to a specific node. As such it wants to be
> preferrably affine to it, memory and CPUs-wise.
> 
> Use the proper kthread API to achieve that. As a bonus it takes care of
> CPU-hotplug events and CPU-isolation on its behalf.
> 
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
> Signed-off-by: Frederic Weisbecker <frederic@kernel.org>

Acked-by: Michal Hocko <mhocko@suse.com>
Clear simplification, thanks!

> ---
>  mm/compaction.c | 43 +++----------------------------------------
>  1 file changed, 3 insertions(+), 40 deletions(-)
> 
> diff --git a/mm/compaction.c b/mm/compaction.c
> index eb95e9b435d0..69742555f2e5 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -3179,15 +3179,9 @@ void wakeup_kcompactd(pg_data_t *pgdat, int order, int highest_zoneidx)
>  static int kcompactd(void *p)
>  {
>  	pg_data_t *pgdat = (pg_data_t *)p;
> -	struct task_struct *tsk = current;
>  	long default_timeout = msecs_to_jiffies(HPAGE_FRAG_CHECK_INTERVAL_MSEC);
>  	long timeout = default_timeout;
>  
> -	const struct cpumask *cpumask = cpumask_of_node(pgdat->node_id);
> -
> -	if (!cpumask_empty(cpumask))
> -		set_cpus_allowed_ptr(tsk, cpumask);
> -
>  	set_freezable();
>  
>  	pgdat->kcompactd_max_order = 0;
> @@ -3258,10 +3252,12 @@ void __meminit kcompactd_run(int nid)
>  	if (pgdat->kcompactd)
>  		return;
>  
> -	pgdat->kcompactd = kthread_run(kcompactd, pgdat, "kcompactd%d", nid);
> +	pgdat->kcompactd = kthread_create_on_node(kcompactd, pgdat, nid, "kcompactd%d", nid);
>  	if (IS_ERR(pgdat->kcompactd)) {
>  		pr_err("Failed to start kcompactd on node %d\n", nid);
>  		pgdat->kcompactd = NULL;
> +	} else {
> +		wake_up_process(pgdat->kcompactd);
>  	}
>  }
>  
> @@ -3279,30 +3275,6 @@ void __meminit kcompactd_stop(int nid)
>  	}
>  }
>  
> -/*
> - * It's optimal to keep kcompactd on the same CPUs as their memory, but
> - * not required for correctness. So if the last cpu in a node goes
> - * away, we get changed to run anywhere: as the first one comes back,
> - * restore their cpu bindings.
> - */
> -static int kcompactd_cpu_online(unsigned int cpu)
> -{
> -	int nid;
> -
> -	for_each_node_state(nid, N_MEMORY) {
> -		pg_data_t *pgdat = NODE_DATA(nid);
> -		const struct cpumask *mask;
> -
> -		mask = cpumask_of_node(pgdat->node_id);
> -
> -		if (cpumask_any_and(cpu_online_mask, mask) < nr_cpu_ids)
> -			/* One of our CPUs online: restore mask */
> -			if (pgdat->kcompactd)
> -				set_cpus_allowed_ptr(pgdat->kcompactd, mask);
> -	}
> -	return 0;
> -}
> -
>  static int proc_dointvec_minmax_warn_RT_change(const struct ctl_table *table,
>  		int write, void *buffer, size_t *lenp, loff_t *ppos)
>  {
> @@ -3362,15 +3334,6 @@ static struct ctl_table vm_compaction[] = {
>  static int __init kcompactd_init(void)
>  {
>  	int nid;
> -	int ret;
> -
> -	ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
> -					"mm/compaction:online",
> -					kcompactd_cpu_online, NULL);
> -	if (ret < 0) {
> -		pr_err("kcompactd: failed to register hotplug callbacks.\n");
> -		return ret;
> -	}
>  
>  	for_each_node_state(nid, N_MEMORY)
>  		kcompactd_run(nid);
> -- 
> 2.46.0

-- 
Michal Hocko
SUSE Labs