sched: update_top_cache_domain only at the times of building sched domain.

Currently, update_top_cache_domain() is called whenever schedule domain is built or destroyed. But, the following
callpath shows that they're at the same callpath and can be avoided update_top_cache_domain() while destroying schedule domain and update only at the times of building schedule domains.

partition_sched_domains()
detach_destroy_domain()
cpu_attach_domain()
update_top_cache_domain()
build_sched_domains()
cpu_attach_domain()
update_top_cache_domain()

Signed-off-by Rakib Mullick <[email protected]>

---

kernel/sched/core.c +5 -4

index b7c32cb..8c6fee4 100644
@@ -5102,8 +5102,8 @@ static void update_top_cache_domain(int cpu)
  * Attach the domain 'sd' to 'cpu' as its base domain. Callers must
  * hold the hotplug lock.
  */
-static void
-cpu_attach_domain(struct sched_domain *sd, struct root_domain *rd, int cpu)
+static void cpu_attach_domain(struct sched_domain *sd, struct root_domain *rd,
+				int cpu, bool update_cache_domain)
 {
 	struct rq *rq = cpu_rq(cpu);
 	struct sched_domain *tmp;

@@ -5138,7 +5138,8 @@ cpu_attach_domain(struct sched_domain *sd, struct root_domain *rd, int cpu)
 	rcu_assign_pointer(rq->sd, sd);
 	destroy_sched_domains(tmp, cpu);
 
-	update_top_cache_domain(cpu);
+	if (update_cache_domain)
+		update_top_cache_domain(cpu);
 }
 
 /* cpus with isolated domains */

@@ -6021,7 +6022,7 @@ static int build_sched_domains(const struct cpumask *cpu_map,
 	rcu_read_lock();
 	for_each_cpu(i, cpu_map) {
 		sd = *per_cpu_ptr(d.sd, i);
-		cpu_attach_domain(sd, d.rd, i);
+		cpu_attach_domain(sd, d.rd, i, 1);
 	}
 	rcu_read_unlock();
 

Quoted 4 lines · Rakib Mullick

Currently, update_top_cache_domain() is called whenever schedule domain is built or destroyed. But, the following
callpath shows that they're at the same callpath and can be avoided update_top_cache_domain() while destroying schedule domain and update only at the times of building schedule domains.

Quoted 8 lines

partition_sched_domains()
detach_destroy_domain()
cpu_attach_domain()
update_top_cache_domain()
build_sched_domains()
cpu_attach_domain()
update_top_cache_domain()

Does it really matter?

This just makes the code uglier for no gain afaict.

If you really need to do this, key off @sd == NULL or something.

Quoted 14 lines · Peter Zijlstra

Quoted 4 lines · Rakib Mullick

Currently, update_top_cache_domain() is called whenever schedule domain is built or destroyed. But, the following
callpath shows that they're at the same callpath and can be avoided update_top_cache_domain() while destroying schedule domain and update only at the times of building schedule domains.

Quoted 8 lines

partition_sched_domains()
detach_destroy_domain()
cpu_attach_domain()
update_top_cache_domain()
build_sched_domains()
cpu_attach_domain()
update_top_cache_domain()

Quoted 1 line

Does it really matter?

Why should we do it twice? More importantly at the time of destroying even though we know it'll be built again just few moment later.

Quoted 2 lines

Quoted 3 lines

This just makes the code uglier for no gain afaict.

If you really need to do this, key off @sd == NULL or something.

Sorry, would you please a bit more clearer? I can't pick what you're suggesting.

You mean using sd == NULL rather than using update_cache_domain variable ?

Quoted 1 line · Rakib Mullick

You mean using sd == NULL rather than using update_cache_domain variable ?

Yes, note how:

kernel/sched/core.c +0 -1

@@ -6109,7 +6110,7 @@ static void detach_destroy_domains(const struct cpumask *cpu_map)
 
        rcu_read_lock();
        for_each_cpu(i, cpu_map)
-               cpu_attach_domain(NULL, &def_root_domain, i);
        rcu_read_unlock();
 }
 

Always has NULL for sd? Which means you can do: