[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] read_atomic, write_atomic, add_sized
Hi, On 12/06/17 11:55, Jan Beulich wrote: >>>> On 12.06.17 at 12:24, <julien.grall@xxxxxxx> wrote: >> I am trying to understand why we decided to implement the helpers >> read_atomic, write_atomic, add_sized in assembly rather than directly in C. >> >> AFAICT implementation in C similar to Linux helpers WRITE_ONCE/READ_ONCE >> would work here. Did I miss anything? > > For one at least our current ACCESS_ONCE() doesn't allow non- > scalar types to be read/written, whereas {read,write}_atomic() > solely look at sizeof(). What is the rationale behind this? I would assume the ACCESS_ONCE() does not care about atomicity, it is basically a pretty version of a volatile cast to tell the compiler to cache the value (and not re-read at will). And so it should be able to read an arbitrarily sized data structure, one chunk after the other, but only each chunk once? This is what Linux supports, at least: switch (size) { ... default: barrier(); __builtin_memcpy((void *)res, (const void *)p, size); barrier(); } Or does ACCESS_ONCE() cover more than just this volatile property? > Plus ACCESS_ONCE() doesn't enforce a > single instruction to be used in the resulting assembly - while the > compiler may not fold multiple accesses, it still may break them > up if it wishes to (but of course it usually won't it the whole thing > can be expressed with a single instruction). So looking at the implementation of our single-instruction read_atomic() implementation, I can't find one point: All of i386, x86_64, arm and arm64 seem to guarantee atomicity for a native data type *only* if the address is naturally aligned. "Newer" x86 CPUs seems to guarantee atomicity even for unaligned addresses when the data hits and fits in the cache, but I think this is nothing we can check for or rely on. So I don't see how this alignment is enforced here. I took code snippets from atomic.h and compiled a test case, and indeed passing in a pointer to an unaligned uint32_t (from a packed struct) resulted in a single, but unaligned access: ldr x3, [x29,#57] and: mov 0x11(%rsp),%edx x29 is the frame pointer, so it' strictly aligned as it's derived from the stack pointer, which makes x29 + 57 not aligned. So read_atomic() is not atomic in this case. I don't see an simple and generic way out of this, as there does not seem to be an easy (or cheap) way of checking for unaligned pointers at compile time (although I think the compiler could know this). Can we live with silently assuming aligned variables in the Xen code (due to ABI requirements)? That wouldn't cover crafted or computed pointers, but chances are we use &some_var anyway as a *_atomic() argument? Cheers, Andre. > For add_sized() it's even worse: The macro enforces (on x86) an > operation on a memory operand (i.e. again a single instruction). > "ACCESS_ONCE(x) += n", otoh, may (and iirc normally will) be > translated to a memory of x, addition of the read value with n, > and a memory write. > >> Note that the naming is also confusing as it is easily to mix with the >> atomic_read, atomic_write helpers. > > Well, yes, unfortunately. > > Jan > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |