Re: __{read,write}_once

[Mindaugas Rasiukevicius <rmind%netbsd.org@localhost>]

Maxime Villard <max%m00nbsd.net@localhost> wrote:
> > They are "atomic" in a sense that they prevent from tearing, fusing and
> > invented loads/stores.  Terms and conditions apply (e.g. they assume
> > properly aligned and word-sized accesses).  Additionally, READ_ONCE()
> > provides a data-dependency barrier, applicable only to DEC Alpha.  I
> > think it was the right decision on the Linux side (even though trying
> > to get all the data-dependency barriers right these days is kind of a
> > lost cause, IMO).
> > 
> > So, READ_ONCE()/WRITE_ONCE() is more or less equivalent to the C11
> > atomic load/stores routines with memory_order_relaxed (or
> > memory_order_consume, if you care about DEC Alpha).
> 
> But... Didn't Marco just say that 'volatile' accesses do not actually
> prevent tearing/fusing/invented loads/stores? READ_ONCE/WRITE_ONCE only
> do volatile accesses.

Let me try to clarify:

- The main purpose of READ_ONCE()/WRITE_ONCE() is to provide a way to
perform atomic loads/stores (in a sense of preventing from the said
behaviours), even though they help to get the memory ordering right too.
Currently, 'volatile' is a key instrument in achieving that.  However,
as stated before, terms and conditions apply: 'volatile' just in itself
does not provide the guarantee; the loads/stores also have to be properly
aligned and word-sized (these are the pre-C11 assumptions we always had).
Note: C11 introduces atomic _types_, so that the compiler could leverage
the type system and thus provide the necessary guarantees.

- Having re-read Marco's emails in this thread, I think we are very much
in agreement.  I think he merely points out that 'volatile' in itself is
not sufficient; it does not mean it's not necessary.

- There is quite a bit of confusion regarding 'volatile' amongst the
developers.  This is partly because 'volatile' is arguably underspecified
in the C standard.  AFAIK, some people in the C standardization committee
have a view that it provides weaker guarantees; however, others maintain
that the intent has always been clear and the wording is sufficient.
Without going into the details (somewhat philosophical anyway), at least
for now, 'volatile' is a de facto ingredient (one of a few, but absolutely
necessary) in achieving atomic loads/stores.

Sorry if this is a bit repetitive, but I hope it gets the point across.

> 
> To fix that, do you agree that I should
>   - Remove the first branch (because no lockless fastpath possible)
>   - Move down the second branch (KASSERT) right after the mutex_enter
> ?

Feel free to write a patch and I'll have a look at it once I have a
little bit more free time.

> 
> > There is more code in the NetBSD kernel which needs fixing.  I would say
> > pretty much all lock-free code should be audited.
> 
> I believe KCSAN can greatly help with that, since it automatically reports
> concurrent accesses. Up to us then to switch to atomic, or other kinds of
> markers like READ_ONCE.

Is there a CSAN i.e. such sanitizer for userspace applications?

Thanks.

-- 
Mindaugas