Re: [PATCH] sched/fair: clean up asym packing

[Valentin Schneider]

Hi,

On 03/06/2019 15:17, Vincent Guittot wrote:
> Clean up asym packing to follow the default load balance behavior:
> - classify the group by creating a group_asym_capacity field.

Being nitpicky here, this doesn't classify the group in the usual way
- it doesn't get a specific group_type value (group_classify()). So maybe
"classify" isn't the best term here.

Also, why tag this group in update_sd_pick_busiest()? It would make more
sense to do so in update_sg_lb_stats() like with the other sg_lb_stats fields:

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 93c24473c8a0..537710026c3a 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8298,6 +8298,10 @@ static inline void update_sg_lb_stats(struct lb_env *env,
                }
        }
 
+       if (sgs->sum_nr_running &&
+           sched_asym_prefer(env->dst_cpu, group->asym_prefer_cpu))
+               sgs->group_asym_capacity = 1;
+
        /* Adjust by relative CPU capacity of the group */
        sgs->group_capacity = group->sgc->capacity;
        sgs->avg_load = (sgs->group_load*SCHED_CAPACITY_SCALE) / sgs->group_capacity;
@@ -8391,9 +8395,7 @@ static bool update_sd_pick_busiest(struct lb_env *env,
         * perform better since they share less core resources.  Hence when we
         * have idle threads, we want them to be the higher ones.
         */
-       if (sgs->sum_nr_running &&
-           sched_asym_prefer(env->dst_cpu, sg->asym_prefer_cpu)) {
-               sgs->group_asym_capacity = 1;
+       if (sgs->group_asym_capacity) {
                if (!sds->busiest)
                        return true;