Re: [RFC][PATCH] sched: attach extra runtime to the right avg

[Ingo Molnar <mingo@kernel.org>]

* Peter Zijlstra <peterz@infradead.org> wrote:

> > An intermediate approach to improve that skew would be something like below. 
> > It doesn't track the remainder like your patch does, but doesn't lose 
> > precision either, just rounds down 'now' to the nearest 1024 boundary.
> 
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 008c514dc241..b03703cd7989 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -2965,7 +2965,7 @@ ___update_load_avg(u64 now, int cpu, struct sched_avg *sa,
> >  	if (!delta)
> >  		return 0;
> >  
> > -	sa->last_update_time += delta << 10;
> > +	sa->last_update_time = now & ~1023ULL;
> >  
> 
> So if we have a task that always runs <1024ns it should still get blocks
> of runtime because the difference between now and now&~1023 can be !0
> and spill.

Agreed - in the first approximation I was trying to figure out why Josef was 
seeing an effect from the patch.

> I'm just not immediately seeing how its different from the 0-sum we had.
> It should be identical since delta*1024 would equally land us on those
> same edges (there's an offset in the differential form between the two,
> but since we start with last_update_time=0, the resulting edges are the
> same afaict).

So I think the difference is that this:

	sa->last_update_time = now & ~1023ULL;

is tracking the absolute value of 'now' (i.e. rq->clock in most cases) by and 
large, with a 1024 ns imprecision.

This code on the other hand:

	sa->last_update_time += delta << 10;

... in essence creates a whole new absolute clock value that slowly but surely is 
drifting away from the real rq->clock, because 'delta' is always rounded down to 
the nearest 1024 ns boundary, so we accumulate the 'remainder' losses.

That is because:

        delta >>= 10;
	...
        sa->last_update_time += delta << 10;

Given enough time, ->last_update_time can drift a long way, and this delta:

	delta = now - sa->last_update_time;

... becomes meaningless AFAICS, because it's essentially two different clocks that 
get compared.

But I might be super confused about this myself ...

Thanks,

	Ingo