2020-02-18 00:46:17

by Wei Yang

[permalink] [raw]
Subject: [PATCH] mm/vmscan.c: remove cpu online notification for now

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 <[email protected]>
Signed-off-by: Wei Yang <[email protected]>

---
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


2020-02-18 08:42:03

by Michal Hocko

[permalink] [raw]
Subject: Re: [PATCH] mm/vmscan.c: remove cpu online notification for now

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 <[email protected]>
> Signed-off-by: Wei Yang <[email protected]>

Acked-by: Michal Hocko <[email protected]>

>
> ---
> 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

2020-02-18 22:40:28

by Wei Yang

[permalink] [raw]
Subject: Re: [PATCH] mm/vmscan.c: remove cpu online notification for now

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 <[email protected]>
>> Signed-off-by: Wei Yang <[email protected]>
>
>Acked-by: Michal Hocko <[email protected]>
>
>>
>> ---
>> 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

--
Wei Yang
Help you, Help me