| Message ID | 20200126132052.11921-1-richardw.yang@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm/vmscan.c: only adjust related kswapd cpu affinity when online cpu | expand |
On Sun, 26 Jan 2020, Wei Yang wrote: > When onlining a cpu, kswapd_cpu_online() is called to adjust kswapd cpu > affinity. > > Current routine does like this: > > * Iterate all the numa node > * Adjust cpu affinity when node has an online cpu > > Actually we could improve this by: > > * Just adjust the numa node to which current online cpu belongs > > Because we know which numa node the cpu belongs to and this cpu would > not affect other node. > Is there ever a situation where the cpu to be onlined would have appeared in the cpumask of another node and so a different kswapd's cpumask would now include an off-node cpu? > Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> > --- > mm/vmscan.c | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) > > diff --git a/mm/vmscan.c b/mm/vmscan.c > index 572fb17c6273..19c92d35045c 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -4049,18 +4049,19 @@ unsigned long shrink_all_memory(unsigned long nr_to_reclaim) > restore their cpu bindings. */ > static int kswapd_cpu_online(unsigned int cpu) > { > - int nid; > + int nid = cpu_to_node(cpu); > + pg_data_t *pgdat; > + const struct cpumask *mask; > > - for_each_node_state(nid, N_MEMORY) { > - pg_data_t *pgdat = NODE_DATA(nid); > - const struct cpumask *mask; > + if (!node_state(nid, N_MEMORY)) > + return 0; > > - mask = cpumask_of_node(pgdat->node_id); > + pgdat = NODE_DATA(nid); > + mask = cpumask_of_node(nid); > + > + /* One of our CPUs online: restore mask */ > + set_cpus_allowed_ptr(pgdat->kswapd, mask); > > - 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; > } > > -- > 2.17.1 > > >
On Sun, Jan 26, 2020 at 02:44:31PM -0800, David Rientjes wrote: >On Sun, 26 Jan 2020, Wei Yang wrote: > >> When onlining a cpu, kswapd_cpu_online() is called to adjust kswapd cpu >> affinity. >> >> Current routine does like this: >> >> * Iterate all the numa node >> * Adjust cpu affinity when node has an online cpu >> >> Actually we could improve this by: >> >> * Just adjust the numa node to which current online cpu belongs >> >> Because we know which numa node the cpu belongs to and this cpu would >> not affect other node. >> > >Is there ever a situation where the cpu to be onlined would have appeared >in the cpumask of another node and so a different kswapd's cpumask would >now include an off-node cpu? No, I don't think so. Per my understanding, kswapd_cpu_online() will be invoked when a cpu is onlined. And the particular cpu belongs to a particular numa node. So it is not necessary to iterate on every nodes. And current code use cpumask_and_any() to do the check. If my understanding is correct, the check would return true if this node has any online cpu. This is likely to be true. This is why I want to make the logic clear. > >> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> >> --- >> mm/vmscan.c | 19 ++++++++++--------- >> 1 file changed, 10 insertions(+), 9 deletions(-) >> >> diff --git a/mm/vmscan.c b/mm/vmscan.c >> index 572fb17c6273..19c92d35045c 100644 >> --- a/mm/vmscan.c >> +++ b/mm/vmscan.c >> @@ -4049,18 +4049,19 @@ unsigned long shrink_all_memory(unsigned long nr_to_reclaim) >> restore their cpu bindings. */ >> static int kswapd_cpu_online(unsigned int cpu) >> { >> - int nid; >> + int nid = cpu_to_node(cpu); >> + pg_data_t *pgdat; >> + const struct cpumask *mask; >> >> - for_each_node_state(nid, N_MEMORY) { >> - pg_data_t *pgdat = NODE_DATA(nid); >> - const struct cpumask *mask; >> + if (!node_state(nid, N_MEMORY)) >> + return 0; >> >> - mask = cpumask_of_node(pgdat->node_id); >> + pgdat = NODE_DATA(nid); >> + mask = cpumask_of_node(nid); >> + >> + /* One of our CPUs online: restore mask */ >> + set_cpus_allowed_ptr(pgdat->kswapd, mask); >> >> - 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; >> } >> >> -- >> 2.17.1 >> >> >>
On Tue, 28 Jan 2020 08:39:42 +0800 Wei Yang <richardw.yang@linux.intel.com> wrote: > >Is there ever a situation where the cpu to be onlined would have appeared > >in the cpumask of another node and so a different kswapd's cpumask would > >now include an off-node cpu? > > No, I don't think so. > > Per my understanding, kswapd_cpu_online() will be invoked when a cpu is > onlined. And the particular cpu belongs to a particular numa node. So it is > not necessary to iterate on every nodes. > > And current code use cpumask_and_any() to do the check. If my understanding is > correct, the check would return true if this node has any online cpu. This is > likely to be true. > > This is why I want to make the logic clear. Please resend with a changelog which explains the above?
diff --git a/mm/vmscan.c b/mm/vmscan.c index 572fb17c6273..19c92d35045c 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -4049,18 +4049,19 @@ unsigned long shrink_all_memory(unsigned long nr_to_reclaim) restore their cpu bindings. */ static int kswapd_cpu_online(unsigned int cpu) { - int nid; + int nid = cpu_to_node(cpu); + pg_data_t *pgdat; + const struct cpumask *mask; - for_each_node_state(nid, N_MEMORY) { - pg_data_t *pgdat = NODE_DATA(nid); - const struct cpumask *mask; + if (!node_state(nid, N_MEMORY)) + return 0; - mask = cpumask_of_node(pgdat->node_id); + pgdat = NODE_DATA(nid); + mask = cpumask_of_node(nid); + + /* One of our CPUs online: restore mask */ + set_cpus_allowed_ptr(pgdat->kswapd, mask); - 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; }
When onlining a cpu, kswapd_cpu_online() is called to adjust kswapd cpu affinity. Current routine does like this: * Iterate all the numa node * Adjust cpu affinity when node has an online cpu Actually we could improve this by: * Just adjust the numa node to which current online cpu belongs Because we know which numa node the cpu belongs to and this cpu would not affect other node. Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> --- mm/vmscan.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-)