Messages in this thread | | | Date | Wed, 21 Dec 2022 18:12:52 +0100 | Subject | Re: [PATCH v2 3/7] sched: Teach arch_asym_cpu_priority() the idle state of SMT siblings | From | Dietmar Eggemann <> |
| |
On 12/12/2022 18:54, Ricardo Neri wrote: > On Tue, Dec 06, 2022 at 06:54:39PM +0100, Dietmar Eggemann wrote: >> On 22/11/2022 21:35, Ricardo Neri wrote:
[...]
>>> + * want to check the idle state of the SMT siblngs of @cpu. >> >> s/siblngs/siblings >> >> The scheduler calls sched_asym_prefer(..., true) from >> find_busiest_queue(), asym_active_balance() and nohz_balancer_kick() > > In these places we are comparing two specific CPUs, of which the idle > state of its siblings impact their throughput and, in consequence, the > decision of attempt to balance load. > > In the places were sched_asym_prefer(...., false) is called we compare the > destination CPU with a CPU that bears the priority of a sched group, > regardless of the idle state of its siblings.
OK.
>> even from SMT layer on !x86. > > This is true, but the default arch_asym_cpu_priority ignores check_smt.
True.
>> So I guess a `bool check_smt` wouldn't be >> sufficient to distinguish whether sched_smt_siblings_idle() should be >> called or not. > > But it is the caller who determines whether the idle state of the SMT > siblings of @cpu may be relevant.
I assume caller being the task scheduler here. Callers with `check_smt=true` can be called from any SD level with SD_ASYM_PACKING.
Imagine an arch w/ SD_ASYM_PACKING on SMT & MC overwriting arch_asym_cpu_priority(). `bool check_smt` wouldn't be sufficient to know whether a call to something like sched_smt_siblings_idle() (is_core_idle()) which iterates over cpu_smt_mask(cpu) would make sense.
>> To me this comment is a little bit misleading. Not an >> issue currently since there is only the x86 overwrite right now. > > If my justification make sense to you, I can expand the comment to state > that the caller decides whether check_smt is needed but arch-specific > implementations are free to ignore it.
Not a big issue but to me if the task scheduler asks for `bool check_smt` then archs would have to check to guarantee common behaviour. And the meaning of `bool check_smt` on SMT is unclear to me. Since only x86 would use this so far it can be adapted later for others if needed.
[...]
| |