Hello, Peter. On Mon, May 01, 2017 at 04:17:33PM +0200, Peter Zijlstra wrote: > So this here does: > > ( tg->load_avg = \Sum cfs_rq->load_avg ) > > load = cfs_rq->load.weight > > tg_weight = tg->load_avg - cfs_rq->contrib + load > > > tg->shares * load > shares = ----------------- > tg_weight > > > cfs_rq->load_avg > avg_shares = shares * ---------------- > load > > tg->shares * cfs_rq->load_avg > = ----------------------------- > tg_weight > > > ( se->load.weight = shares ) > > se->load_avg = min(shares, avg_shares); > > > So where shares (and se->load.weight) are an upper bound (due to using > cfs_rq->load.weight, see calc_cfs_shares), avg_shares is supposedly a > more accurate representation based on our PELT averages. > > This looks OK; and I agree with Vincent that we should use > cfs_rq->avg.load_avg, not cfs_rq->runnable_load_avg, since tg->load_avg > is a sum of the former, not the latter.
With this, we end up using a different metric for picking the busiest queue depending on whether there are nested cfs_rq's or not. The root's runnable_load_avg ends up including blocked load avgs queued behind nested cfs_rq's because we lose the resolution across threads across nesting. > Also, arguably calculating the above avg_shares directly (using the > second equation) might be more precise; but I doubt it makes much of a > difference, however since we do min(), we should at least clamp against > MIN_SHARES again. > > Furthermore, it appears to me we want a different tg_weight value for > the avg_shares, something like: > > tg_weight = tg->load_avg - cfs_rq->contrib + cfs_rq->avg.load_avg > > To better match with the numerator's units, otherwise it will have a > tendency to push avg_shares down further than it needs to be. > > > (All assuming it actually works of course.. compile tested only) So, if changing gcfs_rq se->load_avg.avg to match the gcfs_rq's runnable_load_avg is icky, and I can see why that would be, we can simply introduce a separate channel of propagation so that runnable_load_avg gets propagated independently from se->load_avg propagation, so that for all every cfs_rq, its runnable_load_avg is the sum of all active load_avgs queued on itself and its descendents, which is the number we want for load balancing anyway. I'll try to spin a patch which does that. I still wonder what gcfs_rq se->load_avg.avg is good for tho? It's nice to keep the value in line but is it actually used anywhere? The parent cfs_rq's values are independently calculated and, AFAICS, the only time the value is used is to propagate into the parent's runnable_load_sum, which has to use a different value, as explained above. Thanks. -- tejun
