Messages in this thread | | | Subject | Re: [PATCH v2 1/3] sched/fair: Add asymmetric CPU capacity wakeup scan | From | Valentin Schneider <> | Date | Fri, 24 Jan 2020 14:14:47 +0000 |
| |
(copied over from the v1 thread)
On 24/01/2020 12:59, Quentin Perret wrote: > Hey Valentin, > > On Friday 24 Jan 2020 at 12:42:53 (+0000), Valentin Schneider wrote: >> +/* >> + * Scan the asym_capacity domain for idle CPUs; pick the first idle one on which >> + * the task fits. >> + */ >> +static int select_idle_capacity(struct task_struct *p, int target) >> +{ >> + struct sched_domain *sd; >> + struct cpumask *cpus; >> + int cpu; >> + >> + if (!static_branch_unlikely(&sched_asym_cpucapacity)) >> + return -1; >> + >> + sd = rcu_dereference(per_cpu(sd_asym_cpucapacity, target)); >> + if (!sd) >> + return -1; >> + > > You might want 'sync_entity_load_avg(&p->se)' here no ? > find_idlest_cpu() and wake_cap() need one, but since we're going to use > them, you'll want to sync here too I think. >
Yeah, I think you're right, it's the exact same scenario here.
>> + cpus = this_cpu_cpumask_var_ptr(select_idle_mask); >> + cpumask_and(cpus, sched_domain_span(sd), p->cpus_ptr); >> + >> + for_each_cpu_wrap(cpu, cpus, target) { >> + if (!available_idle_cpu(cpu)) >> + continue; >> + if (!task_fits_capacity(p, capacity_of(cpu))) >> + continue; >> + >> + return cpu; > > If we found an idle CPU, but not one big enough, should we still go > ahead and choose it ? Misfit / idle balance will fix that later when a > big CPU does become available. >
If we fail to find a big enough CPU, we'll just fallback to the rest of select_idle_sibling() which will pick an idle CPU, just without caring about capacity.
Now an alternative here would be to: - return the first idle CPU on which the task fits (what the above does) - else, return the biggest idle CPU we found (this could e.g. still steer the task towards a medium on a tri-capacity system)
I think what we were trying to go with here is to not entirely hijack select_idle_sibling(). If we go with the above alternative, topologies with sched_asym_cpucapacity enabled would only ever see select_idle_capacity() and not the rest of select_idle_sibling(). Not sure if it's a bad thing or not, but it's something to ponder over.
| |