Re: [PATCH 2/2 v2] sched: Rewrite per entity runnable load average tracking

From: Yuyang Du
Date: Wed Jul 16 2014 - 03:47:55 EST


On Tue, Jul 15, 2014 at 10:27:45AM -0700, bsegall@xxxxxxxxxx wrote:
> >
> >> > +static __always_inline u64 decay_load64(u64 val, u64 n)
> >> > +{
> >> > + if (likely(val <= UINT_MAX))
> >> > + val = decay_load(val, n);
> >> > + else {
> >> > + /*
> >> > + * LOAD_AVG_MAX can last ~500ms (=log_2(LOAD_AVG_MAX)*32ms).
> >> > + * Since we have so big runnable load_avg, it is impossible
> >> > + * load_avg has not been updated for such a long time. So
> >> > + * LOAD_AVG_MAX is enough here.
> >> > + */
> >>
> >> I mean, LOAD_AVG_MAX is irrelevant - the constant could just as well be
> >> 1<<20, or whatever, yes? In fact, if you're going to then turn it into a
> >> fraction of 1<<10, just do (with whatever temporaries you find most tasteful):
> >>
> >> val *= (u32) decay_load(1 << 10, n);
> >> val >>= 10;
> >>
> >
> > LOAD_AVG_MAX is selected on purpose. The val arriving here specifies that it is really
> > big. So the decay_load may not decay it to 0 even period n is not small. If we use 1<<10
> > here, n=10*32 will decay it to 0, but val (larger than 1<<32) can
> > survive.
>
> But when you do *1024 / LOAD_AVG_MAX it will go back to zero. In general
> the code you have now is exactly equivalent to factor = decay_load(1<<10,n)
> ignoring possible differences in rounding.
>
Oh, yes...., I did not go to that deep.

Then to avoid this, maybe should first val*factor, then val/LOAD_AVG_MAX, but then risk
overflow again... Ok, I will do:

val *= (u32) decay_load(1 << 10, n);
val >>= 10;

if not enough, I believe decay_load(1 << 15, n) should be safe too.

Thanks a lot,
Yuyang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/