Re: [PATCH v5 2/3] sched/topology: Rework CPU capacity asymmetry detection
From: Beata Michalska <hidden>
Date: 2021-06-02 19:48:20
Also in:
lkml
Subsystem:
scheduler, the rest · Maintainers:
Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot, Linus Torvalds
On Wed, Jun 02, 2021 at 07:17:12PM +0200, Dietmar Eggemann wrote:
quoted hunk ↗ jump to hunk
On 27/05/2021 19:07, Beata Michalska wrote:quoted
On Thu, May 27, 2021 at 05:08:42PM +0200, Dietmar Eggemann wrote:quoted
On 26/05/2021 23:40, Beata Michalska wrote:quoted
On Wed, May 26, 2021 at 08:17:41PM +0200, Dietmar Eggemann wrote:quoted
On 26/05/2021 14:51, Beata Michalska wrote:quoted
On Wed, May 26, 2021 at 01:15:46PM +0100, Beata Michalska wrote:quoted
On Wed, May 26, 2021 at 11:52:25AM +0200, Dietmar Eggemann wrote:quoted
On 25/05/2021 12:29, Beata Michalska wrote:quoted
On Tue, May 25, 2021 at 10:53:07AM +0100, Valentin Schneider wrote:quoted
On 24/05/21 23:55, Beata Michalska wrote:quoted
On Mon, May 24, 2021 at 07:01:04PM +0100, Valentin Schneider wrote:quoted
On 24/05/21 11:16, Beata Michalska wrote:[...]quoted
quoted
quoted
We could possibly add a warning (like in EAS) if the asymmetry is detected for SMT which would give some indication that there is smth ... wrong ?Maybe, in case you find an easy way to detect this. But the issue already exists today. Not with the topology mentioned above but in case we slightly change it to: cpus = { ([446 1024] [871 1024] [446 1024] ) ([1024 1024]) } ^^^^ so that we have a 1024 CPU in the lowest sd for each CPU, we would get SD_ASYM_CPUCAPACITY on SMT.The asymmetry capacity flags are being set on a sched domain level, so we could use the SD_SHARE_CPUCAPACITY|SD_SHARE_PKG_RESOURCES (cpu_smt_flags) flags to determine if having asymmetry is valid or not ? If this is enough this could be handled by the classify function?Or maybe something directly in sd_init(), like the WARN_ONCE() which triggers if somebody wants to sneak in a ~topology flag via a sched_domain_topology_level table? IMHO checking `SD_SHARE_CPUCAPACITY | SD_ASYM_CPUCAPACITY` will be sufficient here.diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c index 62d412013df8..77b73abbb9a4 100644 --- a/kernel/sched/topology.c +++ b/kernel/sched/topology.c@@ -1561,6 +1561,11 @@ sd_init(struct sched_domain_topology_level *tl, sd_id = cpumask_first(sched_domain_span(sd)); sd->flags |= asym_cpu_capacity_classify(sd, cpu_map); + + WARN_ONCE((sd->flags & (SD_SHARE_CPUCAPACITY | SD_ASYM_CPUCAPACITY)) == + (SD_SHARE_CPUCAPACITY | SD_ASYM_CPUCAPACITY), + "CPU capacity asymmetry not supported on SMT\n"); + /* * Convert topological properties into behaviour. */In case we can agree on something simple here I guess you can incorporate it into v7.
So what I have done is :
diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 77e6f79235ad..ec4ae225687e 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c@@ -1324,6 +1324,7 @@ asym_cpu_capacity_classify(struct sched_domain *sd, if (!asym_cap_miss) sd_asym_flags |= SD_ASYM_CPUCAPACITY_FULL; + WARN_ONCE(cpu_smt_flags() & sd->flags, "Detected CPU capacity asymmetry on SMT level"); leave: return sd_asym_flags; }
Comment can be adjusted. This would sit in the classify function to nicely wrap asymmetry bits in one place. What do you think ? --- BR B.