On 01/07/21 09:45, Srikar Dronamraju wrote:
> @@ -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;
}
