On Wed, Jun 02, 2021 at 01:50:21PM +0100, Valentin Schneider wrote:

On 27/05/21 16:38, Beata Michalska wrote:

quoted

Suggested-by: Peter Zijlstra <peterz@infradead.org>
Suggested-by: Valentin Schneider <redacted>
Signed-off-by: Beata Michalska <redacted>

I ran this through the usual series of tests ('exotic' topologies, hotplug
and exclusive cpusets), it all behaves as expected.

Thanks for that!

Tested-by: Valentin Schneider <redacted>
Reviewed-by: Valentin Schneider <redacted>

Some tiny cosmetic nits below, which don't warrant a new revision, and a
comment wrt purely symmetric systems.

quoted

---
 kernel/sched/topology.c | 194 ++++++++++++++++++++++++----------------
 1 file changed, 118 insertions(+), 76 deletions(-)

diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 55a0a243e871..77e6f79235ad 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c

quoted

+/*
+ * Verify whether there is any CPU capacity asymmetry in a given sched domain.
+ * Provides sd_flags reflecting the asymmetry scope.
+ */
+static inline int
+asym_cpu_capacity_classify(struct sched_domain *sd,
+			   const struct cpumask *cpu_map)
+{
+	struct asym_cap_data *entry;
+	int sd_asym_flags = 0;
+	int asym_cap_count = 0;
+	int asym_cap_miss = 0;
+
+	/*
+	 * Count how many unique CPU capacities this domain spans across
+	 * (compare sched_domain CPUs mask with ones representing  available
+	 * CPUs capacities). Take into account CPUs that might be offline:
+	 * skip those.
+	 */
+	list_for_each_entry(entry, &asym_cap_list, link) {
+		if (cpumask_intersects(sched_domain_span(sd),
+				       cpu_capacity_span(entry)))

IMO this is one such place where the 80 chars limit can be omitted.

quoted

+			++asym_cap_count;
+		else if (cpumask_intersects(cpu_capacity_span(entry), cpu_map))
+			++asym_cap_miss;
+	}

quoted

+/*
+ * Build-up/update list of CPUs grouped by their capacities
+ * An update requires explicit request to rebuild sched domains
+ * with state indicating CPU topology changes.
+ */
+static void asym_cpu_capacity_scan(void)
+{
+	struct asym_cap_data *entry, *next;
+	int cpu;
+
+	list_for_each_entry(entry, &asym_cap_list, link)
+		cpumask_clear(cpu_capacity_span(entry));
+
+	for_each_cpu_and(cpu, cpu_possible_mask,
+			 housekeeping_cpumask(HK_FLAG_DOMAIN))

Ditto on keeping this on a single line.

quoted

+		asym_cpu_capacity_update_data(cpu);
+
+	list_for_each_entry_safe(entry, next, &asym_cap_list, link) {
+		if (cpumask_empty(cpu_capacity_span(entry))) {
+			list_del(&entry->link);
+			kfree(entry);
+		}
+	}
+}

One "corner case" that comes to mind is systems / architectures which are
purely symmetric wrt CPU capacity. Our x86 friends might object to us
reserving a puny 24 bytes + cpumask_size() in a corner of their
memory.

Perhaps we could clear the list in the list_is_singular_case(), and since
the rest of the code only does list iteration, this should 'naturally'
cover this case:

Can do that.
I am also waiting for a reply regarding the asymmetry detected on an SMT level.
Once I get that solved, I will push new version with embedding your suggestions
as well.

Thanks for having a look!

---
BR
B.