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

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



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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.