tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: __{read,write}_once

Le 08/11/2019 à 13:40, Mindaugas Rasiukevicius a écrit :
Maxime Villard <> 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.

Alright, thanks.

Do you think there is a performance degradation with using explicitly atomic
operations (with the "lock" prefix on x86), compared to just using an aligned
volatile which may not be exactly atomic in that sense (even if it happens to
be on x86)?

Typically in sys/uvm/uvm_fault.c there are several lockless stat increments
like the following:

	/* Increment the counters.*/

In your (not just rmind@, but tech-kern@) opinion, is it better to switch to:
or to
	__add_once(uvmexp.fltanget, 1);
as I did in my patch? Considering that the latter may possibly not be atomic
on certain arches, which could possibly result in garbage when the value is

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.

I've committed it right away, because this one seemed clear enough, feel free
to comment or improve if you have a better idea:


Home | Main Index | Thread Index | Old Index