Re: [PATCH -v2 12/18] sched/fair: Rewrite PELT migration propagation

[Vincent Guittot]

On 31 October 2017 at 16:01, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> 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.

Yes. I have to rebase and test lastest changes i did