Messages in this thread | | | Date | Thu, 23 Dec 2021 20:09:46 +0530 | From | "Gautham R. Shenoy" <> | Subject | Re: [PATCH 1/2] sched: Add per_cpu cluster domain info and cpus_share_cluster API |
| |
Hello Vincent, Barry,
On Wed, Dec 22, 2021 at 02:33:37PM +0100, Vincent Guittot wrote: > On Wed, 22 Dec 2021 at 14:14, Vincent Guittot [..snip..]
> > > > > @@ -669,6 +671,12 @@ static void update_top_cache_domain(int cpu) > > > > > per_cpu(sd_llc_id, cpu) = id; > > > > > rcu_assign_pointer(per_cpu(sd_llc_shared, cpu), sds); > > > > > > > > > > + sd = lowest_flag_domain(cpu, SD_CLUSTER); > > > > > + if (sd) > > > > > + id = cpumask_first(sched_domain_span(sd)); > > > > > + rcu_assign_pointer(per_cpu(sd_cluster, cpu), sd); > > > > > + per_cpu(sd_cluster_id, cpu) = id; > > > > > + > > > > This deserves a large comment to highlight that the new code above is > > not self contained and relies on the fact that it is done just after > > looking for LLC and sd and that id and sd must not be changed in > > between inorder to ensures that per_cpu(sd_cluster_id, cpu) equals > > per_cpu(sd_llc_id, cpu) if there is no domain with SD_CLUSTER. > > > > and per_cpu(sd_cluster_id, cpu) might not be cluster but llc > > Maybe you should not name this sd_cluster_id at all but > per_cpu(sd_share_id, cpu) = id; > > Then you have a function named > > +bool cpus_share_resources(int this_cpu, int that_cpu) > +{ > + if (this_cpu == that_cpu) > + return true; > + > + return per_cpu(sd_share_id, this_cpu) == per_cpu(sd_share_id, that_cpu); > +} > + > which returns true when cpu shares resources which can be LLC or cluster
+1. This would make it more readable than overloading the meaning of cluster itself.
-- Thanks and Regards gautham.
| |