Re: [Xen-devel] [PATCH v11 01/34] ARM: vGIC: avoid rank lock when reading priority

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 14 Jun 2017, Julien Grall wrote:
> > > > > In any case, all those macros does not prevent re-ordering at the
> > > > > processor
> > > > > level nor read/write atomicity if the variable is misaligned.
> > > > 
> > > > My understanding is that the unwritten assumption in Xen is that
> > > > variables are always aligned. You are right about processor level
> > > > reordering, in fact when needed we have to have barriers
> > > > 
> > > > I have read Andre's well written README.atomic, and he ends the
> > > > document stating the following:
> > > > 
> > > > 
> > > > > This makes read and write accesses to ints and longs (and their
> > > > > respective
> > > > > unsigned counter parts) naturally atomic.
> > > > > However it would be beneficial to use atomic primitives anyway to
> > > > > annotate
> > > > > the code as being concurrent and to prevent silent breakage when
> > > > > changing
> > > > > the code
> > > > 
> > > > with which I completely agree
> > > 
> > > Which means you are happy to use either ACCESS_ONCE or
> > > read_atomic/write_atomic as they in-fine exactly the same on the compiler
> > > we
> > > support.
> > 
> > I do understand that both of them will produce the same output,
> > therefore, both work for this use-case.
> > 
> > I don't understand why anybody would prefer ACCESS_ONCE over
> > read/write_atomic, given that with ACCESS_ONCE as a contributor/reviewer
> > you additionally need to remember to check whether the argument is a
> > native data type. Basically, I see ACCESS_ONCE as "more work" for me.
> > Why do you think that ACCESS_ONCE is "better"?
> 
> Have you looked at the implementation of ACCESS_ONCE? You don't have to check
> the data type when using ACCESS_ONCE. There are safety check to avoid misusing
> it.

It checks for a scalar type, not for native data type. They are not
always the same thing but I think they are on arm.


> What I want to avoid is this split mind we currently have about atomic.
> They are either all safe or not.

What split mind? Do you mean ACCESS_ONCE vs. read/write_atomic? So far,
there are no instances of ACCESS_ONCE in xen/arch/arm.


> As Andre suggested, we should probably import a lighter version of
> WRITE_ONCE/READ_ONCE. They are first easier to understand than
> read_atomic/write_atomic that could be confused with atomic_read/atomic_write
> (IIRC Jan agreed here).
> 
> The main goal is to avoid assembly code when it is deem not necessary.

All right, this is one reason. Sorry if I seem unnecessarily contrarian,
but this is the first time I read a reason for this recent push for using
ACCESS_ONCE. You wrote that you preferred the read/write_atomic
functions yourself on Monday.

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