On Wed, 19 Dec 2018 at 12:58, Valentin Schneider <valentin.schnei...@arm.com> wrote: > > On 19/12/2018 08:32, Vincent Guittot wrote: > [...] > > This is another UC, asym packing is used at SMT level for now and we > > don't face this kind of problem, it has been also tested and DynamiQ > > configuration which is similar to SMT : 1 CPU per sched_group > > The legacy b.L one was not the main target although it can be > > > > The rounding error is there and should be fixed and your UC is another > > case for legacy b.L that can be treated in another patch > > > > I used that setup out of convenience for myself, but AFAICT that use-case > just stresses that issue.
After looking at you UC in details, your problem comes from the wl=1 for cpu0 whereas there is no running task. But wl!=0 without running task should not be possible since fdf5f3 ("wsched/fair: Disable LB_BIAS by default") This explain the 1023 instead of 1022 > > In the end we still have an imbalance value that can vary with only > slight group_capacity changes. And that's true even if that group > contains a single CPU, as the imbalance value computed by > check_asym_packing() can vary by +/-1 with very tiny capacity changes > (rt/IRQ pressure). That means that sometimes you're off by one and don't > do the packing because some CPU was pressured, but some other time you > might do it because that CPU was *more* pressured. It's doesn't make sense. > > > In the end problem is that in update_sg_lb_stats() we do: > > sgs->avg_load = (sgs->group_load*SCHED_CAPACITY_SCALE) / > sgs->group_capacity; > > and in check_asym_packing() we do: > > env->imbalance = DIV_ROUND_CLOSEST( > sds->busiest_stat.avg_load * sds->busiest_stat.group_capacity, > SCHED_CAPACITY_SCALE) > > So we end up with something like: > > group_load * SCHED_CAPACITY_SCALE * group_capacity > imbalance = -------------------------------------------------- > group_capacity * SCHED_CAPACITY_SCALE > > Which you'd hope to reduce down to: > > imbalance = group_load > > but you get division errors all over the place. So actually, the fix we'd > want would be: > > -----8<----- > @@ -8463,9 +8463,7 @@ static int check_asym_packing(struct lb_env *env, > struct sd_lb_stats *sds) > if (sched_asym_prefer(busiest_cpu, env->dst_cpu)) > return 0; > > - env->imbalance = DIV_ROUND_CLOSEST( > - sds->busiest_stat.avg_load * sds->busiest_stat.group_capacity, > - SCHED_CAPACITY_SCALE); > + env->imbalance = sds->busiest_stat.group_load; > > return 1; > } > ----->8-----