On Tue, Oct 31, 2017 at 12:14:11PM +0100, Vincent Guittot wrote:

> > +       if (runnable_sum >= 0) {
> > +               /*
> > +                * Add runnable; clip at LOAD_AVG_MAX. Reflects that until
> > +                * the CPU is saturated running == runnable.
> > +                */
> > +               runnable_sum += se->avg.load_sum;
> > +               runnable_sum = min(runnable_sum, (long)LOAD_AVG_MAX);
> > +       } else {
> > +               /*
> > +                * Estimate the departing task's runnable by assuming all tasks
> > +                * are equally runnable.
> > +                *
> > +                * XXX: doesn't deal with multiple departures?
> 
> Why this would not deal with multiple departures ?
> we are using gcfs_rq->avg.load_sum that reflects the new state of the
> gcfs_rq to evaluate the runnable_sum

Ah, I figured the load_sum thing below reflected one average task worth
of runnable.


> > +       /* runnable_sum can't be lower than running_sum */
> > +       running_sum = se->avg.util_sum >> SCHED_CAPACITY_SHIFT; /* XXX ? */
> 
> running_sum is scaled by cpu's capacity but not load_sum
> 
> I have made the shortcut of using SCHED_CAPACITY_SHIFT for capacity
> but we might better use arch_scale_cpu_capacity(NULL, cpu) instead

Ah, right. We should improve the comments thereabouts, I got totally
lost trying to track that yesterday.

Also; we should look at doing that invariant patch you're still sitting
on.