Re: [Qemu-devel] [PATCH v3 03/15] exec-all.h: revert tb_page_addr_t to target_ulong

[Alex Bennée]

Paolo Bonzini <address@hidden> writes:

> On 03/10/2016 11:32, Alex Bennée wrote:
>> 1. For atomic_read/set fall back to plain access for sizeof(*ptr) >
>> sizeof(void *)
>>
>> This would throw up warnings in ThreadSanitizer on 64-on-32 but
>> practically generate the same code as before.
>>
>> 2. Split sizeof(*ptr) > sizeof(void *) accesses over two
>>
>> This would keep the sanitizer happy but be incorrect behaviour, you
>> could get tears.
>>
>> 3. Add a generic 64-on-32 lock for these accesses
>>
>> It would be a global lock for any oversized access which could end up
>> being highly contended but at least any behaviour would be always
>> correct.
>
> (3) is what libatomic does.
>
> For generic statistics/counters I have written, but not yet upstreamed,
> a module which defines two abstract data types:
>
> - Stat64 is a 64-bit value and it can do 64-bit add/min/max.  Reads
> block writes, but writes try to bypass the lock if possible (e.g.
> min/max that don't modify the stored value, or add that only touches the
> lower 32-bits).
>
> - Count64 is actually a 63-bit value and can do 32-bit add or 63-bit
> get/set.  Writes block reads, but 32-bit adds try pretty hard not to.
>
> These are meant for the block layer, so they would be necessary even if
> 64-on-32 went away.  Unfortunately, neither is a perfect replacement for
> a 64-bit variable...

With rth's CONFIG_ATOMIC64 patch mingw doesn't complain with this:

atomic.h: relax sizeof(*ptr) > sizeof(*void) check

If the compiler can already handle oversized accesses let it do so.

Signed-off-by: Alex Bennée <address@hidden>

1 file changed, 10 insertions(+), 4 deletions(-)
include/qemu/atomic.h | 14 ++++++++++----

modified   include/qemu/atomic.h
@@ -99,15 +99,21 @@
  * no effect on the generated code but not using the atomic primitives
  * will get flagged by sanitizers as a violation.
  */
+#ifdef CONFIG_ATOMIC64
+#define ATOMIC_BUILD_BUG_ON(ptr) QEMU_BUILD_BUG_ON(sizeof(*ptr) > 
sizeof(uint64_t));
+#else
+#define ATOMIC_BUILD_BUG_ON(ptr) QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void 
*));
+#endif
+
 #define atomic_read(ptr)                              \
     ({                                                \
-    QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \
-    __atomic_load_n(ptr, __ATOMIC_RELAXED);           \
+        ATOMIC_BUILD_BUG_ON(ptr);                     \
+        __atomic_load_n(ptr, __ATOMIC_RELAXED);       \
     })

 #define atomic_set(ptr, i)  do {                      \
-    QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \
-    __atomic_store_n(ptr, i, __ATOMIC_RELAXED);       \
+        ATOMIC_BUILD_BUG_ON(ptr);                     \
+        __atomic_store_n(ptr, i, __ATOMIC_RELAXED);   \
 } while(0)

 /* See above: most compilers currently treat consume and acquire the