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

[Yuyang Du <yuyang.du@intel.com>]

On Tue, Jul 15, 2014 at 10:27:45AM -0700, bsegall@google.com 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