Re: [PATCH v2 1/2] sched/topology: Skip updating masks for non-online nodes
From: Valentin Schneider <hidden>
Date: 2021-07-01 14:28:54
Also in:
lkml
Subsystem:
scheduler, the rest · Maintainers:
Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot, Linus Torvalds
On 01/07/21 09:45, Srikar Dronamraju wrote:
quoted hunk ↗ jump to hunk
@@ -1891,12 +1894,30 @@ void sched_init_numa(void) void sched_domains_numa_masks_set(unsigned int cpu) { int node = cpu_to_node(cpu); - int i, j; + int i, j, empty; + empty = cpumask_empty(sched_domains_numa_masks[0][node]); for (i = 0; i < sched_domains_numa_levels; i++) { for (j = 0; j < nr_node_ids; j++) { - if (node_distance(j, node) <= sched_domains_numa_distance[i]) + if (!node_online(j)) + continue; + + if (node_distance(j, node) <= sched_domains_numa_distance[i]) { cpumask_set_cpu(cpu, sched_domains_numa_masks[i][j]); + + /* + * We skip updating numa_masks for offline + * nodes. However now that the node is + * finally online, CPUs that were added + * earlier, should now be accommodated into + * newly oneline node's numa mask. + */ + if (node != j && empty) { + cpumask_or(sched_domains_numa_masks[i][node], + sched_domains_numa_masks[i][node], + sched_domains_numa_masks[0][j]); + } + }
Hmph, so we're playing games with masks of offline nodes - is that really necessary? Your modification of sched_init_numa() still scans all of the nodes (regardless of their online status) to build the distance map, and that is never updated (sched_init_numa() is pretty much an __init function). So AFAICT this is all to cope with topology_span_sane() not applying 'cpu_map' to its masks. That seemed fine to me back when I wrote it, but in light of having bogus distance values for offline nodes, not so much... What about the below instead? ---
diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index b77ad49dc14f..c2d9caad4aa6 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c@@ -2075,6 +2075,7 @@ static struct sched_domain *build_sched_domain(struct sched_domain_topology_leve static bool topology_span_sane(struct sched_domain_topology_level *tl, const struct cpumask *cpu_map, int cpu) { + struct cpumask *intersect = sched_domains_tmpmask; int i; /* NUMA levels are allowed to overlap */
@@ -2090,14 +2091,17 @@ static bool topology_span_sane(struct sched_domain_topology_level *tl, for_each_cpu(i, cpu_map) { if (i == cpu) continue; + /* - * We should 'and' all those masks with 'cpu_map' to exactly - * match the topology we're about to build, but that can only - * remove CPUs, which only lessens our ability to detect - * overlaps + * We shouldn't have to bother with cpu_map here, unfortunately + * some architectures (powerpc says hello) have to deal with + * offline NUMA nodes reporting bogus distance values. This can + * lead to funky NODE domain spans, but since those are offline + * we can mask them out. */ + cpumask_and(intersect, tl->mask(cpu), tl->mask(i)); if (!cpumask_equal(tl->mask(cpu), tl->mask(i)) && - cpumask_intersects(tl->mask(cpu), tl->mask(i))) + cpumask_intersects(intersect, cpu_map)) return false; }