This patch is incomplete but I am looking for feedback on the approach before fully implementing it (which will involve lots of changes).
QEMU's atomic.h provides atomic operations and is intended to work with or without <stdatomic.h>. Some of the atomic.h APIs are from C11 Atomics while others are Linux-inspired or QEMU-specific extensions. atomic.h works fine with gcc but clang enforces the following: atomic_fetch_add() and friends must use C11 Atomic atomic_* types. __atomic_fetch_add() and friends must use direct types (int, etc) and NOT C11 Atomic types. The consequences are: 1. atomic_fetch_*() produces compilation errors since QEMU code uses direct types and not C11 Atomic types. 2. atomic_fetch_*() cannot be used on the same variables as __atomic_fetch_*() because they support different types. This is a problem because QEMU's atomic.h builds on __atomic_fetch_*() and code expects to use both atomic_fetch_*() and QEMU atomic.h APIs on the same variables. I would like to move QEMU to C11 Atomics, removing QEMU-specific APIs which have C11 equivalents. The new atomic.h would #include <stdatomic.h> and define additional APIs on top. It also needs to carry a <stdatomic.h> fallback implementation for RHEL 7 because gcc does not have <stdatomic.h> there. The upshot is that all atomic variables in QEMU need to use C11 Atomic atomic_* types. This is a big change! Also, existing atomic.h APIs that use __atomic_fetch_*() need to move to C11 Atomics instead so that they take atomic_* types. This patch contains a few examples of this conversion. Things to note: 1. Reimplement everything in terms of atomic_fetch_*() and other C11 Atomic APIs. For example atomic_fetch_inc() becomes atomic_fetch_add(ptr, 1). 2. atomic_*_fetch() is not available in C11 Atomics so emulate it by performing the operation twice (once atomic, then again non-atomic using the fetched old atomic value). Yuck! 3. Can everything in atomic.h really be converted to C11 Atomics? I'm not sure yet :(. Better ideas? Signed-off-by: Stefan Hajnoczi <[email protected]> --- include/block/aio.h | 2 +- include/qemu/atomic.h | 79 +++++++++++++++++++++++++++++++++---------- include/qemu/bitops.h | 2 +- include/qom/object.h | 3 +- util/aio-posix.h | 2 +- util/async.c | 2 +- meson.build | 3 ++ 7 files changed, 70 insertions(+), 23 deletions(-) diff --git a/include/block/aio.h b/include/block/aio.h index b2f703fa3f..466c058880 100644 --- a/include/block/aio.h +++ b/include/block/aio.h @@ -220,7 +220,7 @@ struct AioContext { */ QEMUTimerListGroup tlg; - int external_disable_cnt; + atomic_int external_disable_cnt; /* Number of AioHandlers without .io_poll() */ int poll_disable_cnt; diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h index ff72db5115..4fbbd5f362 100644 --- a/include/qemu/atomic.h +++ b/include/qemu/atomic.h @@ -15,6 +15,32 @@ #ifndef QEMU_ATOMIC_H #define QEMU_ATOMIC_H +#include <stdbool.h> +#include <stddef.h> + +/* Use C11 Atomics if possible, otherwise fall back to custom definitions */ +#ifdef CONFIG_STDATOMIC_H +#include <stdatomic.h> +#else +/* Commonly used types from C11 "7.17.6 Atomic integer types" */ +typedef bool atomic_bool; +typedef char atomic_char; +typedef signed char atomic_schar; +typedef unsigned char atomic_uchar; +typedef short atomic_short; +typedef unsigned short atomic_ushort; +typedef int atomic_int; +typedef unsigned int atomic_uint; +typedef long atomic_long; +typedef unsigned long atomic_ulong; +typedef long long atomic_llong; +typedef unsigned long long atomic_ullong; +typedef intptr_t atomic_intptr_t; +typedef uintptr_t atomic_uintptr_t; +typedef size_t atomic_size_t; +typedef ptrdiff_t atomic_ptrdiff_t; +#endif + /* Compiler barrier */ #define barrier() ({ asm volatile("" ::: "memory"); (void)0; }) @@ -205,10 +231,6 @@ atomic_cmpxchg__nocheck(ptr, old, new); \ }) -/* Provide shorter names for GCC atomic builtins, return old value */ -#define atomic_fetch_inc(ptr) __atomic_fetch_add(ptr, 1, __ATOMIC_SEQ_CST) -#define atomic_fetch_dec(ptr) __atomic_fetch_sub(ptr, 1, __ATOMIC_SEQ_CST) - #ifndef atomic_fetch_add #define atomic_fetch_add(ptr, n) __atomic_fetch_add(ptr, n, __ATOMIC_SEQ_CST) #define atomic_fetch_sub(ptr, n) __atomic_fetch_sub(ptr, n, __ATOMIC_SEQ_CST) @@ -217,22 +239,41 @@ #define atomic_fetch_xor(ptr, n) __atomic_fetch_xor(ptr, n, __ATOMIC_SEQ_CST) #endif -#define atomic_inc_fetch(ptr) __atomic_add_fetch(ptr, 1, __ATOMIC_SEQ_CST) -#define atomic_dec_fetch(ptr) __atomic_sub_fetch(ptr, 1, __ATOMIC_SEQ_CST) -#define atomic_add_fetch(ptr, n) __atomic_add_fetch(ptr, n, __ATOMIC_SEQ_CST) -#define atomic_sub_fetch(ptr, n) __atomic_sub_fetch(ptr, n, __ATOMIC_SEQ_CST) -#define atomic_and_fetch(ptr, n) __atomic_and_fetch(ptr, n, __ATOMIC_SEQ_CST) -#define atomic_or_fetch(ptr, n) __atomic_or_fetch(ptr, n, __ATOMIC_SEQ_CST) -#define atomic_xor_fetch(ptr, n) __atomic_xor_fetch(ptr, n, __ATOMIC_SEQ_CST) +/* Provide shorter names for GCC atomic builtins, return old value */ +#define atomic_fetch_inc(ptr) atomic_fetch_add(ptr, 1) +#define atomic_fetch_dec(ptr) atomic_fetch_sub(ptr, 1) + +#define atomic_inc_fetch(ptr) (atomic_fetch_add(ptr, 1) + 1) +#define atomic_dec_fetch(ptr) (atomic_fetch_sub(ptr, 1) - 1) +#define atomic_add_fetch(ptr, n) ({ \ + typeof(n) _n = n; \ + atomic_fetch_add(ptr, _n) + _n; \ +}) +#define atomic_sub_fetch(ptr, n) ({ \ + typeof(n) _n = n; \ + atomic_fetch_sub(ptr, _n) - n; \ +}) +#define atomic_and_fetch(ptr, n) ({ \ + typeof(n) _n = n; \ + atomic_fetch_and(ptr, _n) & _n; \ +}) +#define atomic_or_fetch(ptr, n) ({ \ + typeof(n) _n = n; \ + atomic_fetch_or(ptr, _n) | _n; \ +}) +#define atomic_xor_fetch(ptr, n) ({ \ + typeof(n) _n = n; \ + atomic_fetch_xor(ptr, _n) ^ _n; \ +}) /* And even shorter names that return void. */ -#define atomic_inc(ptr) ((void) __atomic_fetch_add(ptr, 1, __ATOMIC_SEQ_CST)) -#define atomic_dec(ptr) ((void) __atomic_fetch_sub(ptr, 1, __ATOMIC_SEQ_CST)) -#define atomic_add(ptr, n) ((void) __atomic_fetch_add(ptr, n, __ATOMIC_SEQ_CST)) -#define atomic_sub(ptr, n) ((void) __atomic_fetch_sub(ptr, n, __ATOMIC_SEQ_CST)) -#define atomic_and(ptr, n) ((void) __atomic_fetch_and(ptr, n, __ATOMIC_SEQ_CST)) -#define atomic_or(ptr, n) ((void) __atomic_fetch_or(ptr, n, __ATOMIC_SEQ_CST)) -#define atomic_xor(ptr, n) ((void) __atomic_fetch_xor(ptr, n, __ATOMIC_SEQ_CST)) +#define atomic_inc(ptr) ((void) atomic_fetch_add(ptr, 1)) +#define atomic_dec(ptr) ((void) atomic_fetch_sub(ptr, 1)) +#define atomic_add(ptr, n) ((void) atomic_fetch_add(ptr, n)) +#define atomic_sub(ptr, n) ((void) atomic_fetch_sub(ptr, n)) +#define atomic_and(ptr, n) ((void) atomic_fetch_and(ptr, n)) +#define atomic_or(ptr, n) ((void) atomic_fetch_or(ptr, n)) +#define atomic_xor(ptr, n) ((void) atomic_fetch_xor(ptr, n)) #else /* __ATOMIC_RELAXED */ @@ -424,6 +465,8 @@ #define atomic_or(ptr, n) ((void) __sync_fetch_and_or(ptr, n)) #define atomic_xor(ptr, n) ((void) __sync_fetch_and_xor(ptr, n)) +#error TODO + #endif /* __ATOMIC_RELAXED */ #ifndef smp_wmb diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h index f55ce8b320..e9d676d112 100644 --- a/include/qemu/bitops.h +++ b/include/qemu/bitops.h @@ -49,7 +49,7 @@ static inline void set_bit(long nr, unsigned long *addr) static inline void set_bit_atomic(long nr, unsigned long *addr) { unsigned long mask = BIT_MASK(nr); - unsigned long *p = addr + BIT_WORD(nr); + atomic_ulong *p = (atomic_ulong *)(addr + BIT_WORD(nr)); atomic_or(p, mask); } diff --git a/include/qom/object.h b/include/qom/object.h index 056f67ab3b..f51244b61f 100644 --- a/include/qom/object.h +++ b/include/qom/object.h @@ -15,6 +15,7 @@ #define QEMU_OBJECT_H #include "qapi/qapi-builtin-types.h" +#include "qemu/atomic.h" #include "qemu/module.h" #include "qom/object.h" @@ -550,7 +551,7 @@ struct Object ObjectClass *class; ObjectFree *free; GHashTable *properties; - uint32_t ref; + atomic_uint ref; Object *parent; }; diff --git a/util/aio-posix.h b/util/aio-posix.h index c80c04506a..c5b446f0a1 100644 --- a/util/aio-posix.h +++ b/util/aio-posix.h @@ -33,7 +33,7 @@ struct AioHandler { QLIST_ENTRY(AioHandler) node_poll; #ifdef CONFIG_LINUX_IO_URING QSLIST_ENTRY(AioHandler) node_submitted; - unsigned flags; /* see fdmon-io_uring.c */ + atomic_uint flags; /* see fdmon-io_uring.c */ #endif int64_t poll_idle_timeout; /* when to stop userspace polling */ bool is_external; diff --git a/util/async.c b/util/async.c index 4266745dee..dcf1a32492 100644 --- a/util/async.c +++ b/util/async.c @@ -60,7 +60,7 @@ struct QEMUBH { QEMUBHFunc *cb; void *opaque; QSLIST_ENTRY(QEMUBH) next; - unsigned flags; + atomic_uint flags; }; /* Called concurrently from any thread */ diff --git a/meson.build b/meson.build index f4d1ab1096..8d80033d90 100644 --- a/meson.build +++ b/meson.build @@ -433,8 +433,11 @@ keyutils = dependency('libkeyutils', required: false, has_gettid = cc.has_function('gettid') +has_stdatomic_h = cc.has_header('stdatomic.h') + # Create config-host.h +config_host_data.set('CONFIG_STDATOMIC_H', has_stdatomic_h) config_host_data.set('CONFIG_SDL', sdl.found()) config_host_data.set('CONFIG_SDL_IMAGE', sdl_image.found()) config_host_data.set('CONFIG_VNC', vnc.found()) -- 2.26.2
