Re: [PATCH 1/3] sched, timer: Remove usages of ACCESS_ONCE in the scheduler

Jason Low
2015-04-16T21:01:34Z

On Thu, 2015-04-16 at 20:15 +0200, Peter Zijlstra wrote:
> On Thu, Apr 16, 2015 at 08:02:27PM +0200, Ingo Molnar wrote:
> > > ACCESS_ONCE() is not a compiler barrier
> >
> > It's not a general compiler barrier (and I didn't claim so) but it is
> > still a compiler barrier: it's documented as a weak, variable specific
> > barrier in Documentation/memor-barriers.txt:
>
> Ok, fair enough. I just don't generally think of them as 'barriers'.
>
> > > The 'read' side uses ACCESS_ONCE() for two purposes:
> > > - to load the value once, we don't want the seq number to change under
> > > us for obvious reasons
> > > - to avoid load tearing and observe weird seq numbers
> > >
> > > The update side uses ACCESS_ONCE() to avoid write tearing, and
> > > strictly speaking it should also worry about read-tearing since its
> > > not hard serialized, although its very unlikely to actually have
> > > concurrency (IIRC).
>
> > This is what I meant by that there's no harm from this race.
>
> Ok, but I would still like to preserve the READ one on the usage site
> and the WRITE one on the update side, if only as documentation that
> there's something 'special' happening.

In that case, in our patch 2, I suppose we also want to use READ_ONCE()
when accessing the running field, which also helps document that we're
reading and writing to a non-atomic value that gets accessed without a
lock.

> And while the effects here might end up being statistical noise, I have
> conceptual problems with re-reading seq counts, that's not proper.
>
> And its not like they really cost anything.