[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 03/15] exec-all.h: revert tb_page_addr_t to t
From: |
Alex Bennée |
Subject: |
Re: [Qemu-devel] [PATCH v3 03/15] exec-all.h: revert tb_page_addr_t to target_ulong |
Date: |
Tue, 04 Oct 2016 15:08:28 +0100 |
User-agent: |
mu4e 0.9.17; emacs 25.1.50.3 |
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