On Thu, Feb 08, 2018 at 04:05:58PM +0100, Vincent Guittot wrote: > On 8 February 2018 at 15:00, Peter Zijlstra <pet...@infradead.org> wrote: > > On Tue, Feb 06, 2018 at 08:23:05PM +0100, Vincent Guittot wrote: > > > >> @@ -9207,13 +9231,15 @@ void nohz_balance_enter_idle(int cpu) > >> if (!housekeeping_cpu(cpu, HK_FLAG_SCHED)) > >> return; > >> > >> + rq->has_blocked_load = 1;
Should we not set that with rq->lock held? We already clear it while holding rq->lock. > >> + > >> if (rq->nohz_tick_stopped) > >> - return; > > > > this case is difficult... needs thinking > > The use case happens when a CPU wakes up and goes back to idle before > the tick fires and clears nohz_tick_stopped. Yes, and so we could have accrued blocked load. Now in this case the CPU must already be set in the cpumask, but we could've already cleared has_blocked. My question is mostly about needing that "goto out" to set the flag, because I think we can loose it on a store collision vs clearing it. But in that case I suppose the iteration must already be in progress, which in turn will observe rq->has_blocked_load and re-set nohz.has_blocked. So yes, this is good, but could use a comment. > > Without this ordering I think it would be possible to loose has_blocked > > and not observe the CPU either. > > I think that you are right. > I also wondered if some barriers were necessary but wrongly concluded > that set operation on nohz.idle_cpus_mask and WRITE_ONCE with volatile > would be enough to ensure the right ordering Yeah, so I forgot to write the comment in my patch, but it had the barriers implied by cmpxchg.