[RFC 3/4] sched: fix computed capacity for HMP

[Vincent Guittot]

On 15 April 2014 19:22, Peter Zijlstra <peterz at infradead.org> wrote:
> On Fri, Mar 28, 2014 at 02:22:28PM +0100, Vincent Guittot wrote:
>> The current sg_capacity solves the ghost cores issue for SMT system and
>> cluster made of big cores which have a cpu_power above SCHED_POWER_SCALE at
>> core level. But it still removes some real cores of a cluster made of LITTLE
>> cores which have a cpu_power below SCHED_POWER_SCALE.
>>
>> Instead of using the power_orig to detect SMT system and compute a smt factor
>> that will be used to calculate the real number of cores, we set a core_fct
>> field when building the sched_domain topology. We can detect SMT system thanks
>> to SD_SHARE_CPUPOWER flag and set core_fct to know how many CPUs per core we
>> have. The core_fct will ensure that sg_capacity will return cores capacity of
>> a SMT system and will not remove any real core of LITTLE cluster.
>>
>> This method also fixes a use case where the capacity of a SMT system was
>> overrated.
>> Let take the example of a system made of 8 cores HT system:
>> At CPU level, sg_capacity is cap to a maximum capacity of 8 whereas
>> DIV_ROUND_CLOSEST(power, SCHED_POWER_SCALE) returns 9.
>> ((589*16) / 1024) = 9.3
>> Now if 2 CPUs (1 core) are fully loaded by rt tasks, sg_capacity still returns
>> a capacity of 8 whereas it should return a capacity of 7. This happen because
>> DIV_ROUND_CLOSEST(power, SCHED_POWER_SCALE) is still above 7.5:
>> ((589*14) / 1024) = 8.05
>>
>> Signed-off-by: Vincent Guittot <vincent.guittot at linaro.org>
>> ---
>>  kernel/sched/core.c  | 7 +++++++
>>  kernel/sched/fair.c  | 6 ++----
>>  kernel/sched/sched.h | 2 +-
>>  3 files changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>> index f9d9776..5b20b27 100644
>> --- a/kernel/sched/core.c
>> +++ b/kernel/sched/core.c
>> @@ -5844,6 +5844,13 @@ static void init_sched_groups_power(int cpu, struct sched_domain *sd)
>>
>>       WARN_ON(!sg);
>>
>> +     if (!sd->child)
>> +             sg->core_fct = 1;
>> +     else if (sd->child->flags & SD_SHARE_CPUPOWER)
>> +             sg->core_fct = cpumask_weight(sched_group_cpus(sg));
>> +     else
>> +             sg->core_fct = sd->child->groups->core_fct;
>> +
>>       do {
>>               sg->group_weight = cpumask_weight(sched_group_cpus(sg));
>>               sg = sg->next;
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index ed42061..7387c05 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -5773,12 +5773,10 @@ static inline int sg_capacity(struct lb_env *env, struct sched_group *group)
>>       power = group->sgp->power;
>>       power_orig = group->sgp->power_orig;
>>       cpus = group->group_weight;
>> +     smt = group->core_fct;
>>
>> -     /* smt := ceil(cpus / power), assumes: 1 < smt_power < 2 */
>> -     smt = DIV_ROUND_UP(SCHED_POWER_SCALE * cpus, power_orig);
>> -     capacity = cpus / smt; /* cores */
>> +     capacity = DIV_ROUND_CLOSEST(power * cpus, power_orig * smt);
>>
>> -     capacity = min_t(unsigned, capacity, DIV_ROUND_CLOSEST(power, SCHED_POWER_SCALE));
>>       if (!capacity)
>>               capacity = fix_small_capacity(env->sd, group);
>>
>
> So this patch only cures a little problem; the much bigger problem is
> that capacity as exists is completely wrong.
>
> We really should be using utilization here. Until a CPU is fully

ok, I'm goiing to see how to replace capacity with utilization

Thanks

> utilized we shouldn't be moving tasks around (unless packing, but where
> not there yet, and in that case you want to stop, where this starts,
> namely full utilization).
>
> So while I appreciate what you're trying to 'fix' here, its really just
> trying to dress a monkey.