Re: __{read,write}_once

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

Maxime Villard <max%m00nbsd.net@localhost> wrote:
> > - If we do not want to stick with the C11 API (its emulation), then I
> > would suggest to use the similar terminology, e.g. atomic_load_relaxed()
> > and atomic_store_relaxed(), or Linux READ_ONCE()/WRITE_ONCE().  There is
> > no reason invent new terminology; it might just add more confusion.
> 
> But... Linux's READ_ONCE/WRITE_ONCE are not actually atomic, is that
> correct? So there is a significant semantic difference.

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).

> Also, in my original patch I marked two branches of subr_xcall.c, but it
> seems to me now they are actually races: ie xc_wait(), the read of
> 'xc_donep' could be made by two 4-byte fetches on 32bit arches (at
> least). Between the two, an xc thread could have updated the value. So
> there is an actual race which could possibly result in returning early
> while we shouldn't have. Does that look correct to you?

Correct.

There is more code in the NetBSD kernel which needs fixing.  I would say
pretty much all lock-free code should be audited.

Back in time, certain liberties compilers have were not fully understood or
were not considered as real problems (after all, C memory model concerning
many of these aspects was not defined).  Times changed, compilers became
much more aggressive and the old code needs to catch up.

-- 
Mindaugas