Re: [patch 3/3] sched: move sched_avg_update() to update_cpu_load()

[Suresh Siddha <suresh.b.siddha@intel.com>]

On Mon, 2010-08-16 at 01:00 -0700, Peter Zijlstra wrote:
> On Fri, 2010-08-13 at 12:45 -0700, Suresh Siddha wrote:
> > plain text document attachment (move_sched_avg_update.patch)
> > Currently sched_avg_update() (which updates rt_avg stats in the rq) is getting
> > called from scale_rt_power() (in the load balance context) which doesn't take
> > rq->lock.
> 
> Right, but I think it doesn't need to, as its never accessed cross CPU
> (except maybe for /proc/sched_debug, and who cares about that).
> 
> The (other) odd case is NO_HZ idle balancing, but there the actual CPU
> won't be updating the fields, so the remote CPU doing NO_HZ idle
> balancing should be good to touch it.

There is no guarantee that the original cpu won't be doing this in
parallel with nohz idle load balancing cpu.

> 
> > Fix it by moving the sched_avg_update() to more appropriate update_cpu_load()
> > where the CFS load gets updated aswell.
> 
> Right, except it breaks things a bit, at the very least you really need
> that update right before reading it, otherwise you can end up with >100%
> fractions, which are odd indeed ;-)

with the patch, the update always happens before reading it. isn't it?

update now happens during the scheduler tick (or during nohz load
balancing tick). And the load balancer gets triggered with the tick.
So the update (at the tick) should happen before reading it (used by
load balancing triggered by the tick). Am I missing something?

thanks,
suresh