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

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 06/14/2017 07:15 PM, Stefano Stabellini wrote:
> 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.
That's the goal of specification (such as AAPCS).

> > 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.
No. I mean there are places with exact same construct but sometimes 
consider we consider safe, sometimes not. We should have a clear common 
answer rather than arguing differently every time.
> > 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.
I preferred {read,write}_atomic because the prototype is nicer to use 
than ACCESS_ONCE. In the ideal we should introduce WRITE_ONCE/READ_ONCE 
improving the naming and also having a nice prototype.
--
Julien Grall

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