[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[RFC] Move to C11 Atomics
From: |
Stefan Hajnoczi |
Subject: |
[RFC] Move to C11 Atomics |
Date: |
Mon, 21 Sep 2020 11:41:07 +0100 |
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 <stefanha@redhat.com>
---
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
- [RFC] Move to C11 Atomics,
Stefan Hajnoczi <=