[dpdk-dev] [PATCH v3 1/3] ring: read tail using atomic load

Ola Liljedahl Ola.Liljedahl at arm.com
Fri Oct 5 16:21:09 CEST 2018



On 05/10/2018, 15:45, "Ananyev, Konstantin" <konstantin.ananyev at intel.com> wrote:

    We all know that 32bit load/store on cpu we support - are atomic.
Well, not necessarily true for unaligned loads and stores. But the "problem" here is that we are not directly generating 32-bit load and store instructions (that would require inline assembly), we are performing C-level reads and writes and trust the compiler to generate the machine instructions.

    If it wouldn't be the case - DPDK would be broken in dozen places.
And maybe it is, if you compile for a new architecture or with a new compiler (which are getting more and more aggressive with regards to utilising e.g. undefined behavior in order to optimise the generated code).

    So what the point to pretend that "it might be not atomic" if we do know for sure that it is?
Any argument that includes the words "for sure" is surely suspect.

    I do understand that you want to use atomic_load(relaxed) here for consistency,
    and to conform with C11 mem-model and I don't see any harm in that.
    But argument that we shouldn't assume 32bit load/store ops as atomic sounds a bit flaky to me.
    Konstantin
I prefer to declare intent and requirements to the compiler, not to depend on assumptions even if I can be reasonably sure my assumptions are correct right here right now. Compilers will find new ways to break non-compliant code if that means it can improve the execution time of compliant code.

Someone may modify the code or follow the model for some similar thing but change that 32-bit variable to a 64-bit variable. Now for a 32-bit target, the plain C read from a volatile 64-bit variable will not be atomic but there will be no warning from the compiler, it will happily generate a sequence of non-atomic loads. If you instead use __atomic_load_n() to read the variable, you would get a compiler or linker error (unless you link with -latomic which will actually provide an atomic load, e.g. by using a lock to protect the access).




More information about the dev mailing list