Messages in this thread | | | From | Valentin Schneider <> | Subject | Re: [PATCH v2 5/7] x86/sched: Remove SD_ASYM_PACKING from the "SMT" domain | Date | Tue, 10 Jan 2023 19:17:51 +0000 |
| |
On 29/12/22 11:02, Ricardo Neri wrote: > On Thu, Dec 22, 2022 at 04:56:51PM +0000, Valentin Schneider wrote: >> diff --git a/include/linux/sched/sd_flags.h b/include/linux/sched/sd_flags.h >> index 57bde66d95f7a..8dc16942135b4 100644 >> --- a/include/linux/sched/sd_flags.h >> +++ b/include/linux/sched/sd_flags.h >> @@ -132,12 +132,12 @@ SD_FLAG(SD_SERIALIZE, SDF_SHARED_PARENT | SDF_NEEDS_GROUPS) >> /* >> * Place busy tasks earlier in the domain >> * >> - * SHARED_CHILD: Usually set on the SMT level. Technically could be set further >> - * up, but currently assumed to be set from the base domain >> - * upwards (see update_top_cache_domain()). >> + * SHARED_PARENT: Usually set on the SMT level. Can be set further up if all >> + * siblings of an SMT core are identical, but SMT cores themselves >> + * have different priorites. >> * NEEDS_GROUPS: Load balancing flag. >> */ >> -SD_FLAG(SD_ASYM_PACKING, SDF_SHARED_CHILD | SDF_NEEDS_GROUPS) >> +SD_FLAG(SD_ASYM_PACKING, SDF_SHARED_PARENT | SDF_NEEDS_GROUPS) > > But this would not work for Power7. It only has SD_ASYM_PACKING in the SMT > sched domain. Must it have either of these flags? >
It's not mandatory, but making sure SD flags conform to either of them means the topology debugging infra can help spot misshapen topologies...
> In Power7 SMT siblings have the different priority but, IIUC, physical > cores are identical. >
...But you're right, this doesn't work with Power7 as it would need SD_ASYM_PACKING all the way up the topology to conform with SDF_SHARED_PARENT, which clearly doesn't work with how Power7 uses asym_packing.
> It seems to me that asym_packing is specific to a domain. >
For Power7 it is, since the asymmetry is only between siblings of a given core. For other systems where the asymmetry is between cores, that could theoretically affect several levels. Consider:
DIE [ ] MC [ ][ ] SMT [ ][ ][ ][ ] CPU 0 1 2 3 4 5 6 7 prio 3 3 2 2 1 1 0 0
As done in your patch, here asym_packing doesn't make sense for SMT, but it does for MC and DIE.
Anywho, I think what this means if we should drop the SDF_SHARED_* metaflag for SD_ASYM_PACKING, unless we can think of a nice way to programmatically describe how SD_ASYM_PACKING should be set.
> Thanks and BR, > Ricardo
| |