Re: [Xen-devel] read_atomic, write_atomic, add_sized

[Andre Przywara <andre.przywara@xxxxxxx>]

Hi,

On 13/06/17 14:11, Jan Beulich wrote:
>>>> On 13.06.17 at 15:02, <julien.grall@xxxxxxx> wrote:
>> On 13/06/17 13:48, Jan Beulich wrote:
>>>>>> On 13.06.17 at 11:42, <julien.grall@xxxxxxx> wrote:
>>>> Looking at the usage on Linux, many places is using
>>>> ACCESS_ONCE/WRITE_ONCE/READ_ONCE assume this will be done in a single
>>>> instructions (see atomic_read/atomic_write on both x86 and ARM).
>>>>
>>>> My understanding is, at least on ARM GCC (?), assignment will be atomic
>>>> (though will not prevent the compiler to only write/read once). We make
>>>> this assumption in quite a few places (such as PV protocol for the
>>>> index) and I am not sure to understand why it cannot be done in other
>>>> places...
>>>
>>> To a certain degree we can / want to assume compilers don't do
>>> insane things. I.e. why would they commonly emit multiple insns
>>> if a single suffices. However, we would better write our code
>>> such that we don't _depend_ on this, the more that I've explained
>>> (or was that on a different thread) that e.g. read-modify-write
>>> operations on memory operands commonly get carried out by the
>>> compiler by 3 insns even though most of the time it could be done
>>> by just one.
>>
>> That's from an x86 POV. ARM (at least until ARMv8.0) does not have 
>> read-modify-write operations. If the compiler is not using them on x86 
>> then maybe you should look at fixing that in the compiler?
>>
>> However, looking at the patch that introduced add_sized, the intention 
>> was not about preventing the compiler to use 3 insns:
> 
> Sure - I've only given this as an example to demonstrate that we
> can't always assume the compiler does exactly what we want it to.
> 
>>> And then, as the cited code still visible above shows, atomicity is
>>> not going to be guaranteed for most of the access sizes anyway.
>>> How can you imply that the given constructs are safe to use as
>>> atomic accesses for some variables, but not for others? Such a
>>> property ought to be universal (and the build should fail if it is
>>> being violated). Otherwise a simple type change of some
>>> variable may render the code buggy possibly without anyone
>>> noticing.
>>
>> It is a choice from Linux side to allow using READ_ONCE/WRITE_ONCE with 
>> bigger size. I am not suggesting to do the same on Xen but avoiding the 
>> bigger size.
> 
> But Andre had raised that very question.

TBH I don't really want to have that functionality, as I don't believe
there would be a user right now. I just wanted to point out that be
deviating from the Linux implementation we might find the situation
different in Xen, so reasoning which would be valid for Linux might not
apply to Xen anymore - so it's up to us to define the meaning.

But actually confining ACCESS_ONCE to native data types gives us
additional atomicity guarantees for most practical cases, so I'd rather
keep it that way.
Yes, by strictly following the letters of the C standard this may still
be violated, but I think we all agree that going there would be an
academic and pointless exercise, as there are more compiler restrictions
anyway and more code which assumes a sane compiler already.

To summarize the issue and to document our assumptions here (and also to
make sure my understanding is right as well ;-) I just crafted a
README.atomic file, which should describe the atomic primitives and
their expectations and limitations. I think having such a text in the
code tree would be beneficial for documentation purposes.

I will send it as an RFC ASAP (and am happy to make this a subject of
heavy discussions).

Cheers,
Andre.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel