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

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.

Reply via email to