| Message ID | 20200218004354.24996-1-richardw.yang@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm/vmscan.c: remove cpu online notification for now | expand |
On Tue 18-02-20 08:43:54, Wei Yang wrote: > The cpu online notification is used to adjust kswapd cpu affinity when a > NUMA node gains a new CPU. > > Since currently we don't see a real runtime configuration like this, > let's drop this online notification for now. This deserves much more explanation IMHO. What would you say about the following. " kswapd kernel thread starts either with a CPU affinity set to the full cpu mask of its target node or without any affinity at all if the node is CPUless. There is a cpu hotplug callback (kswapd_cpu_online) that implements an elaborate way to update this mask when a cpu is onlined. It is not really clear whether there is any actual benefit from this scheme. Completely CPU-less NUMA nodes rarely gain a new CPU during runtime. Drop the code for that reason. If there is a real usecase then we can resurrect and simplify the code. " > > Suggested-by: Michal Hocko <mhocko@kernel.org> > Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> Acked-by: Michal Hocko <mhocko@suse.com> > > --- > v3: > * remove the cpu online notification suggested by Michal > v2: > * rephrase the changelog > --- > mm/vmscan.c | 27 +-------------------------- > 1 file changed, 1 insertion(+), 26 deletions(-) > > diff --git a/mm/vmscan.c b/mm/vmscan.c > index 665f33258cd7..a4fdf3dc8887 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -4023,27 +4023,6 @@ unsigned long shrink_all_memory(unsigned long nr_to_reclaim) > } > #endif /* CONFIG_HIBERNATION */ > > -/* It's optimal to keep kswapds 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 kswapd_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 */ > - set_cpus_allowed_ptr(pgdat->kswapd, mask); > - } > - return 0; > -} > - > /* > * This kswapd start function will be called by init and node-hot-add. > * On node-hot-add, kswapd will moved to proper cpus if cpus are hot-added. > @@ -4083,15 +4062,11 @@ void kswapd_stop(int nid) > > static int __init kswapd_init(void) > { > - int nid, ret; > + int nid; > > swap_setup(); > for_each_node_state(nid, N_MEMORY) > kswapd_run(nid); > - ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, > - "mm/vmscan:online", kswapd_cpu_online, > - NULL); > - WARN_ON(ret < 0); > return 0; > } > > -- > 2.17.1
On Tue, Feb 18, 2020 at 09:40:23AM +0100, Michal Hocko wrote: >On Tue 18-02-20 08:43:54, Wei Yang wrote: >> The cpu online notification is used to adjust kswapd cpu affinity when a >> NUMA node gains a new CPU. >> >> Since currently we don't see a real runtime configuration like this, >> let's drop this online notification for now. > >This deserves much more explanation IMHO. What would you say about the >following. >" >kswapd kernel thread starts either with a CPU affinity set to the full >cpu mask of its target node or without any affinity at all if the node >is CPUless. There is a cpu hotplug callback (kswapd_cpu_online) that >implements an elaborate way to update this mask when a cpu is onlined. > >It is not really clear whether there is any actual benefit from this >scheme. Completely CPU-less NUMA nodes rarely gain a new CPU during >runtime. Drop the code for that reason. If there is a real usecase then >we can resurrect and simplify the code. >" More better :-) Thanks >> >> Suggested-by: Michal Hocko <mhocko@kernel.org> >> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> > >Acked-by: Michal Hocko <mhocko@suse.com> > >> >> --- >> v3: >> * remove the cpu online notification suggested by Michal >> v2: >> * rephrase the changelog >> --- >> mm/vmscan.c | 27 +-------------------------- >> 1 file changed, 1 insertion(+), 26 deletions(-) >> >> diff --git a/mm/vmscan.c b/mm/vmscan.c >> index 665f33258cd7..a4fdf3dc8887 100644 >> --- a/mm/vmscan.c >> +++ b/mm/vmscan.c >> @@ -4023,27 +4023,6 @@ unsigned long shrink_all_memory(unsigned long nr_to_reclaim) >> } >> #endif /* CONFIG_HIBERNATION */ >> >> -/* It's optimal to keep kswapds 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 kswapd_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 */ >> - set_cpus_allowed_ptr(pgdat->kswapd, mask); >> - } >> - return 0; >> -} >> - >> /* >> * This kswapd start function will be called by init and node-hot-add. >> * On node-hot-add, kswapd will moved to proper cpus if cpus are hot-added. >> @@ -4083,15 +4062,11 @@ void kswapd_stop(int nid) >> >> static int __init kswapd_init(void) >> { >> - int nid, ret; >> + int nid; >> >> swap_setup(); >> for_each_node_state(nid, N_MEMORY) >> kswapd_run(nid); >> - ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, >> - "mm/vmscan:online", kswapd_cpu_online, >> - NULL); >> - WARN_ON(ret < 0); >> return 0; >> } >> >> -- >> 2.17.1 > >-- >Michal Hocko >SUSE Labs
diff --git a/mm/vmscan.c b/mm/vmscan.c index 665f33258cd7..a4fdf3dc8887 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -4023,27 +4023,6 @@ unsigned long shrink_all_memory(unsigned long nr_to_reclaim) } #endif /* CONFIG_HIBERNATION */ -/* It's optimal to keep kswapds 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 kswapd_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 */ - set_cpus_allowed_ptr(pgdat->kswapd, mask); - } - return 0; -} - /* * This kswapd start function will be called by init and node-hot-add. * On node-hot-add, kswapd will moved to proper cpus if cpus are hot-added. @@ -4083,15 +4062,11 @@ void kswapd_stop(int nid) static int __init kswapd_init(void) { - int nid, ret; + int nid; swap_setup(); for_each_node_state(nid, N_MEMORY) kswapd_run(nid); - ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, - "mm/vmscan:online", kswapd_cpu_online, - NULL); - WARN_ON(ret < 0); return 0; }
The cpu online notification is used to adjust kswapd cpu affinity when a NUMA node gains a new CPU. Since currently we don't see a real runtime configuration like this, let's drop this online notification for now. Suggested-by: Michal Hocko <mhocko@kernel.org> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> --- v3: * remove the cpu online notification suggested by Michal v2: * rephrase the changelog --- mm/vmscan.c | 27 +-------------------------- 1 file changed, 1 insertion(+), 26 deletions(-)