Re: [RFC][PATCH] sched: attach extra runtime to the right avg
From: Ingo Molnar
Date: Tue Jul 04 2017 - 06:13:17 EST
* Peter Zijlstra <peterz@xxxxxxxxxxxxx> 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