* [PATCH experiment 01/16] coroutine: add missing coroutine_fn annotations for CoRwlock functions
2022-03-14 9:31 [PATCH experiment 00/16] C++20 coroutine backend Paolo Bonzini
@ 2022-03-14 9:31 ` Paolo Bonzini
2022-03-14 9:31 ` [PATCH experiment 02/16] coroutine: qemu_coroutine_get_aio_context is not a coroutine_fn Paolo Bonzini
` (17 subsequent siblings)
18 siblings, 0 replies; 53+ messages in thread
From: Paolo Bonzini @ 2022-03-14 9:31 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, hreitz, berrange, qemu-block, stefanha
CoRwlock can only be taken or released from a coroutine, and it
can yield. Mark it as coroutine_fn.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/coroutine.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index c828a95ee0..da68be5ad2 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -261,7 +261,7 @@ void qemu_co_rwlock_init(CoRwlock *lock);
* of a parallel writer, control is transferred to the caller of the current
* coroutine.
*/
-void qemu_co_rwlock_rdlock(CoRwlock *lock);
+void coroutine_fn qemu_co_rwlock_rdlock(CoRwlock *lock);
/**
* Write Locks the CoRwlock from a reader. This is a bit more efficient than
@@ -270,7 +270,7 @@ void qemu_co_rwlock_rdlock(CoRwlock *lock);
* to the caller of the current coroutine; another writer might run while
* @qemu_co_rwlock_upgrade blocks.
*/
-void qemu_co_rwlock_upgrade(CoRwlock *lock);
+void coroutine_fn qemu_co_rwlock_upgrade(CoRwlock *lock);
/**
* Downgrades a write-side critic section to a reader. Downgrading with
@@ -278,20 +278,20 @@ void qemu_co_rwlock_upgrade(CoRwlock *lock);
* followed by @qemu_co_rwlock_rdlock. This makes it more efficient, but
* may also sometimes be necessary for correctness.
*/
-void qemu_co_rwlock_downgrade(CoRwlock *lock);
+void coroutine_fn qemu_co_rwlock_downgrade(CoRwlock *lock);
/**
* Write Locks the mutex. If the lock cannot be taken immediately because
* of a parallel reader, control is transferred to the caller of the current
* coroutine.
*/
-void qemu_co_rwlock_wrlock(CoRwlock *lock);
+void coroutine_fn qemu_co_rwlock_wrlock(CoRwlock *lock);
/**
* Unlocks the read/write lock and schedules the next coroutine that was
* waiting for this lock to be run.
*/
-void qemu_co_rwlock_unlock(CoRwlock *lock);
+void coroutine_fn qemu_co_rwlock_unlock(CoRwlock *lock);
typedef struct QemuCoSleep {
Coroutine *to_wake;
--
2.35.1
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH experiment 02/16] coroutine: qemu_coroutine_get_aio_context is not a coroutine_fn
2022-03-14 9:31 [PATCH experiment 00/16] C++20 coroutine backend Paolo Bonzini
2022-03-14 9:31 ` [PATCH experiment 01/16] coroutine: add missing coroutine_fn annotations for CoRwlock functions Paolo Bonzini
@ 2022-03-14 9:31 ` Paolo Bonzini
2022-03-14 9:31 ` [PATCH experiment 03/16] coroutine: small code cleanup in qemu_co_rwlock_wrlock Paolo Bonzini
` (16 subsequent siblings)
18 siblings, 0 replies; 53+ messages in thread
From: Paolo Bonzini @ 2022-03-14 9:31 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, hreitz, berrange, qemu-block, stefanha
Since it operates on a given coroutine, qemu_coroutine_get_aio_context
can be called from outside coroutine context.
This is for example how qio_channel_restart_read uses it.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/coroutine.h | 2 +-
util/qemu-coroutine.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index da68be5ad2..666f3ba0e0 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -92,7 +92,7 @@ void coroutine_fn qemu_coroutine_yield(void);
/**
* Get the AioContext of the given coroutine
*/
-AioContext *coroutine_fn qemu_coroutine_get_aio_context(Coroutine *co);
+AioContext *qemu_coroutine_get_aio_context(Coroutine *co);
/**
* Get the currently executing coroutine
diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c
index c03b2422ff..9f2bd96fa0 100644
--- a/util/qemu-coroutine.c
+++ b/util/qemu-coroutine.c
@@ -200,7 +200,7 @@ bool qemu_coroutine_entered(Coroutine *co)
return co->caller;
}
-AioContext *coroutine_fn qemu_coroutine_get_aio_context(Coroutine *co)
+AioContext *qemu_coroutine_get_aio_context(Coroutine *co)
{
return co->ctx;
}
--
2.35.1
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH experiment 03/16] coroutine: small code cleanup in qemu_co_rwlock_wrlock
2022-03-14 9:31 [PATCH experiment 00/16] C++20 coroutine backend Paolo Bonzini
2022-03-14 9:31 ` [PATCH experiment 01/16] coroutine: add missing coroutine_fn annotations for CoRwlock functions Paolo Bonzini
2022-03-14 9:31 ` [PATCH experiment 02/16] coroutine: qemu_coroutine_get_aio_context is not a coroutine_fn Paolo Bonzini
@ 2022-03-14 9:31 ` Paolo Bonzini
2022-03-14 13:32 ` Philippe Mathieu-Daudé
2022-03-14 9:31 ` [PATCH experiment 04/16] coroutine: introduce QemuCoLockable Paolo Bonzini
` (15 subsequent siblings)
18 siblings, 1 reply; 53+ messages in thread
From: Paolo Bonzini @ 2022-03-14 9:31 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, hreitz, berrange, qemu-block, stefanha
qemu_co_rwlock_wrlock stores the current coroutine in a loc variable,
use it instead of calling qemu_coroutine_self() again.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
util/qemu-coroutine-lock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/util/qemu-coroutine-lock.c b/util/qemu-coroutine-lock.c
index 2669403839..490fb32891 100644
--- a/util/qemu-coroutine-lock.c
+++ b/util/qemu-coroutine-lock.c
@@ -436,7 +436,7 @@ void qemu_co_rwlock_wrlock(CoRwlock *lock)
lock->owners = -1;
qemu_co_mutex_unlock(&lock->mutex);
} else {
- CoRwTicket my_ticket = { false, qemu_coroutine_self() };
+ CoRwTicket my_ticket = { false, self };
QSIMPLEQ_INSERT_TAIL(&lock->tickets, &my_ticket, next);
qemu_co_mutex_unlock(&lock->mutex);
--
2.35.1
^ permalink raw reply related [flat|nested] 53+ messages in thread* Re: [PATCH experiment 03/16] coroutine: small code cleanup in qemu_co_rwlock_wrlock
2022-03-14 9:31 ` [PATCH experiment 03/16] coroutine: small code cleanup in qemu_co_rwlock_wrlock Paolo Bonzini
@ 2022-03-14 13:32 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 53+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-14 13:32 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: kwolf, hreitz, berrange, stefanha, qemu-block
On 14/3/22 10:31, Paolo Bonzini wrote:
> qemu_co_rwlock_wrlock stores the current coroutine in a loc variable,
> use it instead of calling qemu_coroutine_self() again.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> util/qemu-coroutine-lock.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH experiment 04/16] coroutine: introduce QemuCoLockable
2022-03-14 9:31 [PATCH experiment 00/16] C++20 coroutine backend Paolo Bonzini
` (2 preceding siblings ...)
2022-03-14 9:31 ` [PATCH experiment 03/16] coroutine: small code cleanup in qemu_co_rwlock_wrlock Paolo Bonzini
@ 2022-03-14 9:31 ` Paolo Bonzini
2022-03-14 9:31 ` [PATCH experiment 05/16] port atomic.h to C++ Paolo Bonzini
` (14 subsequent siblings)
18 siblings, 0 replies; 53+ messages in thread
From: Paolo Bonzini @ 2022-03-14 9:31 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, hreitz, berrange, qemu-block, stefanha
In preparation for splitting "from coroutine" ("awaitable" in other
languages) and "not from coroutine" functions, remove the CoMutex case
from QemuLockable---thus making qemu_lockable_lock and qemu_lockable_unlock
"not awaitable".
To satisfy the qemu_co_queue_wait use case, introduce QemuCoLockable
which can be used for both QemuMutex (which will trivially never yield)
and CoMutex. qemu_co_lockable_lock and qemu_co_lockable_unlock are
coroutine_fns.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/co-lockable.h | 98 +++++++++++++++++++++++++++++++++++++
include/qemu/coroutine.h | 5 +-
include/qemu/lockable.h | 13 ++---
include/qemu/typedefs.h | 1 +
tests/unit/test-coroutine.c | 10 ++--
util/qemu-coroutine-lock.c | 6 +--
6 files changed, 114 insertions(+), 19 deletions(-)
create mode 100644 include/qemu/co-lockable.h
diff --git a/include/qemu/co-lockable.h b/include/qemu/co-lockable.h
new file mode 100644
index 0000000000..09f4620017
--- /dev/null
+++ b/include/qemu/co-lockable.h
@@ -0,0 +1,98 @@
+/*
+ * Polymorphic locking functions (aka poor man templates)
+ *
+ * Copyright Red Hat, Inc. 2017, 2018
+ *
+ * Author: Paolo Bonzini <pbonzini@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef QEMU_CO_LOCKABLE_H
+#define QEMU_CO_LOCKABLE_H
+
+#include "qemu/coroutine.h"
+#include "qemu/thread.h"
+
+typedef void coroutine_fn QemuCoLockUnlockFunc(void *);
+
+struct QemuCoLockable {
+ void *object;
+ QemuCoLockUnlockFunc *lock;
+ QemuCoLockUnlockFunc *unlock;
+};
+
+static inline __attribute__((__always_inline__)) QemuCoLockable *
+qemu_make_co_lockable(void *x, QemuCoLockable *lockable)
+{
+ /*
+ * We cannot test this in a macro, otherwise we get compiler
+ * warnings like "the address of 'm' will always evaluate as 'true'".
+ */
+ return x ? lockable : NULL;
+}
+
+static inline __attribute__((__always_inline__)) QemuCoLockable *
+qemu_null_co_lockable(void *x)
+{
+ if (x != NULL) {
+ qemu_build_not_reached();
+ }
+ return NULL;
+}
+
+/*
+ * In C, compound literals have the lifetime of an automatic variable.
+ * In C++ it would be different, but then C++ wouldn't need QemuCoLockable
+ * either...
+ */
+#define QMCL_OBJ_(x, name) (&(QemuCoLockable) { \
+ .object = (x), \
+ .lock = (QemuCoLockUnlockFunc *) qemu_ ## name ## _lock, \
+ .unlock = (QemuCoLockUnlockFunc *) qemu_ ## name ## _unlock \
+ })
+
+/**
+ * QEMU_MAKE_CO_LOCKABLE - Make a polymorphic QemuCoLockable
+ *
+ * @x: a lock object (currently one of QemuMutex, CoMutex).
+ *
+ * Returns a QemuCoLockable object that can be passed around
+ * to a function that can operate with locks of any kind, or
+ * NULL if @x is %NULL.
+ *
+ * Note the speci case for void *, so that we may pass "NULL".
+ */
+#define QEMU_MAKE_CO_LOCKABLE(x) \
+ _Generic((x), QemuCoLockable *: (x), \
+ void *: qemu_null_co_lockable(x), \
+ QemuMutex *: qemu_make_co_lockable(x, QMCL_OBJ_(x, mutex)), \
+ CoMutex *: qemu_make_co_lockable(x, QMCL_OBJ_(x, co_mutex))) \
+
+/**
+ * QEMU_MAKE_CO_LOCKABLE_NONNULL - Make a polymorphic QemuCoLockable
+ *
+ * @x: a lock object (currently one of QemuMutex, QemuRecMutex,
+ * CoMutex, QemuSpin).
+ *
+ * Returns a QemuCoLockable object that can be passed around
+ * to a function that can operate with locks of any kind.
+ */
+#define QEMU_MAKE_CO_LOCKABLE_NONNULL(x) \
+ _Generic((x), QemuCoLockable *: (x), \
+ QemuMutex *: QMCL_OBJ_(x, mutex), \
+ CoMutex *: QMCL_OBJ_(x, co_mutex))
+
+static inline void coroutine_fn qemu_co_lockable_lock(QemuCoLockable *x)
+{
+ x->lock(x->object);
+}
+
+static inline void coroutine_fn qemu_co_lockable_unlock(QemuCoLockable *x)
+{
+ x->unlock(x->object);
+}
+
+#endif
diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index 666f3ba0e0..6f4596fc5b 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -204,8 +204,8 @@ void qemu_co_queue_init(CoQueue *queue);
* locked again afterwards.
*/
#define qemu_co_queue_wait(queue, lock) \
- qemu_co_queue_wait_impl(queue, QEMU_MAKE_LOCKABLE(lock))
-void coroutine_fn qemu_co_queue_wait_impl(CoQueue *queue, QemuLockable *lock);
+ qemu_co_queue_wait_impl(queue, QEMU_MAKE_CO_LOCKABLE(lock))
+void coroutine_fn qemu_co_queue_wait_impl(CoQueue *queue, QemuCoLockable *lock);
/**
* Removes the next coroutine from the CoQueue, and wake it up.
@@ -342,5 +342,6 @@ void qemu_coroutine_increase_pool_batch_size(unsigned int additional_pool_size);
void qemu_coroutine_decrease_pool_batch_size(unsigned int additional_pool_size);
#include "qemu/lockable.h"
+#include "qemu/co-lockable.h"
#endif /* QEMU_COROUTINE_H */
diff --git a/include/qemu/lockable.h b/include/qemu/lockable.h
index 86db7cb04c..c860f81737 100644
--- a/include/qemu/lockable.h
+++ b/include/qemu/lockable.h
@@ -13,7 +13,6 @@
#ifndef QEMU_LOCKABLE_H
#define QEMU_LOCKABLE_H
-#include "qemu/coroutine.h"
#include "qemu/thread.h"
typedef void QemuLockUnlockFunc(void *);
@@ -57,8 +56,7 @@ qemu_null_lockable(void *x)
/**
* QEMU_MAKE_LOCKABLE - Make a polymorphic QemuLockable
*
- * @x: a lock object (currently one of QemuMutex, QemuRecMutex,
- * CoMutex, QemuSpin).
+ * @x: a lock object (currently one of QemuMutex, QemuRecMutex, QemuSpin).
*
* Returns a QemuLockable object that can be passed around
* to a function that can operate with locks of any kind, or
@@ -71,14 +69,12 @@ qemu_null_lockable(void *x)
void *: qemu_null_lockable(x), \
QemuMutex *: qemu_make_lockable(x, QML_OBJ_(x, mutex)), \
QemuRecMutex *: qemu_make_lockable(x, QML_OBJ_(x, rec_mutex)), \
- CoMutex *: qemu_make_lockable(x, QML_OBJ_(x, co_mutex)), \
QemuSpin *: qemu_make_lockable(x, QML_OBJ_(x, spin)))
/**
* QEMU_MAKE_LOCKABLE_NONNULL - Make a polymorphic QemuLockable
*
- * @x: a lock object (currently one of QemuMutex, QemuRecMutex,
- * CoMutex, QemuSpin).
+ * @x: a lock object (currently one of QemuMutex, QemuRecMutex, QemuSpin).
*
* Returns a QemuLockable object that can be passed around
* to a function that can operate with locks of any kind.
@@ -87,7 +83,6 @@ qemu_null_lockable(void *x)
_Generic((x), QemuLockable *: (x), \
QemuMutex *: QML_OBJ_(x, mutex), \
QemuRecMutex *: QML_OBJ_(x, rec_mutex), \
- CoMutex *: QML_OBJ_(x, co_mutex), \
QemuSpin *: QML_OBJ_(x, spin))
static inline void qemu_lockable_lock(QemuLockable *x)
@@ -124,7 +119,7 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(QemuLockable, qemu_lockable_auto_unlock)
/**
* WITH_QEMU_LOCK_GUARD - Lock a lock object for scope
*
- * @x: a lock object (currently one of QemuMutex, CoMutex, QemuSpin).
+ * @x: a lock object (currently one of QemuMutex, QemuRecMutex, QemuSpin).
*
* This macro defines a lock scope such that entering the scope takes the lock
* and leaving the scope releases the lock. Return statements are allowed
@@ -149,7 +144,7 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(QemuLockable, qemu_lockable_auto_unlock)
/**
* QEMU_LOCK_GUARD - Lock an object until the end of the scope
*
- * @x: a lock object (currently one of QemuMutex, CoMutex, QemuSpin).
+ * @x: a lock object (currently one of QemuMutex, QemuRecMutex, QemuSpin).
*
* This macro takes a lock until the end of the scope. Return statements
* release the lock.
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 42f4ceb701..144ce82b8b 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -103,6 +103,7 @@ typedef struct QBool QBool;
typedef struct QDict QDict;
typedef struct QEMUBH QEMUBH;
typedef struct QemuConsole QemuConsole;
+typedef struct QemuCoLockable QemuCoLockable;
typedef struct QEMUFile QEMUFile;
typedef struct QemuLockable QemuLockable;
typedef struct QemuMutex QemuMutex;
diff --git a/tests/unit/test-coroutine.c b/tests/unit/test-coroutine.c
index aa77a3bcb3..82e22db070 100644
--- a/tests/unit/test-coroutine.c
+++ b/tests/unit/test-coroutine.c
@@ -213,13 +213,13 @@ static void coroutine_fn mutex_fn(void *opaque)
static void coroutine_fn lockable_fn(void *opaque)
{
- QemuLockable *x = opaque;
- qemu_lockable_lock(x);
+ QemuCoLockable *x = opaque;
+ qemu_co_lockable_lock(x);
assert(!locked);
locked = true;
qemu_coroutine_yield();
locked = false;
- qemu_lockable_unlock(x);
+ qemu_co_lockable_unlock(x);
done++;
}
@@ -259,9 +259,9 @@ static void test_co_mutex_lockable(void)
CoMutex *null_pointer = NULL;
qemu_co_mutex_init(&m);
- do_test_co_mutex(lockable_fn, QEMU_MAKE_LOCKABLE(&m));
+ do_test_co_mutex(lockable_fn, QEMU_MAKE_CO_LOCKABLE(&m));
- g_assert(QEMU_MAKE_LOCKABLE(null_pointer) == NULL);
+ g_assert(QEMU_MAKE_CO_LOCKABLE(null_pointer) == NULL);
}
static CoRwlock rwlock;
diff --git a/util/qemu-coroutine-lock.c b/util/qemu-coroutine-lock.c
index 490fb32891..3f12b53a31 100644
--- a/util/qemu-coroutine-lock.c
+++ b/util/qemu-coroutine-lock.c
@@ -39,13 +39,13 @@ void qemu_co_queue_init(CoQueue *queue)
QSIMPLEQ_INIT(&queue->entries);
}
-void coroutine_fn qemu_co_queue_wait_impl(CoQueue *queue, QemuLockable *lock)
+void coroutine_fn qemu_co_queue_wait_impl(CoQueue *queue, QemuCoLockable *lock)
{
Coroutine *self = qemu_coroutine_self();
QSIMPLEQ_INSERT_TAIL(&queue->entries, self, co_queue_next);
if (lock) {
- qemu_lockable_unlock(lock);
+ qemu_co_lockable_unlock(lock);
}
/* There is no race condition here. Other threads will call
@@ -63,7 +63,7 @@ void coroutine_fn qemu_co_queue_wait_impl(CoQueue *queue, QemuLockable *lock)
* other cases of QemuLockable.
*/
if (lock) {
- qemu_lockable_lock(lock);
+ qemu_co_lockable_lock(lock);
}
}
--
2.35.1
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH experiment 05/16] port atomic.h to C++
2022-03-14 9:31 [PATCH experiment 00/16] C++20 coroutine backend Paolo Bonzini
` (3 preceding siblings ...)
2022-03-14 9:31 ` [PATCH experiment 04/16] coroutine: introduce QemuCoLockable Paolo Bonzini
@ 2022-03-14 9:31 ` Paolo Bonzini
2022-03-14 9:31 ` [PATCH experiment 06/16] use g_new0 instead of g_malloc0 Paolo Bonzini
` (13 subsequent siblings)
18 siblings, 0 replies; 53+ messages in thread
From: Paolo Bonzini @ 2022-03-14 9:31 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, hreitz, berrange, qemu-block, stefanha
The functionality of typeof_strip_qual is provided by the standard library.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/atomic.h | 5 +++++
include/qemu/osdep.h | 1 +
2 files changed, 6 insertions(+)
diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index 112a29910b..0889a9c5d2 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -26,6 +26,10 @@
* implicit promotion. int and larger types, as well as pointers, can be
* converted to a non-qualified type just by applying a binary operator.
*/
+#ifdef __cplusplus
+#define typeof_strip_qual(expr) \
+ std::remove_cv<typeof(expr)>::type
+#else
#define typeof_strip_qual(expr) \
typeof( \
__builtin_choose_expr( \
@@ -59,6 +63,7 @@
__builtin_types_compatible_p(typeof(expr), const volatile unsigned short), \
(unsigned short)1, \
(expr)+0))))))
+#endif
#ifndef __ATOMIC_RELAXED
#error "Expecting C11 atomic ops"
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index c9ec7830c9..28707bbde3 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -140,6 +140,7 @@ QEMU_EXTERN_C int daemon(int, int);
#endif
#ifdef __cplusplus
+#include <type_traits>
extern "C" {
#endif
--
2.35.1
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH experiment 06/16] use g_new0 instead of g_malloc0
2022-03-14 9:31 [PATCH experiment 00/16] C++20 coroutine backend Paolo Bonzini
` (4 preceding siblings ...)
2022-03-14 9:31 ` [PATCH experiment 05/16] port atomic.h to C++ Paolo Bonzini
@ 2022-03-14 9:31 ` Paolo Bonzini
2022-03-14 11:16 ` Markus Armbruster
2022-03-14 9:31 ` [PATCH experiment 07/16] start porting compiler.h to C++ Paolo Bonzini
` (12 subsequent siblings)
18 siblings, 1 reply; 53+ messages in thread
From: Paolo Bonzini @ 2022-03-14 9:31 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, hreitz, berrange, qemu-block, stefanha
Casting to/from void* must be explicit in C++. g_new0 takes care of that.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/timer.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 88ef114689..ee071e07d1 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -520,7 +520,7 @@ static inline QEMUTimer *timer_new_full(QEMUTimerListGroup *timer_list_group,
int scale, int attributes,
QEMUTimerCB *cb, void *opaque)
{
- QEMUTimer *ts = g_malloc0(sizeof(QEMUTimer));
+ QEMUTimer *ts = g_new0(QEMUTimer, 1);
timer_init_full(ts, timer_list_group, type, scale, attributes, cb, opaque);
return ts;
}
--
2.35.1
^ permalink raw reply related [flat|nested] 53+ messages in thread* Re: [PATCH experiment 06/16] use g_new0 instead of g_malloc0
2022-03-14 9:31 ` [PATCH experiment 06/16] use g_new0 instead of g_malloc0 Paolo Bonzini
@ 2022-03-14 11:16 ` Markus Armbruster
0 siblings, 0 replies; 53+ messages in thread
From: Markus Armbruster @ 2022-03-14 11:16 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kwolf, berrange, qemu-block, qemu-devel, hreitz, stefanha
Paolo Bonzini <pbonzini@redhat.com> writes:
> Casting to/from void* must be explicit in C++. g_new0 takes care of that.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> include/qemu/timer.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/qemu/timer.h b/include/qemu/timer.h
> index 88ef114689..ee071e07d1 100644
> --- a/include/qemu/timer.h
> +++ b/include/qemu/timer.h
> @@ -520,7 +520,7 @@ static inline QEMUTimer *timer_new_full(QEMUTimerListGroup *timer_list_group,
> int scale, int attributes,
> QEMUTimerCB *cb, void *opaque)
> {
> - QEMUTimer *ts = g_malloc0(sizeof(QEMUTimer));
> + QEMUTimer *ts = g_new0(QEMUTimer, 1);
> timer_init_full(ts, timer_list_group, type, scale, attributes, cb, opaque);
> return ts;
> }
Looks like a rerun of commit b45c03f585's Coccinelle script is due.
I'll take care of it.
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH experiment 07/16] start porting compiler.h to C++
2022-03-14 9:31 [PATCH experiment 00/16] C++20 coroutine backend Paolo Bonzini
` (5 preceding siblings ...)
2022-03-14 9:31 ` [PATCH experiment 06/16] use g_new0 instead of g_malloc0 Paolo Bonzini
@ 2022-03-14 9:31 ` Paolo Bonzini
2022-03-14 9:31 ` [PATCH experiment 08/16] tracetool: add extern "C" around generated headers Paolo Bonzini
` (11 subsequent siblings)
18 siblings, 0 replies; 53+ messages in thread
From: Paolo Bonzini @ 2022-03-14 9:31 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, hreitz, berrange, qemu-block, stefanha
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/compiler.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index 3baa5e3790..18848f0d49 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -72,6 +72,10 @@
int:(x) ? -1 : 1; \
}
+#ifdef __cplusplus
+#define _Static_assert static_assert
+#endif
+
#define QEMU_BUILD_BUG_MSG(x, msg) _Static_assert(!(x), msg)
#define QEMU_BUILD_BUG_ON(x) QEMU_BUILD_BUG_MSG(x, "not expecting: " #x)
--
2.35.1
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH experiment 08/16] tracetool: add extern "C" around generated headers
2022-03-14 9:31 [PATCH experiment 00/16] C++20 coroutine backend Paolo Bonzini
` (6 preceding siblings ...)
2022-03-14 9:31 ` [PATCH experiment 07/16] start porting compiler.h to C++ Paolo Bonzini
@ 2022-03-14 9:31 ` Paolo Bonzini
2022-03-14 13:33 ` Philippe Mathieu-Daudé
2022-03-14 9:31 ` [PATCH experiment 09/16] start adding extern "C" markers Paolo Bonzini
` (10 subsequent siblings)
18 siblings, 1 reply; 53+ messages in thread
From: Paolo Bonzini @ 2022-03-14 9:31 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, hreitz, berrange, qemu-block, stefanha
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
scripts/tracetool/format/h.py | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
index e94f0be7da..2d92fa8bd2 100644
--- a/scripts/tracetool/format/h.py
+++ b/scripts/tracetool/format/h.py
@@ -27,6 +27,9 @@ def generate(events, backend, group):
'#define TRACE_%s_GENERATED_TRACERS_H' % group.upper(),
'',
'#include "%s"' % header,
+ '#ifdef __cplusplus',
+ 'extern "C" {',
+ '#endif'
'')
for e in events:
@@ -100,4 +103,7 @@ def generate(events, backend, group):
backend.generate_end(events, group)
- out('#endif /* TRACE_%s_GENERATED_TRACERS_H */' % group.upper())
+ out('#ifdef __cplusplus',
+ '}',
+ '#endif',
+ '#endif /* TRACE_%s_GENERATED_TRACERS_H */' % group.upper())
--
2.35.1
^ permalink raw reply related [flat|nested] 53+ messages in thread* Re: [PATCH experiment 08/16] tracetool: add extern "C" around generated headers
2022-03-14 9:31 ` [PATCH experiment 08/16] tracetool: add extern "C" around generated headers Paolo Bonzini
@ 2022-03-14 13:33 ` Philippe Mathieu-Daudé
2022-03-14 13:44 ` Paolo Bonzini
0 siblings, 1 reply; 53+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-14 13:33 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: kwolf, hreitz, berrange, stefanha, qemu-block
On 14/3/22 10:31, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> scripts/tracetool/format/h.py | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
> index e94f0be7da..2d92fa8bd2 100644
> --- a/scripts/tracetool/format/h.py
> +++ b/scripts/tracetool/format/h.py
> @@ -27,6 +27,9 @@ def generate(events, backend, group):
> '#define TRACE_%s_GENERATED_TRACERS_H' % group.upper(),
> '',
> '#include "%s"' % header,
> + '#ifdef __cplusplus',
> + 'extern "C" {',
> + '#endif'
Why not use G_BEGIN_DECLS?
Anyhow,
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 53+ messages in thread* Re: [PATCH experiment 08/16] tracetool: add extern "C" around generated headers
2022-03-14 13:33 ` Philippe Mathieu-Daudé
@ 2022-03-14 13:44 ` Paolo Bonzini
0 siblings, 0 replies; 53+ messages in thread
From: Paolo Bonzini @ 2022-03-14 13:44 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: kwolf, hreitz, berrange, qemu-block, stefanha
On 3/14/22 14:33, Philippe Mathieu-Daudé wrote:
> On 14/3/22 10:31, Paolo Bonzini wrote:
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>> scripts/tracetool/format/h.py | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/scripts/tracetool/format/h.py
>> b/scripts/tracetool/format/h.py
>> index e94f0be7da..2d92fa8bd2 100644
>> --- a/scripts/tracetool/format/h.py
>> +++ b/scripts/tracetool/format/h.py
>> @@ -27,6 +27,9 @@ def generate(events, backend, group):
>> '#define TRACE_%s_GENERATED_TRACERS_H' % group.upper(),
>> '',
>> '#include "%s"' % header,
>> + '#ifdef __cplusplus',
>> + 'extern "C" {',
>> + '#endif'
>
> Why not use G_BEGIN_DECLS?
I wasn't sure if tracetool dependend on glib. It's more a philosophical
question than an actual difference, of course.
Paolo
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH experiment 09/16] start adding extern "C" markers
2022-03-14 9:31 [PATCH experiment 00/16] C++20 coroutine backend Paolo Bonzini
` (7 preceding siblings ...)
2022-03-14 9:31 ` [PATCH experiment 08/16] tracetool: add extern "C" around generated headers Paolo Bonzini
@ 2022-03-14 9:31 ` Paolo Bonzini
2022-03-14 9:31 ` [PATCH experiment 10/16] add space between liter and string macro Paolo Bonzini
` (9 subsequent siblings)
18 siblings, 0 replies; 53+ messages in thread
From: Paolo Bonzini @ 2022-03-14 9:31 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, hreitz, berrange, qemu-block, stefanha
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/block/aio.h | 5 +++++
include/fpu/softfloat-types.h | 4 ++++
include/qemu/bitops.h | 3 +++
include/qemu/bswap.h | 10 +++-------
include/qemu/coroutine.h | 4 ++++
include/qemu/host-utils.h | 4 ++++
include/qemu/notify.h | 4 ++++
include/qemu/qsp.h | 4 ++++
include/qemu/thread.h | 4 ++++
include/qemu/timer.h | 4 ++++
10 files changed, 39 insertions(+), 7 deletions(-)
diff --git a/include/block/aio.h b/include/block/aio.h
index 5634173b12..4b21d95f0b 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -17,12 +17,15 @@
#ifdef CONFIG_LINUX_IO_URING
#include <liburing.h>
#endif
+
#include "qemu/coroutine.h"
#include "qemu/queue.h"
#include "qemu/event_notifier.h"
#include "qemu/thread.h"
#include "qemu/timer.h"
+G_BEGIN_DECLS
+
typedef struct BlockAIOCB BlockAIOCB;
typedef void BlockCompletionFunc(void *opaque, int ret);
@@ -769,4 +772,6 @@ void aio_context_set_poll_params(AioContext *ctx, int64_t max_ns,
void aio_context_set_aio_params(AioContext *ctx, int64_t max_batch,
Error **errp);
+G_END_DECLS
+
#endif
diff --git a/include/fpu/softfloat-types.h b/include/fpu/softfloat-types.h
index 8abd9ab4ec..aaf7b0b5fa 100644
--- a/include/fpu/softfloat-types.h
+++ b/include/fpu/softfloat-types.h
@@ -80,6 +80,8 @@ this code that are retained.
#ifndef SOFTFLOAT_TYPES_H
#define SOFTFLOAT_TYPES_H
+G_BEGIN_DECLS
+
/*
* Software IEC/IEEE floating-point types.
*/
@@ -197,4 +199,6 @@ typedef struct float_status {
bool no_signaling_nans;
} float_status;
+G_END_DECLS
+
#endif /* SOFTFLOAT_TYPES_H */
diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
index 03213ce952..677884bead 100644
--- a/include/qemu/bitops.h
+++ b/include/qemu/bitops.h
@@ -12,6 +12,7 @@
#ifndef BITOPS_H
#define BITOPS_H
+G_BEGIN_DECLS
#include "host-utils.h"
#include "atomic.h"
@@ -618,4 +619,6 @@ static inline uint64_t half_unshuffle64(uint64_t x)
return x;
}
+G_END_DECLS
+
#endif
diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index 2d3bb8bbed..439e755ed4 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -15,12 +15,10 @@
#define BSWAP_FROM_FALLBACKS
#endif /* ! CONFIG_MACHINE_BSWAP_H */
-#ifdef __cplusplus
-extern "C" {
-#endif
-
#include "fpu/softfloat-types.h"
+G_BEGIN_DECLS
+
#ifdef BSWAP_FROM_BYTESWAP
static inline uint16_t bswap16(uint16_t x)
{
@@ -508,8 +506,6 @@ DO_STN_LDN_P(be)
#undef le_bswaps
#undef be_bswaps
-#ifdef __cplusplus
-}
-#endif
+G_END_DECLS
#endif /* BSWAP_H */
diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index 6f4596fc5b..428e97d946 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -18,6 +18,8 @@
#include "qemu/queue.h"
#include "qemu/timer.h"
+G_BEGIN_DECLS
+
/**
* Coroutines are a mechanism for stack switching and can be used for
* cooperative userspace threading. These functions provide a simple but
@@ -341,6 +343,8 @@ void qemu_coroutine_increase_pool_batch_size(unsigned int additional_pool_size);
*/
void qemu_coroutine_decrease_pool_batch_size(unsigned int additional_pool_size);
+G_END_DECLS
+
#include "qemu/lockable.h"
#include "qemu/co-lockable.h"
diff --git a/include/qemu/host-utils.h b/include/qemu/host-utils.h
index ca979dc6cc..406e593dff 100644
--- a/include/qemu/host-utils.h
+++ b/include/qemu/host-utils.h
@@ -30,6 +30,8 @@
#ifndef HOST_UTILS_H
#define HOST_UTILS_H
+G_BEGIN_DECLS
+
#include "qemu/compiler.h"
#include "qemu/bswap.h"
@@ -849,4 +851,6 @@ static inline uint64_t udiv_qrnnd(uint64_t *r, uint64_t n1,
#endif
}
+G_END_DECLS
+
#endif
diff --git a/include/qemu/notify.h b/include/qemu/notify.h
index bcfa70fb2e..a8effa39b7 100644
--- a/include/qemu/notify.h
+++ b/include/qemu/notify.h
@@ -14,6 +14,8 @@
#ifndef QEMU_NOTIFY_H
#define QEMU_NOTIFY_H
+G_BEGIN_DECLS
+
#include "qemu/queue.h"
typedef struct Notifier Notifier;
@@ -71,4 +73,6 @@ void notifier_with_return_remove(NotifierWithReturn *notifier);
int notifier_with_return_list_notify(NotifierWithReturnList *list,
void *data);
+G_END_DECLS
+
#endif
diff --git a/include/qemu/qsp.h b/include/qemu/qsp.h
index bf36aabfa8..65389837a1 100644
--- a/include/qemu/qsp.h
+++ b/include/qemu/qsp.h
@@ -16,6 +16,8 @@ enum QSPSortBy {
QSP_SORT_BY_AVG_WAIT_TIME,
};
+G_BEGIN_DECLS
+
void qsp_report(size_t max, enum QSPSortBy sort_by,
bool callsite_coalesce);
@@ -24,4 +26,6 @@ void qsp_enable(void);
void qsp_disable(void);
void qsp_reset(void);
+G_END_DECLS
+
#endif /* QEMU_QSP_H */
diff --git a/include/qemu/thread.h b/include/qemu/thread.h
index 460568d67d..ec27b7ec58 100644
--- a/include/qemu/thread.h
+++ b/include/qemu/thread.h
@@ -22,6 +22,8 @@ typedef struct QemuThread QemuThread;
#define QEMU_THREAD_JOINABLE 0
#define QEMU_THREAD_DETACHED 1
+G_BEGIN_DECLS
+
void qemu_mutex_init(QemuMutex *mutex);
void qemu_mutex_destroy(QemuMutex *mutex);
int qemu_mutex_trylock_impl(QemuMutex *mutex, const char *file, const int line);
@@ -397,4 +399,6 @@ void qemu_lockcnt_inc_and_unlock(QemuLockCnt *lockcnt);
*/
unsigned qemu_lockcnt_count(QemuLockCnt *lockcnt);
+G_END_DECLS
+
#endif
diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index ee071e07d1..236c45f1c6 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -5,6 +5,8 @@
#include "qemu/notify.h"
#include "qemu/host-utils.h"
+G_BEGIN_DECLS
+
#define NANOSECONDS_PER_SECOND 1000000000LL
/* timers */
@@ -998,4 +1000,6 @@ static inline int64_t profile_getclock(void)
extern int64_t dev_time;
#endif
+G_END_DECLS
+
#endif
--
2.35.1
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH experiment 10/16] add space between liter and string macro
2022-03-14 9:31 [PATCH experiment 00/16] C++20 coroutine backend Paolo Bonzini
` (8 preceding siblings ...)
2022-03-14 9:31 ` [PATCH experiment 09/16] start adding extern "C" markers Paolo Bonzini
@ 2022-03-14 9:31 ` Paolo Bonzini
2022-03-14 9:31 ` [PATCH experiment 11/16] bump to C++20 Paolo Bonzini
` (8 subsequent siblings)
18 siblings, 0 replies; 53+ messages in thread
From: Paolo Bonzini @ 2022-03-14 9:31 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, hreitz, berrange, qemu-block, stefanha
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
util/trace-events | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/util/trace-events b/util/trace-events
index c8f53d7d9f..5bc718eff7 100644
--- a/util/trace-events
+++ b/util/trace-events
@@ -1,10 +1,10 @@
# See docs/devel/tracing.rst for syntax documentation.
# aio-posix.c
-run_poll_handlers_begin(void *ctx, int64_t max_ns, int64_t timeout) "ctx %p max_ns %"PRId64 " timeout %"PRId64
-run_poll_handlers_end(void *ctx, bool progress, int64_t timeout) "ctx %p progress %d new timeout %"PRId64
-poll_shrink(void *ctx, int64_t old, int64_t new) "ctx %p old %"PRId64" new %"PRId64
-poll_grow(void *ctx, int64_t old, int64_t new) "ctx %p old %"PRId64" new %"PRId64
+run_poll_handlers_begin(void *ctx, int64_t max_ns, int64_t timeout) "ctx %p max_ns %" PRId64 " timeout %" PRId64
+run_poll_handlers_end(void *ctx, bool progress, int64_t timeout) "ctx %p progress %d new timeout %" PRId64
+poll_shrink(void *ctx, int64_t old, int64_t new) "ctx %p old %" PRId64" new %" PRId64
+poll_grow(void *ctx, int64_t old, int64_t new) "ctx %p old %" PRId64" new %" PRId64
poll_add(void *ctx, void *node, int fd, unsigned revents) "ctx %p node %p fd %d revents 0x%x"
poll_remove(void *ctx, void *node, int fd) "ctx %p node %p fd %d"
@@ -52,9 +52,9 @@ qemu_vfree(void *ptr) "ptr %p"
qemu_anon_ram_free(void *ptr, size_t size) "ptr %p size %zu"
# hbitmap.c
-hbitmap_iter_skip_words(const void *hb, void *hbi, uint64_t pos, unsigned long cur) "hb %p hbi %p pos %"PRId64" cur 0x%lx"
-hbitmap_reset(void *hb, uint64_t start, uint64_t count, uint64_t sbit, uint64_t ebit) "hb %p items %"PRIu64",%"PRIu64" bits %"PRIu64"..%"PRIu64
-hbitmap_set(void *hb, uint64_t start, uint64_t count, uint64_t sbit, uint64_t ebit) "hb %p items %"PRIu64",%"PRIu64" bits %"PRIu64"..%"PRIu64
+hbitmap_iter_skip_words(const void *hb, void *hbi, uint64_t pos, unsigned long cur) "hb %p hbi %p pos %" PRId64" cur 0x%lx"
+hbitmap_reset(void *hb, uint64_t start, uint64_t count, uint64_t sbit, uint64_t ebit) "hb %p items %" PRIu64",%" PRIu64" bits %" PRIu64"..%" PRIu64
+hbitmap_set(void *hb, uint64_t start, uint64_t count, uint64_t sbit, uint64_t ebit) "hb %p items %" PRIu64",%" PRIu64" bits %" PRIu64"..%" PRIu64
# lockcnt.c
lockcnt_fast_path_attempt(const void *lockcnt, int expected, int new) "lockcnt %p fast path %d->%d"
@@ -82,15 +82,15 @@ qemu_vfio_ram_block_added(void *s, void *p, size_t size) "s %p host %p size 0x%z
qemu_vfio_ram_block_removed(void *s, void *p, size_t size) "s %p host %p size 0x%zx"
qemu_vfio_dump_mapping(void *host, uint64_t iova, size_t size) "vfio mapping %p to iova 0x%08" PRIx64 " size 0x%zx"
qemu_vfio_find_mapping(void *s, void *p) "s %p host %p"
-qemu_vfio_new_mapping(void *s, void *host, size_t size, int index, uint64_t iova) "s %p host %p size 0x%zx index %d iova 0x%"PRIx64
-qemu_vfio_do_mapping(void *s, void *host, uint64_t iova, size_t size) "s %p host %p <-> iova 0x%"PRIx64 " size 0x%zx"
+qemu_vfio_new_mapping(void *s, void *host, size_t size, int index, uint64_t iova) "s %p host %p size 0x%zx index %d iova 0x%" PRIx64
+qemu_vfio_do_mapping(void *s, void *host, uint64_t iova, size_t size) "s %p host %p <-> iova 0x%" PRIx64 " size 0x%zx"
qemu_vfio_dma_map(void *s, void *host, size_t size, bool temporary, uint64_t *iova) "s %p host %p size 0x%zx temporary %d &iova %p"
-qemu_vfio_dma_mapped(void *s, void *host, uint64_t iova, size_t size) "s %p host %p <-> iova 0x%"PRIx64" size 0x%zx"
+qemu_vfio_dma_mapped(void *s, void *host, uint64_t iova, size_t size) "s %p host %p <-> iova 0x%" PRIx64" size 0x%zx"
qemu_vfio_dma_unmap(void *s, void *host) "s %p host %p"
-qemu_vfio_pci_read_config(void *buf, int ofs, int size, uint64_t region_ofs, uint64_t region_size) "read cfg ptr %p ofs 0x%x size 0x%x (region addr 0x%"PRIx64" size 0x%"PRIx64")"
-qemu_vfio_pci_write_config(void *buf, int ofs, int size, uint64_t region_ofs, uint64_t region_size) "write cfg ptr %p ofs 0x%x size 0x%x (region addr 0x%"PRIx64" size 0x%"PRIx64")"
-qemu_vfio_region_info(const char *desc, uint64_t region_ofs, uint64_t region_size, uint32_t cap_offset) "region '%s' addr 0x%"PRIx64" size 0x%"PRIx64" cap_ofs 0x%"PRIx32
-qemu_vfio_pci_map_bar(int index, uint64_t region_ofs, uint64_t region_size, int ofs, void *host) "map region bar#%d addr 0x%"PRIx64" size 0x%"PRIx64" ofs 0x%x host %p"
+qemu_vfio_pci_read_config(void *buf, int ofs, int size, uint64_t region_ofs, uint64_t region_size) "read cfg ptr %p ofs 0x%x size 0x%x (region addr 0x%" PRIx64" size 0x%" PRIx64")"
+qemu_vfio_pci_write_config(void *buf, int ofs, int size, uint64_t region_ofs, uint64_t region_size) "write cfg ptr %p ofs 0x%x size 0x%x (region addr 0x%" PRIx64" size 0x%" PRIx64")"
+qemu_vfio_region_info(const char *desc, uint64_t region_ofs, uint64_t region_size, uint32_t cap_offset) "region '%s' addr 0x%" PRIx64" size 0x%" PRIx64" cap_ofs 0x%" PRIx32
+qemu_vfio_pci_map_bar(int index, uint64_t region_ofs, uint64_t region_size, int ofs, void *host) "map region bar#%d addr 0x%" PRIx64" size 0x%" PRIx64" ofs 0x%x host %p"
#userfaultfd.c
uffd_query_features_nosys(int err) "errno: %i"
--
2.35.1
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH experiment 11/16] bump to C++20
2022-03-14 9:31 [PATCH experiment 00/16] C++20 coroutine backend Paolo Bonzini
` (9 preceding siblings ...)
2022-03-14 9:31 ` [PATCH experiment 10/16] add space between liter and string macro Paolo Bonzini
@ 2022-03-14 9:31 ` Paolo Bonzini
2022-03-14 9:31 ` [PATCH experiment 12/16] remove "new" keyword from trace-events Paolo Bonzini
` (7 subsequent siblings)
18 siblings, 0 replies; 53+ messages in thread
From: Paolo Bonzini @ 2022-03-14 9:31 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, hreitz, berrange, qemu-block, stefanha
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
configure | 4 ++--
meson.build | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/configure b/configure
index 886000346a..091710ec03 100755
--- a/configure
+++ b/configure
@@ -157,8 +157,8 @@ update_cxxflags() {
# Set QEMU_CXXFLAGS from QEMU_CFLAGS by filtering out those
# options which some versions of GCC's C++ compiler complain about
# because they only make sense for C programs.
- QEMU_CXXFLAGS="-D__STDC_LIMIT_MACROS -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS"
- CONFIGURE_CXXFLAGS=$(echo "$CONFIGURE_CFLAGS" | sed s/-std=gnu11/-std=gnu++11/)
+ QEMU_CXXFLAGS="-D__STDC_LIMIT_MACROS -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -fcoroutines"
+ CONFIGURE_CXXFLAGS=$(echo "$CONFIGURE_CFLAGS" | sed s/-std=gnu11/-std=gnu++20/)
for arg in $QEMU_CFLAGS; do
case $arg in
-Wstrict-prototypes|-Wmissing-prototypes|-Wnested-externs|\
diff --git a/meson.build b/meson.build
index 2d6601467f..810ebb0865 100644
--- a/meson.build
+++ b/meson.build
@@ -1,5 +1,5 @@
project('qemu', ['c'], meson_version: '>=0.59.3',
- default_options: ['warning_level=1', 'c_std=gnu11', 'cpp_std=gnu++11', 'b_colorout=auto',
+ default_options: ['warning_level=1', 'c_std=gnu11', 'cpp_std=gnu++20', 'b_colorout=auto',
'b_staticpic=false', 'stdsplit=false'],
version: files('VERSION'))
--
2.35.1
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH experiment 12/16] remove "new" keyword from trace-events
2022-03-14 9:31 [PATCH experiment 00/16] C++20 coroutine backend Paolo Bonzini
` (10 preceding siblings ...)
2022-03-14 9:31 ` [PATCH experiment 11/16] bump to C++20 Paolo Bonzini
@ 2022-03-14 9:31 ` Paolo Bonzini
2022-03-14 13:30 ` Philippe Mathieu-Daudé
2022-03-14 9:32 ` [PATCH experiment 13/16] disable some code Paolo Bonzini
` (6 subsequent siblings)
18 siblings, 1 reply; 53+ messages in thread
From: Paolo Bonzini @ 2022-03-14 9:31 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, hreitz, berrange, qemu-block, stefanha
This is invalid in C++.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
util/trace-events | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/util/trace-events b/util/trace-events
index 5bc718eff7..9e23c11d11 100644
--- a/util/trace-events
+++ b/util/trace-events
@@ -3,8 +3,8 @@
# aio-posix.c
run_poll_handlers_begin(void *ctx, int64_t max_ns, int64_t timeout) "ctx %p max_ns %" PRId64 " timeout %" PRId64
run_poll_handlers_end(void *ctx, bool progress, int64_t timeout) "ctx %p progress %d new timeout %" PRId64
-poll_shrink(void *ctx, int64_t old, int64_t new) "ctx %p old %" PRId64" new %" PRId64
-poll_grow(void *ctx, int64_t old, int64_t new) "ctx %p old %" PRId64" new %" PRId64
+poll_shrink(void *ctx, int64_t prev, int64_t curr) "ctx %p old %" PRId64" new %" PRId64
+poll_grow(void *ctx, int64_t prev, int64_t curr) "ctx %p old %" PRId64" new %" PRId64
poll_add(void *ctx, void *node, int fd, unsigned revents) "ctx %p node %p fd %d revents 0x%x"
poll_remove(void *ctx, void *node, int fd) "ctx %p node %p fd %d"
@@ -57,13 +57,13 @@ hbitmap_reset(void *hb, uint64_t start, uint64_t count, uint64_t sbit, uint64_t
hbitmap_set(void *hb, uint64_t start, uint64_t count, uint64_t sbit, uint64_t ebit) "hb %p items %" PRIu64",%" PRIu64" bits %" PRIu64"..%" PRIu64
# lockcnt.c
-lockcnt_fast_path_attempt(const void *lockcnt, int expected, int new) "lockcnt %p fast path %d->%d"
-lockcnt_fast_path_success(const void *lockcnt, int expected, int new) "lockcnt %p fast path %d->%d succeeded"
-lockcnt_unlock_attempt(const void *lockcnt, int expected, int new) "lockcnt %p unlock %d->%d"
-lockcnt_unlock_success(const void *lockcnt, int expected, int new) "lockcnt %p unlock %d->%d succeeded"
-lockcnt_futex_wait_prepare(const void *lockcnt, int expected, int new) "lockcnt %p preparing slow path %d->%d"
+lockcnt_fast_path_attempt(const void *lockcnt, int expected, int newval) "lockcnt %p fast path %d->%d"
+lockcnt_fast_path_success(const void *lockcnt, int expected, int newval) "lockcnt %p fast path %d->%d succeeded"
+lockcnt_unlock_attempt(const void *lockcnt, int expected, int newval) "lockcnt %p unlock %d->%d"
+lockcnt_unlock_success(const void *lockcnt, int expected, int newval) "lockcnt %p unlock %d->%d succeeded"
+lockcnt_futex_wait_prepare(const void *lockcnt, int expected, int newval) "lockcnt %p preparing slow path %d->%d"
lockcnt_futex_wait(const void *lockcnt, int val) "lockcnt %p waiting on %d"
-lockcnt_futex_wait_resume(const void *lockcnt, int new) "lockcnt %p after wait: %d"
+lockcnt_futex_wait_resume(const void *lockcnt, int newval) "lockcnt %p after wait: %d"
lockcnt_futex_wake(const void *lockcnt) "lockcnt %p waking up one waiter"
# qemu-sockets.c
--
2.35.1
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH experiment 13/16] disable some code
2022-03-14 9:31 [PATCH experiment 00/16] C++20 coroutine backend Paolo Bonzini
` (11 preceding siblings ...)
2022-03-14 9:31 ` [PATCH experiment 12/16] remove "new" keyword from trace-events Paolo Bonzini
@ 2022-03-14 9:32 ` Paolo Bonzini
2022-03-14 9:32 ` [PATCH experiment 14/16] util: introduce C++ stackless coroutine backend Paolo Bonzini
` (5 subsequent siblings)
18 siblings, 0 replies; 53+ messages in thread
From: Paolo Bonzini @ 2022-03-14 9:32 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, hreitz, berrange, qemu-block, stefanha
Disable a lot of code that I can't be bothered to convert right now.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/coroutine.h | 2 ++
qapi/qmp-dispatch.c | 2 ++
tests/unit/meson.build | 2 +-
tests/unit/test-coroutine.c | 6 ++++++
util/async.c | 2 ++
util/meson.build | 10 +++++-----
util/qemu-coroutine-lock.c | 2 ++
util/qemu-coroutine-sleep.c | 2 ++
util/thread-pool.c | 2 ++
9 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index 428e97d946..ac9891502e 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -200,6 +200,7 @@ typedef struct CoQueue {
*/
void qemu_co_queue_init(CoQueue *queue);
+#if 0
/**
* Adds the current coroutine to the CoQueue and transfers control to the
* caller of the coroutine. The mutex is unlocked during the wait and
@@ -208,6 +209,7 @@ void qemu_co_queue_init(CoQueue *queue);
#define qemu_co_queue_wait(queue, lock) \
qemu_co_queue_wait_impl(queue, QEMU_MAKE_CO_LOCKABLE(lock))
void coroutine_fn qemu_co_queue_wait_impl(CoQueue *queue, QemuCoLockable *lock);
+#endif
/**
* Removes the next coroutine from the CoQueue, and wake it up.
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index d378bccac7..6a4633c133 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -235,7 +235,9 @@ QDict *qmp_dispatch(const QmpCommandList *cmds, QObject *request,
};
aio_bh_schedule_oneshot(qemu_get_aio_context(), do_qmp_dispatch_bh,
&data);
+#if 0
qemu_coroutine_yield();
+#endif
}
qobject_unref(args);
if (err) {
diff --git a/tests/unit/meson.build b/tests/unit/meson.build
index 96b295263e..4ca5fdb699 100644
--- a/tests/unit/meson.build
+++ b/tests/unit/meson.build
@@ -61,7 +61,7 @@ endif
if have_block
tests += {
- 'test-coroutine': [testblock],
+ 'test-coroutine': [],
'test-aio': [testblock],
'test-aio-multithread': [testblock],
'test-throttle': [testblock],
diff --git a/tests/unit/test-coroutine.c b/tests/unit/test-coroutine.c
index 82e22db070..c230c2fa6e 100644
--- a/tests/unit/test-coroutine.c
+++ b/tests/unit/test-coroutine.c
@@ -16,6 +16,7 @@
#include "qemu/coroutine_int.h"
#include "qemu/lockable.h"
+#if 0
/*
* Check that qemu_in_coroutine() works
*/
@@ -638,11 +639,13 @@ static void perf_cost(void)
duration, ops,
(unsigned long)(1000000000.0 * duration / maxcycles));
}
+#endif
int main(int argc, char **argv)
{
g_test_init(&argc, &argv, NULL);
+#if 0
/* This test assumes there is a freelist and marks freed coroutine memory
* with a sentinel value. If there is no freelist this would legitimately
* crash, so skip it.
@@ -650,7 +653,9 @@ int main(int argc, char **argv)
if (CONFIG_COROUTINE_POOL) {
g_test_add_func("/basic/no-dangling-access", test_no_dangling_access);
}
+#endif
+#if 0
g_test_add_func("/basic/lifecycle", test_lifecycle);
g_test_add_func("/basic/yield", test_yield);
g_test_add_func("/basic/nesting", test_nesting);
@@ -669,5 +674,6 @@ int main(int argc, char **argv)
g_test_add_func("/perf/function-call", perf_baseline);
g_test_add_func("/perf/cost", perf_cost);
}
+#endif
return g_test_run();
}
diff --git a/util/async.c b/util/async.c
index 2ea1172f3e..95a9e0f95f 100644
--- a/util/async.c
+++ b/util/async.c
@@ -595,6 +595,7 @@ void aio_co_schedule(AioContext *ctx, Coroutine *co)
aio_context_unref(ctx);
}
+#if 0
typedef struct AioCoRescheduleSelf {
Coroutine *co;
AioContext *new_ctx;
@@ -624,6 +625,7 @@ void coroutine_fn aio_co_reschedule_self(AioContext *new_ctx)
qemu_coroutine_yield();
}
}
+#endif
void aio_co_wake(struct Coroutine *co)
{
diff --git a/util/meson.build b/util/meson.build
index f6ee74ad0c..30949cd481 100644
--- a/util/meson.build
+++ b/util/meson.build
@@ -76,13 +76,13 @@ if have_block
util_ss.add(files('lockcnt.c'))
util_ss.add(files('main-loop.c'))
util_ss.add(files('nvdimm-utils.c'))
- util_ss.add(files('qemu-coroutine.c', 'qemu-coroutine-lock.c', 'qemu-coroutine-io.c'))
- util_ss.add(when: 'CONFIG_LINUX', if_true: [
- files('vhost-user-server.c'), vhost_user
- ])
+ util_ss.add(files('qemu-coroutine.c', 'qemu-coroutine-lock.c')) # 'qemu-coroutine-io.c'
+# util_ss.add(when: 'CONFIG_LINUX', if_true: [
+# files('vhost-user-server.c'), vhost_user
+# ])
util_ss.add(files('block-helpers.c'))
util_ss.add(files('qemu-coroutine-sleep.c'))
- util_ss.add(files('qemu-co-shared-resource.c'))
+# util_ss.add(files('qemu-co-shared-resource.c'))
util_ss.add(files('thread-pool.c', 'qemu-timer.c'))
util_ss.add(files('readline.c'))
util_ss.add(files('throttle.c'))
diff --git a/util/qemu-coroutine-lock.c b/util/qemu-coroutine-lock.c
index 3f12b53a31..d6c0565ba5 100644
--- a/util/qemu-coroutine-lock.c
+++ b/util/qemu-coroutine-lock.c
@@ -34,6 +34,7 @@
#include "block/aio.h"
#include "trace.h"
+#if 0
void qemu_co_queue_init(CoQueue *queue)
{
QSIMPLEQ_INIT(&queue->entries);
@@ -465,3 +466,4 @@ void qemu_co_rwlock_upgrade(CoRwlock *lock)
assert(lock->owners == -1);
}
}
+#endif
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index 571ab521ff..b5bfb4ad18 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -17,6 +17,7 @@
#include "qemu/timer.h"
#include "block/aio.h"
+#if 0
static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";
void qemu_co_sleep_wake(QemuCoSleep *w)
@@ -78,3 +79,4 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
qemu_co_sleep(w);
timer_del(&ts);
}
+#endif
diff --git a/util/thread-pool.c b/util/thread-pool.c
index d763cea505..f621f69a91 100644
--- a/util/thread-pool.c
+++ b/util/thread-pool.c
@@ -266,6 +266,7 @@ BlockAIOCB *thread_pool_submit_aio(ThreadPool *pool,
return &req->common;
}
+#if 0
typedef struct ThreadPoolCo {
Coroutine *co;
int ret;
@@ -288,6 +289,7 @@ int coroutine_fn thread_pool_submit_co(ThreadPool *pool, ThreadPoolFunc *func,
qemu_coroutine_yield();
return tpc.ret;
}
+#endif
void thread_pool_submit(ThreadPool *pool, ThreadPoolFunc *func, void *arg)
{
--
2.35.1
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH experiment 14/16] util: introduce C++ stackless coroutine backend
2022-03-14 9:31 [PATCH experiment 00/16] C++20 coroutine backend Paolo Bonzini
` (12 preceding siblings ...)
2022-03-14 9:32 ` [PATCH experiment 13/16] disable some code Paolo Bonzini
@ 2022-03-14 9:32 ` Paolo Bonzini
2022-03-14 14:37 ` Stefan Hajnoczi
2022-03-14 9:32 ` [PATCH experiment 15/16] port QemuCoLockable to C++ coroutines Paolo Bonzini
` (4 subsequent siblings)
18 siblings, 1 reply; 53+ messages in thread
From: Paolo Bonzini @ 2022-03-14 9:32 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, hreitz, berrange, qemu-block, stefanha
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
configure | 44 +-
include/qemu/coroutine.h | 445 +++++++++++++-----
include/qemu/coroutine_int.h | 8 +
util/coroutine-stackless.cc | 145 ++++++
util/meson.build | 6 +-
...oroutine-lock.c => qemu-coroutine-lock.cc} | 70 +--
...outine-sleep.c => qemu-coroutine-sleep.cc} | 12 +-
util/{qemu-coroutine.c => qemu-coroutine.cc} | 16 -
8 files changed, 530 insertions(+), 216 deletions(-)
create mode 100644 util/coroutine-stackless.cc
rename util/{qemu-coroutine-lock.c => qemu-coroutine-lock.cc} (87%)
rename util/{qemu-coroutine-sleep.c => qemu-coroutine-sleep.cc} (89%)
rename util/{qemu-coroutine.c => qemu-coroutine.cc} (94%)
diff --git a/configure b/configure
index 091710ec03..c02b5edcba 100755
--- a/configure
+++ b/configure
@@ -1220,8 +1220,6 @@ Advanced options (experts only):
--with-trace-file=NAME Full PATH,NAME of file to store traces
Default:trace-<pid>
--cpu=CPU Build for host CPU [$cpu]
- --with-coroutine=BACKEND coroutine backend. Supported options:
- ucontext, sigaltstack, windows
--enable-gcov enable test coverage analysis with gcov
--tls-priority default TLS protocol/cipher priority string
--enable-plugins
@@ -1242,7 +1240,7 @@ cat << EOF
debug-info debugging information
lto Enable Link-Time Optimization.
safe-stack SafeStack Stack Smash Protection. Depends on
- clang/llvm >= 3.7 and requires coroutine backend ucontext.
+ clang/llvm >= 3.7
rdma Enable RDMA-based migration
pvrdma Enable PVRDMA support
vhost-net vhost-net kernel acceleration support
@@ -2338,39 +2336,7 @@ EOF
fi
fi
-if test "$coroutine" = ""; then
- if test "$mingw32" = "yes"; then
- coroutine=win32
- elif test "$ucontext_works" = "yes"; then
- coroutine=ucontext
- else
- coroutine=sigaltstack
- fi
-else
- case $coroutine in
- windows)
- if test "$mingw32" != "yes"; then
- error_exit "'windows' coroutine backend only valid for Windows"
- fi
- # Unfortunately the user visible backend name doesn't match the
- # coroutine-*.c filename for this case, so we have to adjust it here.
- coroutine=win32
- ;;
- ucontext)
- if test "$ucontext_works" != "yes"; then
- feature_not_found "ucontext"
- fi
- ;;
- sigaltstack)
- if test "$mingw32" = "yes"; then
- error_exit "only the 'windows' coroutine backend is valid for Windows"
- fi
- ;;
- *)
- error_exit "unknown coroutine backend $coroutine"
- ;;
- esac
-fi
+coroutine=stackless
##################################################
# SafeStack
@@ -2395,9 +2361,6 @@ EOF
else
error_exit "SafeStack not supported by your compiler"
fi
- if test "$coroutine" != "ucontext"; then
- error_exit "SafeStack is only supported by the coroutine backend ucontext"
- fi
else
cat > $TMPC << EOF
int main(int argc, char *argv[])
@@ -2427,9 +2390,6 @@ else # "$safe_stack" = ""
safe_stack="no"
else
safe_stack="yes"
- if test "$coroutine" != "ucontext"; then
- error_exit "SafeStack is only supported by the coroutine backend ucontext"
- fi
fi
fi
fi
diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index ac9891502e..0f89fbafa0 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -48,25 +48,6 @@ G_BEGIN_DECLS
typedef struct Coroutine Coroutine;
-/**
- * Coroutine entry point
- *
- * When the coroutine is entered for the first time, opaque is passed in as an
- * argument.
- *
- * When this function returns, the coroutine is destroyed automatically and
- * execution continues in the caller who last entered the coroutine.
- */
-typedef void coroutine_fn CoroutineEntry(void *opaque);
-
-/**
- * Create a new coroutine
- *
- * Use qemu_coroutine_enter() to actually transfer control to the coroutine.
- * The opaque argument is passed as the argument to the entry point.
- */
-Coroutine *qemu_coroutine_create(CoroutineEntry *entry, void *opaque);
-
/**
* Transfer control to a coroutine
*/
@@ -83,14 +64,6 @@ void qemu_coroutine_enter_if_inactive(Coroutine *co);
*/
void qemu_aio_coroutine_enter(AioContext *ctx, Coroutine *co);
-/**
- * Transfer control back to a coroutine's caller
- *
- * This function does not return until the coroutine is re-entered using
- * qemu_coroutine_enter().
- */
-void coroutine_fn qemu_coroutine_yield(void);
-
/**
* Get the AioContext of the given coroutine
*/
@@ -157,18 +130,6 @@ struct CoMutex {
*/
void qemu_co_mutex_init(CoMutex *mutex);
-/**
- * Locks the mutex. If the lock cannot be taken immediately, control is
- * transferred to the caller of the current coroutine.
- */
-void coroutine_fn qemu_co_mutex_lock(CoMutex *mutex);
-
-/**
- * Unlocks the mutex and schedules the next coroutine that was waiting for this
- * lock to be run.
- */
-void coroutine_fn qemu_co_mutex_unlock(CoMutex *mutex);
-
/**
* Assert that the current coroutine holds @mutex.
*/
@@ -200,17 +161,6 @@ typedef struct CoQueue {
*/
void qemu_co_queue_init(CoQueue *queue);
-#if 0
-/**
- * Adds the current coroutine to the CoQueue and transfers control to the
- * caller of the coroutine. The mutex is unlocked during the wait and
- * locked again afterwards.
- */
-#define qemu_co_queue_wait(queue, lock) \
- qemu_co_queue_wait_impl(queue, QEMU_MAKE_CO_LOCKABLE(lock))
-void coroutine_fn qemu_co_queue_wait_impl(CoQueue *queue, QemuCoLockable *lock);
-#endif
-
/**
* Removes the next coroutine from the CoQueue, and wake it up.
* Returns true if a coroutine was removed, false if the queue is empty.
@@ -260,66 +210,10 @@ typedef struct CoRwlock {
*/
void qemu_co_rwlock_init(CoRwlock *lock);
-/**
- * Read locks the CoRwlock. If the lock cannot be taken immediately because
- * of a parallel writer, control is transferred to the caller of the current
- * coroutine.
- */
-void coroutine_fn qemu_co_rwlock_rdlock(CoRwlock *lock);
-
-/**
- * Write Locks the CoRwlock from a reader. This is a bit more efficient than
- * @qemu_co_rwlock_unlock followed by a separate @qemu_co_rwlock_wrlock.
- * Note that if the lock cannot be upgraded immediately, control is transferred
- * to the caller of the current coroutine; another writer might run while
- * @qemu_co_rwlock_upgrade blocks.
- */
-void coroutine_fn qemu_co_rwlock_upgrade(CoRwlock *lock);
-
-/**
- * Downgrades a write-side critic section to a reader. Downgrading with
- * @qemu_co_rwlock_downgrade never blocks, unlike @qemu_co_rwlock_unlock
- * followed by @qemu_co_rwlock_rdlock. This makes it more efficient, but
- * may also sometimes be necessary for correctness.
- */
-void coroutine_fn qemu_co_rwlock_downgrade(CoRwlock *lock);
-
-/**
- * Write Locks the mutex. If the lock cannot be taken immediately because
- * of a parallel reader, control is transferred to the caller of the current
- * coroutine.
- */
-void coroutine_fn qemu_co_rwlock_wrlock(CoRwlock *lock);
-
-/**
- * Unlocks the read/write lock and schedules the next coroutine that was
- * waiting for this lock to be run.
- */
-void coroutine_fn qemu_co_rwlock_unlock(CoRwlock *lock);
-
typedef struct QemuCoSleep {
Coroutine *to_wake;
} QemuCoSleep;
-/**
- * Yield the coroutine for a given duration. Initializes @w so that,
- * during this yield, it can be passed to qemu_co_sleep_wake() to
- * terminate the sleep.
- */
-void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
- QEMUClockType type, int64_t ns);
-
-/**
- * Yield the coroutine until the next call to qemu_co_sleep_wake.
- */
-void coroutine_fn qemu_co_sleep(QemuCoSleep *w);
-
-static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
-{
- QemuCoSleep w = { 0 };
- qemu_co_sleep_ns_wakeable(&w, type, ns);
-}
-
/**
* Wake a coroutine if it is sleeping in qemu_co_sleep_ns. The timer will be
* deleted. @sleep_state must be the variable whose address was given to
@@ -328,13 +222,6 @@ static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
*/
void qemu_co_sleep_wake(QemuCoSleep *w);
-/**
- * Yield until a file descriptor becomes readable
- *
- * Note that this function clobbers the handlers for the file descriptor.
- */
-void coroutine_fn yield_until_fd_readable(int fd);
-
/**
* Increase coroutine pool size
*/
@@ -348,6 +235,338 @@ void qemu_coroutine_decrease_pool_batch_size(unsigned int additional_pool_size);
G_END_DECLS
#include "qemu/lockable.h"
+
+#ifdef __cplusplus
+#include <cstdint>
+#include <coroutine>
+#include <exception>
+
+// BaseCoroutine is a simple wrapper type for a Promise. It mostly
+// exists because C++ says so, but it also provides two extra features:
+// RAII destruction of the coroutine (which is more efficient but
+// beware, the promise's final_suspend must always suspend to avoid
+// double free) and a cast to std::coroutine_handle<>, which makes
+// it resumable.
+
+template<typename Promise> struct BaseCoroutine
+{
+ using promise_type = Promise;
+
+ BaseCoroutine() = default;
+ explicit BaseCoroutine (Promise &promise) :
+ _coroutine{std::coroutine_handle<Promise>::from_promise(promise)} {}
+
+ BaseCoroutine(BaseCoroutine const&) = delete;
+ BaseCoroutine(BaseCoroutine&& other) : _coroutine{other._coroutine} {
+ other._coroutine = nullptr;
+ }
+
+ BaseCoroutine& operator=(BaseCoroutine const&) = delete;
+ BaseCoroutine& operator=(BaseCoroutine&& other) {
+ if (&other != this) {
+ _coroutine = other._coroutine;
+ other._coroutine = nullptr;
+ }
+ return *this;
+ }
+
+ ~BaseCoroutine() {
+ //printf("!!!! destroying %p\n", _coroutine);
+ if (_coroutine) _coroutine.destroy();
+ }
+
+ operator bool() const noexcept {
+ return _coroutine;
+ }
+ operator std::coroutine_handle<>() const noexcept {
+ return _coroutine;
+ }
+ Promise &promise() const noexcept {
+ return _coroutine.promise();
+ }
+
+private:
+ std::coroutine_handle<Promise> _coroutine = nullptr;
+};
+
+// This is a simple awaitable object that takes care of resuming a
+// parent coroutine. It's needed because co_await suspends all
+// parent coroutines on the stack. It does not need a specific
+// "kind" of coroutine_handle, so no need to put it inside the
+// templates below.
+//
+// If next is NULL, then this degrades to std::suspend_always.
+
+struct ResumeAndFinish {
+ explicit ResumeAndFinish(std::coroutine_handle<> next) noexcept :
+ _next{next} {}
+
+ bool await_ready() const noexcept {
+ return false;
+ }
+ bool await_suspend(std::coroutine_handle<> ch) const noexcept {
+ if (_next) {
+ _next.resume();
+ }
+ return true;
+ }
+ void await_resume() const noexcept {}
+
+private:
+ std::coroutine_handle<> _next;
+};
+
+// ------------------------
+
+// CoroutineFn does not even need anything more than what
+// BaseCoroutine provides, so it's just a type alias. The magic
+// is all in ValuePromise<T>.
+//
+// Suspended CoroutineFns are chained between themselves. Whenever a
+// coroutine is suspended, all those that have done a co_await are
+// also suspended, and whenever a coroutine finishes, it has to
+// check if its parent can now be resumed.
+//
+// The two auxiliary classes Awaiter and ResumeAndFinish take
+// care of the two sides of this. Awaiter's await_suspend() stores
+// the parent coroutine into ValuePromise; ResumeAndFinish's runs
+// after a coroutine returns, and resumes the parent coroutine.
+
+template<typename T> struct ValuePromise;
+template<typename T>
+using CoroutineFn = BaseCoroutine<ValuePromise<T>>;
+
+typedef CoroutineFn<void> CoroutineFunc(void *);
+
+// Unfortunately it is forbidden to define both return_void() and
+// return_value() in the same class. In order to cut on the
+// code duplication, define a superclass for both ValuePromise<T>
+// and ValuePromise<void>.
+//
+// The "curiously recurring template pattern" is used to substitute
+// ValuePromise<T> into the methods of the base class and its Awaited.
+// For example await_resume() needs to retrieve a value with the
+// correct type from the subclass's value() method.
+
+template<typename T, typename Derived>
+struct BasePromise
+{
+ using coro_handle_type = std::coroutine_handle<Derived>;
+
+#if 0
+ // Same as get_return_object().address() but actually works.
+ // Useful as an identifier to identify the promise in debugging
+ // output, because it matches the values passed to await_suspend().
+ void *coro_address() const {
+ return __builtin_coro_promise((char *)this, __alignof(*this), true);
+ }
+
+ BasePromise() {
+ printf("!!!! created %p\n", coro_address());
+ }
+ ~BasePromise() {
+ printf("!!!! destroyed %p\n", coro_address());
+ }
+#endif
+
+ CoroutineFn<T> get_return_object() noexcept { return CoroutineFn<T>{downcast()}; }
+ void unhandled_exception() { std::terminate(); }
+ auto initial_suspend() const noexcept { return std::suspend_never{}; }
+ auto final_suspend() noexcept {
+ auto continuation = ResumeAndFinish{_next};
+ mark_ready();
+ return continuation;
+ }
+private:
+ std::coroutine_handle<> _next = nullptr;
+
+ static const std::uintptr_t READY_MARKER = 1;
+ void mark_ready() {
+ _next = std::coroutine_handle<>::from_address((void *)READY_MARKER);
+ }
+
+ bool is_ready() const {
+ return _next.address() == (void *)READY_MARKER;
+ }
+
+ Derived& downcast() noexcept { return *static_cast<Derived*>(this); }
+ Derived const& downcast() const noexcept { return *static_cast<const Derived*>(this); }
+
+ // This records the parent coroutine, before a co_await suspends
+ // all parent coroutines on the stack.
+ void then(std::coroutine_handle<> parent) { _next = parent; }
+
+ // This is the object that lets us co_await a CoroutineFn<T> (of which
+ // this class is the corresponding promise object). This is just mapping
+ // C++ awaitable naming into the more convention promise naming.
+ struct Awaiter {
+ Derived &_promise;
+
+ explicit Awaiter(Derived &promise) : _promise{promise} {}
+
+ bool await_ready() const noexcept {
+ return _promise.is_ready();
+ }
+
+ void await_suspend(std::coroutine_handle<> parent) const noexcept {
+ _promise.then(parent);
+ }
+
+ Derived::await_resume_type await_resume() const noexcept {
+ return _promise.value();
+ }
+ };
+
+ // C++ connoisseurs will tell you that this is not private.
+ friend Awaiter operator co_await(CoroutineFn<T> co) {
+ return Awaiter{co.promise()};
+ }
+};
+
+// The actu promises, respectively for non-void and void types.
+// All that's left is storing and retrieving the value.
+
+template<typename T>
+struct ValuePromise: BasePromise<T, ValuePromise<T>>
+{
+ using await_resume_type = T&&;
+ T _value;
+ void return_value(T&& value) { _value = std::move(value); }
+ void return_value(T const& value) { _value = value; }
+ T&& value() noexcept { return static_cast<T&&>(_value); }
+};
+
+template<>
+struct ValuePromise<void>: BasePromise<void, ValuePromise<void>>
+{
+ using await_resume_type = void;
+ void return_void() const {}
+ void value() const {}
+};
+
+
+// ---------------------------
+
+// This class takes care of yielding, which is just a matter of doing
+// "co_await Yield{}". This always suspends, and also stores the
+// suspending CoroutineFn in current->top.
+struct Yield: std::suspend_always {
+ void await_suspend(std::coroutine_handle<> parent) const noexcept;
+};
+
+// ---------------------------
+
+// Make it possible to write "co_await qemu_coroutine_yield()"
+static inline Yield qemu_coroutine_yield()
+{
+ return Yield{};
+}
+
+/**
+ * Coroutine entry point
+ *
+ * When the coroutine is entered for the first time, opaque is passed in as an
+ * argument.
+ *
+ * When this function returns, the coroutine is destroyed automatically and
+ * execution continues in the caller who last entered the coroutine.
+ */
+typedef CoroutineFn<void> CoroutineEntry(void *opaque);
+
+/**
+ * Create a new coroutine
+ *
+ * Use qemu_coroutine_enter() to actually transfer control to the coroutine.
+ * The opaque argument is passed as the argument to the entry point.
+ */
+Coroutine *qemu_coroutine_create(CoroutineEntry *entry, void *opaque);
+
+/**
+ * Adds the current coroutine to the CoQueue and transfers control to the
+ * caller of the coroutine. The mutex is unlocked during the wait and
+ * locked again afterwards.
+ */
+#define qemu_co_queue_wait(queue, lock) \
+ qemu_co_queue_wait_impl(queue, QEMU_MAKE_CO_LOCKABLE(lock))
+CoroutineFn<void> qemu_co_queue_wait_impl(CoQueue *queue, QemuCoLockable *lock);
+
+/**
+ * Locks the mutex. If the lock cannot be taken immediately, control is
+ * transferred to the caller of the current coroutine.
+ */
+CoroutineFn<void> qemu_co_mutex_lock(CoMutex *mutex);
+
+/**
+ * Unlocks the mutex and schedules the next coroutine that was waiting for this
+ * lock to be run.
+ */
+CoroutineFn<void> qemu_co_mutex_unlock(CoMutex *mutex);
+
+/**
+ * Read locks the CoRwlock. If the lock cannot be taken immediately because
+ * of a parallel writer, control is transferred to the caller of the current
+ * coroutine.
+ */
+CoroutineFn<void> qemu_co_rwlock_rdlock(CoRwlock *lock);
+
+/**
+ * Write Locks the CoRwlock from a reader. This is a bit more efficient than
+ * @qemu_co_rwlock_unlock followed by a separate @qemu_co_rwlock_wrlock.
+ * Note that if the lock cannot be upgraded immediately, control is transferred
+ * to the caller of the current coroutine; another writer might run while
+ * @qemu_co_rwlock_upgrade blocks.
+ */
+CoroutineFn<void> qemu_co_rwlock_upgrade(CoRwlock *lock);
+
+/**
+ * Downgrades a write-side critic section to a reader. Downgrading with
+ * @qemu_co_rwlock_downgrade never blocks, unlike @qemu_co_rwlock_unlock
+ * followed by @qemu_co_rwlock_rdlock. This makes it more efficient, but
+ * may also sometimes be necessary for correctness.
+ */
+CoroutineFn<void> qemu_co_rwlock_downgrade(CoRwlock *lock);
+
+/**
+ * Write Locks the mutex. If the lock cannot be taken immediately because
+ * of a parallel reader, control is transferred to the caller of the current
+ * coroutine.
+ */
+CoroutineFn<void> qemu_co_rwlock_wrlock(CoRwlock *lock);
+
+/**
+ * Unlocks the read/write lock and schedules the next coroutine that was
+ * waiting for this lock to be run.
+ */
+CoroutineFn<void> qemu_co_rwlock_unlock(CoRwlock *lock);
+
+/**
+ * Yield the coroutine for a given duration. Initializes @w so that,
+ * during this yield, it can be passed to qemu_co_sleep_wake() to
+ * terminate the sleep.
+ */
+CoroutineFn<void> qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
+ QEMUClockType type, int64_t ns);
+
+/**
+ * Yield the coroutine until the next call to qemu_co_sleep_wake.
+ */
+CoroutineFn<void> qemu_co_sleep(QemuCoSleep *w);
+
+static inline CoroutineFn<void> qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
+{
+ QemuCoSleep w = { 0 };
+ co_await qemu_co_sleep_ns_wakeable(&w, type, ns);
+}
+
+/**
+ * Yield until a file descriptor becomes readable
+ *
+ * Note that this function clobbers the handlers for the file descriptor.
+ */
+CoroutineFn<void> yield_until_fd_readable(int fd);
+
+
#include "qemu/co-lockable.h"
+#endif
#endif /* QEMU_COROUTINE_H */
diff --git a/include/qemu/coroutine_int.h b/include/qemu/coroutine_int.h
index 1da148552f..67d6586997 100644
--- a/include/qemu/coroutine_int.h
+++ b/include/qemu/coroutine_int.h
@@ -28,6 +28,8 @@
#include "qemu/queue.h"
#include "qemu/coroutine.h"
+G_BEGIN_DECLS
+
#ifdef CONFIG_SAFESTACK
/* Pointer to the unsafe stack, defined by the compiler */
extern __thread void *__safestack_unsafe_stack_ptr;
@@ -41,6 +43,10 @@ typedef enum {
COROUTINE_ENTER = 3,
} CoroutineAction;
+#ifndef __cplusplus
+typedef struct IncompleteType *CoroutineEntry;
+#endif
+
struct Coroutine {
CoroutineEntry *entry;
void *entry_arg;
@@ -74,4 +80,6 @@ void qemu_coroutine_delete(Coroutine *co);
CoroutineAction qemu_coroutine_switch(Coroutine *from, Coroutine *to,
CoroutineAction action);
+G_END_DECLS
+
#endif
diff --git a/util/coroutine-stackless.cc b/util/coroutine-stackless.cc
new file mode 100644
index 0000000000..ce2f284663
--- /dev/null
+++ b/util/coroutine-stackless.cc
@@ -0,0 +1,145 @@
+/*
+ * stackless coroutine initialization code
+ *
+ * Copyright (C) 2022 Paolo BOnzini <pbonzini@redhat.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser Gener Public
+ * License as published by the Free Software Foundation; either
+ * version 2.0 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser Gener Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser Gener Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "trace.h"
+#include "qemu/coroutine_int.h"
+
+// CoroutineImpl is the entry point into a coroutine. It stores the
+// coroutine_handle that last called qemu_coroutine_yield(), and
+// Coroutine::resume() then resumes from the last yield point.
+//
+// Together with a thread-loc variable "current", the "caller"
+// member establishes a stack of active coroutines, so that
+// qemu_coroutine_yield() knows which coroutine has yielded.
+//
+// Its promise type, EntryPromise, is pretty much bog-standard.
+// It always suspends on entry, so that the coroutine is only
+// entered by the first call to qemu_coroutine_enter(); and it
+// always suspends on exit too, because we want to clean up the
+// coroutine explicitly in BaseCoroutine's destructor.
+
+struct EntryPromise;
+struct CoroutineImpl: BaseCoroutine<EntryPromise> {
+ std::coroutine_handle<> top;
+ explicit CoroutineImpl(promise_type &promise) :
+ BaseCoroutine{promise}, top{*this} {}
+
+ CoroutineAction resume();
+};
+
+struct EntryPromise
+{
+ CoroutineImpl get_return_object() noexcept { return CoroutineImpl{*this}; }
+ void unhandled_exception() { std::terminate(); }
+ auto initial_suspend() const noexcept { return std::suspend_always{}; }
+ auto final_suspend() const noexcept { return std::suspend_always{}; }
+ void return_void() const noexcept {}
+};
+
+typedef struct {
+ Coroutine base;
+ CoroutineImpl *impl;
+} CoroutineStackless;
+
+static __thread CoroutineStackless leader;
+static __thread Coroutine *current;
+
+// ---------------------------
+
+// Change the type from CoroutineFn<void> to Coroutine,
+// so that it does not start until qemu_coroutine_enter()
+CoroutineImpl coroutine_trampoline(Coroutine *co)
+{
+ co_await co->entry(co->entry_arg);
+}
+
+CoroutineAction CoroutineImpl::resume() {
+ std::coroutine_handle<> old_top = top;
+ top = nullptr;
+ old_top.resume();
+ return top ? COROUTINE_YIELD : COROUTINE_TERMINATE;
+}
+
+void Yield::await_suspend(std::coroutine_handle<> parent) const noexcept {
+ CoroutineStackless *curr = DO_UPCAST(CoroutineStackless, base, current);
+ curr->impl->top = parent;
+}
+
+// ---------------------------
+
+Coroutine *qemu_coroutine_new(void)
+{
+ CoroutineStackless *co;
+
+ co = g_new0(CoroutineStackless, 1);
+ return &co->base;
+}
+
+void qemu_coroutine_delete(Coroutine *co_)
+{
+ CoroutineStackless *co = DO_UPCAST(CoroutineStackless, base, co_);
+
+ g_free(co);
+}
+
+// RAII wrapper to set and restore the current coroutine
+struct WithCurrent {
+ Coroutine &_co;
+ WithCurrent(Coroutine &co): _co(co) {
+ current = &_co;
+ }
+ ~WithCurrent() {
+ current = _co.caller;
+ _co.caller = NULL;
+ }
+};
+
+CoroutineAction
+qemu_coroutine_switch(Coroutine *from, Coroutine *to_,
+ CoroutineAction action)
+{
+ CoroutineStackless *to = DO_UPCAST(CoroutineStackless, base, to_);
+
+ assert(action == COROUTINE_ENTER);
+ assert(to->base.caller);
+ auto w = WithCurrent{*to_};
+ if (!to->impl) {
+ to->impl = new CoroutineImpl(coroutine_trampoline(to_));
+ }
+ if (to->impl->resume() == COROUTINE_YIELD) {
+ return COROUTINE_YIELD;
+ }
+ delete to->impl;
+ to->impl = NULL;
+ return COROUTINE_TERMINATE;
+}
+
+Coroutine *qemu_coroutine_self(void)
+{
+ if (!current) {
+ current = &leader.base;
+ }
+ return current;
+}
+
+bool qemu_in_coroutine(void)
+{
+ return current && current->caller;
+}
diff --git a/util/meson.build b/util/meson.build
index 30949cd481..11ec6534b9 100644
--- a/util/meson.build
+++ b/util/meson.build
@@ -68,7 +68,7 @@ if have_block
util_ss.add(files('base64.c'))
util_ss.add(files('buffer.c'))
util_ss.add(files('bufferiszero.c'))
- util_ss.add(files('coroutine-@0@.c'.format(config_host['CONFIG_COROUTINE_BACKEND'])))
+ util_ss.add(files('coroutine-stackless.cc'))
util_ss.add(files('hbitmap.c'))
util_ss.add(files('hexdump.c'))
util_ss.add(files('iova-tree.c'))
@@ -76,12 +76,12 @@ if have_block
util_ss.add(files('lockcnt.c'))
util_ss.add(files('main-loop.c'))
util_ss.add(files('nvdimm-utils.c'))
- util_ss.add(files('qemu-coroutine.c', 'qemu-coroutine-lock.c')) # 'qemu-coroutine-io.c'
+ util_ss.add(files('qemu-coroutine.cc', 'qemu-coroutine-lock.cc')) # 'qemu-coroutine-io.c'
# util_ss.add(when: 'CONFIG_LINUX', if_true: [
# files('vhost-user-server.c'), vhost_user
# ])
util_ss.add(files('block-helpers.c'))
- util_ss.add(files('qemu-coroutine-sleep.c'))
+ util_ss.add(files('qemu-coroutine-sleep.cc'))
# util_ss.add(files('qemu-co-shared-resource.c'))
util_ss.add(files('thread-pool.c', 'qemu-timer.c'))
util_ss.add(files('readline.c'))
diff --git a/util/qemu-coroutine-lock.c b/util/qemu-coroutine-lock.cc
similarity index 87%
rename from util/qemu-coroutine-lock.c
rename to util/qemu-coroutine-lock.cc
index d6c0565ba5..86c51604b6 100644
--- a/util/qemu-coroutine-lock.c
+++ b/util/qemu-coroutine-lock.cc
@@ -120,6 +120,7 @@ bool qemu_co_queue_empty(CoQueue *queue)
{
return QSIMPLEQ_FIRST(&queue->entries) == NULL;
}
+#endif
/* The wait records are handled with a multiple-producer, single-consumer
* lock-free queue. There cannot be two concurrent pop_waiter() calls
@@ -187,7 +188,7 @@ void qemu_co_mutex_init(CoMutex *mutex)
memset(mutex, 0, sizeof(*mutex));
}
-static void coroutine_fn qemu_co_mutex_wake(CoMutex *mutex, Coroutine *co)
+static void qemu_co_mutex_wake(CoMutex *mutex, Coroutine *co)
{
/* Read co before co->ctx; pairs with smp_wmb() in
* qemu_coroutine_enter().
@@ -197,7 +198,7 @@ static void coroutine_fn qemu_co_mutex_wake(CoMutex *mutex, Coroutine *co)
aio_co_wake(co);
}
-static void coroutine_fn qemu_co_mutex_lock_slowpath(AioContext *ctx,
+static CoroutineFn<void> qemu_co_mutex_lock_slowpath(AioContext *ctx,
CoMutex *mutex)
{
Coroutine *self = qemu_coroutine_self();
@@ -223,17 +224,17 @@ static void coroutine_fn qemu_co_mutex_lock_slowpath(AioContext *ctx,
/* We got the lock ourselves! */
assert(to_wake == &w);
mutex->ctx = ctx;
- return;
+ co_return;
}
qemu_co_mutex_wake(mutex, co);
}
- qemu_coroutine_yield();
+ co_await qemu_coroutine_yield();
trace_qemu_co_mutex_lock_return(mutex, self);
}
-void coroutine_fn qemu_co_mutex_lock(CoMutex *mutex)
+CoroutineFn<void> qemu_co_mutex_lock(CoMutex *mutex)
{
AioContext *ctx = qemu_get_current_aio_context();
Coroutine *self = qemu_coroutine_self();
@@ -267,13 +268,13 @@ retry_fast_path:
trace_qemu_co_mutex_lock_uncontended(mutex, self);
mutex->ctx = ctx;
} else {
- qemu_co_mutex_lock_slowpath(ctx, mutex);
+ co_await qemu_co_mutex_lock_slowpath(ctx, mutex);
}
mutex->holder = self;
self->locks_held++;
}
-void coroutine_fn qemu_co_mutex_unlock(CoMutex *mutex)
+CoroutineFn<void> qemu_co_mutex_unlock(CoMutex *mutex)
{
Coroutine *self = qemu_coroutine_self();
@@ -288,7 +289,7 @@ void coroutine_fn qemu_co_mutex_unlock(CoMutex *mutex)
self->locks_held--;
if (qatomic_fetch_dec(&mutex->locked) == 1) {
/* No waiting qemu_co_mutex_lock(). Pfew, that was easy! */
- return;
+ co_return;
}
for (;;) {
@@ -342,7 +343,7 @@ void qemu_co_rwlock_init(CoRwlock *lock)
}
/* Releases the intern CoMutex. */
-static void qemu_co_rwlock_maybe_wake_one(CoRwlock *lock)
+static CoroutineFn<void> qemu_co_rwlock_maybe_wake_one(CoRwlock *lock)
{
CoRwTicket *tkt = QSIMPLEQ_FIRST(&lock->tickets);
Coroutine *co = NULL;
@@ -368,46 +369,46 @@ static void qemu_co_rwlock_maybe_wake_one(CoRwlock *lock)
if (co) {
QSIMPLEQ_REMOVE_HEAD(&lock->tickets, next);
- qemu_co_mutex_unlock(&lock->mutex);
+ co_await qemu_co_mutex_unlock(&lock->mutex);
aio_co_wake(co);
} else {
- qemu_co_mutex_unlock(&lock->mutex);
+ co_await qemu_co_mutex_unlock(&lock->mutex);
}
}
-void qemu_co_rwlock_rdlock(CoRwlock *lock)
+CoroutineFn<void> qemu_co_rwlock_rdlock(CoRwlock *lock)
{
Coroutine *self = qemu_coroutine_self();
- qemu_co_mutex_lock(&lock->mutex);
+ co_await qemu_co_mutex_lock(&lock->mutex);
/* For fairness, wait if a writer is in line. */
if (lock->owners == 0 || (lock->owners > 0 && QSIMPLEQ_EMPTY(&lock->tickets))) {
lock->owners++;
- qemu_co_mutex_unlock(&lock->mutex);
+ co_await qemu_co_mutex_unlock(&lock->mutex);
} else {
CoRwTicket my_ticket = { true, self };
QSIMPLEQ_INSERT_TAIL(&lock->tickets, &my_ticket, next);
- qemu_co_mutex_unlock(&lock->mutex);
- qemu_coroutine_yield();
+ co_await qemu_co_mutex_unlock(&lock->mutex);
+ co_await qemu_coroutine_yield();
assert(lock->owners >= 1);
/* Possibly wake another reader, which will wake the next in line. */
- qemu_co_mutex_lock(&lock->mutex);
- qemu_co_rwlock_maybe_wake_one(lock);
+ co_await qemu_co_mutex_lock(&lock->mutex);
+ co_await qemu_co_rwlock_maybe_wake_one(lock);
}
self->locks_held++;
}
-void qemu_co_rwlock_unlock(CoRwlock *lock)
+CoroutineFn<void> qemu_co_rwlock_unlock(CoRwlock *lock)
{
Coroutine *self = qemu_coroutine_self();
assert(qemu_in_coroutine());
self->locks_held--;
- qemu_co_mutex_lock(&lock->mutex);
+ co_await qemu_co_mutex_lock(&lock->mutex);
if (lock->owners > 0) {
lock->owners--;
} else {
@@ -415,55 +416,54 @@ void qemu_co_rwlock_unlock(CoRwlock *lock)
lock->owners = 0;
}
- qemu_co_rwlock_maybe_wake_one(lock);
+ co_await qemu_co_rwlock_maybe_wake_one(lock);
}
-void qemu_co_rwlock_downgrade(CoRwlock *lock)
+CoroutineFn<void> qemu_co_rwlock_downgrade(CoRwlock *lock)
{
- qemu_co_mutex_lock(&lock->mutex);
+ co_await qemu_co_mutex_lock(&lock->mutex);
assert(lock->owners == -1);
lock->owners = 1;
/* Possibly wake another reader, which will wake the next in line. */
- qemu_co_rwlock_maybe_wake_one(lock);
+ co_await qemu_co_rwlock_maybe_wake_one(lock);
}
-void qemu_co_rwlock_wrlock(CoRwlock *lock)
+CoroutineFn<void> qemu_co_rwlock_wrlock(CoRwlock *lock)
{
Coroutine *self = qemu_coroutine_self();
- qemu_co_mutex_lock(&lock->mutex);
+ co_await qemu_co_mutex_lock(&lock->mutex);
if (lock->owners == 0) {
lock->owners = -1;
- qemu_co_mutex_unlock(&lock->mutex);
+ co_await qemu_co_mutex_unlock(&lock->mutex);
} else {
CoRwTicket my_ticket = { false, self };
QSIMPLEQ_INSERT_TAIL(&lock->tickets, &my_ticket, next);
- qemu_co_mutex_unlock(&lock->mutex);
- qemu_coroutine_yield();
+ co_await qemu_co_mutex_unlock(&lock->mutex);
+ co_await qemu_coroutine_yield();
assert(lock->owners == -1);
}
self->locks_held++;
}
-void qemu_co_rwlock_upgrade(CoRwlock *lock)
+CoroutineFn<void> qemu_co_rwlock_upgrade(CoRwlock *lock)
{
- qemu_co_mutex_lock(&lock->mutex);
+ co_await qemu_co_mutex_lock(&lock->mutex);
assert(lock->owners > 0);
/* For fairness, wait if a writer is in line. */
if (lock->owners == 1 && QSIMPLEQ_EMPTY(&lock->tickets)) {
lock->owners = -1;
- qemu_co_mutex_unlock(&lock->mutex);
+ co_await qemu_co_mutex_unlock(&lock->mutex);
} else {
CoRwTicket my_ticket = { false, qemu_coroutine_self() };
lock->owners--;
QSIMPLEQ_INSERT_TAIL(&lock->tickets, &my_ticket, next);
- qemu_co_rwlock_maybe_wake_one(lock);
- qemu_coroutine_yield();
+ co_await qemu_co_rwlock_maybe_wake_one(lock);
+ co_await qemu_coroutine_yield();
assert(lock->owners == -1);
}
}
-#endif
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.cc
similarity index 89%
rename from util/qemu-coroutine-sleep.c
rename to util/qemu-coroutine-sleep.cc
index b5bfb4ad18..8bb8d91109 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.cc
@@ -17,7 +17,6 @@
#include "qemu/timer.h"
#include "block/aio.h"
-#if 0
static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";
void qemu_co_sleep_wake(QemuCoSleep *w)
@@ -38,11 +37,11 @@ void qemu_co_sleep_wake(QemuCoSleep *w)
static void co_sleep_cb(void *opaque)
{
- QemuCoSleep *w = opaque;
+ QemuCoSleep *w = (QemuCoSleep *)opaque;
qemu_co_sleep_wake(w);
}
-void coroutine_fn qemu_co_sleep(QemuCoSleep *w)
+CoroutineFn<void> qemu_co_sleep(QemuCoSleep *w)
{
Coroutine *co = qemu_coroutine_self();
@@ -56,13 +55,13 @@ void coroutine_fn qemu_co_sleep(QemuCoSleep *w)
}
w->to_wake = co;
- qemu_coroutine_yield();
+ co_await qemu_coroutine_yield();
/* w->to_wake is cleared before resuming this coroutine. */
assert(w->to_wake == NULL);
}
-void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
+CoroutineFn<void> qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
QEMUClockType type, int64_t ns)
{
AioContext *ctx = qemu_get_current_aio_context();
@@ -76,7 +75,6 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
* must happen after qemu_co_sleep yields and there is no race
* between timer_mod and qemu_co_sleep.
*/
- qemu_co_sleep(w);
+ co_await qemu_co_sleep(w);
timer_del(&ts);
}
-#endif
diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.cc
similarity index 94%
rename from util/qemu-coroutine.c
rename to util/qemu-coroutine.cc
index 9f2bd96fa0..0ae2a4090f 100644
--- a/util/qemu-coroutine.c
+++ b/util/qemu-coroutine.cc
@@ -179,22 +179,6 @@ void qemu_coroutine_enter_if_inactive(Coroutine *co)
}
}
-void coroutine_fn qemu_coroutine_yield(void)
-{
- Coroutine *self = qemu_coroutine_self();
- Coroutine *to = self->caller;
-
- trace_qemu_coroutine_yield(self, to);
-
- if (!to) {
- fprintf(stderr, "Co-routine is yielding to no one\n");
- abort();
- }
-
- self->caller = NULL;
- qemu_coroutine_switch(self, to, COROUTINE_YIELD);
-}
-
bool qemu_coroutine_entered(Coroutine *co)
{
return co->caller;
--
2.35.1
^ permalink raw reply related [flat|nested] 53+ messages in thread* Re: [PATCH experiment 14/16] util: introduce C++ stackless coroutine backend
2022-03-14 9:32 ` [PATCH experiment 14/16] util: introduce C++ stackless coroutine backend Paolo Bonzini
@ 2022-03-14 14:37 ` Stefan Hajnoczi
2022-03-14 19:36 ` Paolo Bonzini
0 siblings, 1 reply; 53+ messages in thread
From: Stefan Hajnoczi @ 2022-03-14 14:37 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kwolf, hreitz, berrange, qemu-devel, qemu-block
[-- Attachment #1: Type: text/plain, Size: 1314 bytes --]
On Mon, Mar 14, 2022 at 10:32:01AM +0100, Paolo Bonzini wrote:
> +// ------------------------
> +
> +// CoroutineFn does not even need anything more than what
> +// BaseCoroutine provides, so it's just a type alias. The magic
> +// is all in ValuePromise<T>.
> +//
> +// Suspended CoroutineFns are chained between themselves. Whenever a
> +// coroutine is suspended, all those that have done a co_await are
> +// also suspended, and whenever a coroutine finishes, it has to
> +// check if its parent can now be resumed.
> +//
> +// The two auxiliary classes Awaiter and ResumeAndFinish take
> +// care of the two sides of this. Awaiter's await_suspend() stores
> +// the parent coroutine into ValuePromise; ResumeAndFinish's runs
> +// after a coroutine returns, and resumes the parent coroutine.
> +
> +template<typename T> struct ValuePromise;
> +template<typename T>
> +using CoroutineFn = BaseCoroutine<ValuePromise<T>>;
> +
> +typedef CoroutineFn<void> CoroutineFunc(void *);
CoroutineFunc looks like a coroutine entry point function. If that's
correct then I suggest naming it CoroutineEntryFunc to avoid confusion
between CoroutineFn vs CoroutineFunc (their names are too similar).
Also, where is CoroutineFunc used?
> +// The actu promises, respectively for non-void and void types.
s/actu/actual/?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH experiment 14/16] util: introduce C++ stackless coroutine backend
2022-03-14 14:37 ` Stefan Hajnoczi
@ 2022-03-14 19:36 ` Paolo Bonzini
0 siblings, 0 replies; 53+ messages in thread
From: Paolo Bonzini @ 2022-03-14 19:36 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Wolf, Kevin, Hanna Reitz, P. Berrange, Daniel, qemu-devel,
open list:Block layer core
[-- Attachment #1: Type: text/plain, Size: 1521 bytes --]
Il lun 14 mar 2022, 15:37 Stefan Hajnoczi <stefanha@redhat.com> ha scritto:
> On Mon, Mar 14, 2022 at 10:32:01AM +0100, Paolo Bonzini wrote:
> > +// ------------------------
> > +
> > +// CoroutineFn does not even need anything more than what
> > +// BaseCoroutine provides, so it's just a type alias. The magic
> > +// is all in ValuePromise<T>.
> > +//
> > +// Suspended CoroutineFns are chained between themselves. Whenever a
> > +// coroutine is suspended, all those that have done a co_await are
> > +// also suspended, and whenever a coroutine finishes, it has to
> > +// check if its parent can now be resumed.
> > +//
> > +// The two auxiliary classes Awaiter and ResumeAndFinish take
> > +// care of the two sides of this. Awaiter's await_suspend() stores
> > +// the parent coroutine into ValuePromise; ResumeAndFinish's runs
> > +// after a coroutine returns, and resumes the parent coroutine.
> > +
> > +template<typename T> struct ValuePromise;
> > +template<typename T>
> > +using CoroutineFn = BaseCoroutine<ValuePromise<T>>;
> > +
> > +typedef CoroutineFn<void> CoroutineFunc(void *);
>
> CoroutineFunc looks like a coroutine entry point function. If that's
> correct then I suggest naming it CoroutineEntryFunc to avoid confusion
> between CoroutineFn vs CoroutineFunc (their names are too similar).
>
Nevermind, it's a useless dup of CoroutineEntry.
> Also, where is CoroutineFunc used?
>
> > +// The actu promises, respectively for non-void and void types.
>
> s/actu/actual/?
>
Yes.
Paolo
>
[-- Attachment #2: Type: text/html, Size: 2509 bytes --]
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH experiment 15/16] port QemuCoLockable to C++ coroutines
2022-03-14 9:31 [PATCH experiment 00/16] C++20 coroutine backend Paolo Bonzini
` (13 preceding siblings ...)
2022-03-14 9:32 ` [PATCH experiment 14/16] util: introduce C++ stackless coroutine backend Paolo Bonzini
@ 2022-03-14 9:32 ` Paolo Bonzini
2022-03-14 9:32 ` [PATCH experiment 16/16] port test-coroutine " Paolo Bonzini
` (3 subsequent siblings)
18 siblings, 0 replies; 53+ messages in thread
From: Paolo Bonzini @ 2022-03-14 9:32 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, hreitz, berrange, qemu-block, stefanha
Convert "T coroutine_fn" annotations to the new type CoroutineFn<T>,
and add co_await as needed.
_Generic is replaced by an overloaded constructor. C++ also does
not like & on a temporary, so that is replaced by a function
qemu_make_co_lockable_nonnull that hides it from the compiler.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/co-lockable.h | 111 ++++++++++++++++++-------------------
1 file changed, 53 insertions(+), 58 deletions(-)
diff --git a/include/qemu/co-lockable.h b/include/qemu/co-lockable.h
index 09f4620017..13e3cc7a69 100644
--- a/include/qemu/co-lockable.h
+++ b/include/qemu/co-lockable.h
@@ -16,83 +16,78 @@
#include "qemu/coroutine.h"
#include "qemu/thread.h"
-typedef void coroutine_fn QemuCoLockUnlockFunc(void *);
+typedef CoroutineFn<void> QemuCoLockUnlockFunc(void *);
+
+extern CoroutineFn<void> qemu_mutex_co_lock(QemuMutex *m);
+extern CoroutineFn<void> qemu_mutex_co_unlock(QemuMutex *m);
struct QemuCoLockable {
void *object;
QemuCoLockUnlockFunc *lock;
QemuCoLockUnlockFunc *unlock;
+
+ QemuCoLockable() :
+ object{NULL},
+ lock{(QemuCoLockUnlockFunc *) NULL},
+ unlock{(QemuCoLockUnlockFunc *) NULL} {}
+ QemuCoLockable(QemuMutex *x) :
+ object{x},
+ lock{(QemuCoLockUnlockFunc *) qemu_mutex_co_lock},
+ unlock{(QemuCoLockUnlockFunc *) qemu_mutex_co_unlock} {}
+ QemuCoLockable(CoMutex *x) :
+ object{x},
+ lock{(QemuCoLockUnlockFunc *) qemu_co_mutex_lock},
+ unlock{(QemuCoLockUnlockFunc *) qemu_co_mutex_unlock} {}
};
-static inline __attribute__((__always_inline__)) QemuCoLockable *
-qemu_make_co_lockable(void *x, QemuCoLockable *lockable)
+template<typename T>
+static inline QemuCoLockable qcml_obj_nonnull_(T *x)
{
- /*
- * We cannot test this in a macro, otherwise we get compiler
- * warnings like "the address of 'm' will always evaluate as 'true'".
- */
- return x ? lockable : NULL;
+ return QemuCoLockable{x};
}
-static inline __attribute__((__always_inline__)) QemuCoLockable *
-qemu_null_co_lockable(void *x)
+static inline QemuCoLockable const *qemu_make_co_lockable_nonnull(QemuCoLockable const &x)
{
+ return &x;
+}
+
+template<typename T>
+static inline QemuCoLockable qcml_obj_(T *x)
+{
+ return QemuCoLockable{x};
+}
+extern void build_not_reached();
+
+template<> inline
+QemuCoLockable qcml_obj_(void *x)
+{
+#ifdef __OPTIMIZE__
if (x != NULL) {
- qemu_build_not_reached();
+ build_not_reached();
}
- return NULL;
+#endif
+ return QemuCoLockable{};
}
-/*
- * In C, compound literals have the lifetime of an automatic variable.
- * In C++ it would be different, but then C++ wouldn't need QemuCoLockable
- * either...
- */
-#define QMCL_OBJ_(x, name) (&(QemuCoLockable) { \
- .object = (x), \
- .lock = (QemuCoLockUnlockFunc *) qemu_ ## name ## _lock, \
- .unlock = (QemuCoLockUnlockFunc *) qemu_ ## name ## _unlock \
- })
-
-/**
- * QEMU_MAKE_CO_LOCKABLE - Make a polymorphic QemuCoLockable
- *
- * @x: a lock object (currently one of QemuMutex, CoMutex).
- *
- * Returns a QemuCoLockable object that can be passed around
- * to a function that can operate with locks of any kind, or
- * NULL if @x is %NULL.
- *
- * Note the speci case for void *, so that we may pass "NULL".
- */
-#define QEMU_MAKE_CO_LOCKABLE(x) \
- _Generic((x), QemuCoLockable *: (x), \
- void *: qemu_null_co_lockable(x), \
- QemuMutex *: qemu_make_co_lockable(x, QMCL_OBJ_(x, mutex)), \
- CoMutex *: qemu_make_co_lockable(x, QMCL_OBJ_(x, co_mutex))) \
-
-/**
- * QEMU_MAKE_CO_LOCKABLE_NONNULL - Make a polymorphic QemuCoLockable
- *
- * @x: a lock object (currently one of QemuMutex, QemuRecMutex,
- * CoMutex, QemuSpin).
- *
- * Returns a QemuCoLockable object that can be passed around
- * to a function that can operate with locks of any kind.
- */
-#define QEMU_MAKE_CO_LOCKABLE_NONNULL(x) \
- _Generic((x), QemuCoLockable *: (x), \
- QemuMutex *: QMCL_OBJ_(x, mutex), \
- CoMutex *: QMCL_OBJ_(x, co_mutex))
-
-static inline void coroutine_fn qemu_co_lockable_lock(QemuCoLockable *x)
+static inline QemuCoLockable const *qemu_make_co_lockable(QemuCoLockable const &x)
{
- x->lock(x->object);
+ if (x.object)
+ return &x;
+ else
+ return NULL;
}
-static inline void coroutine_fn qemu_co_lockable_unlock(QemuCoLockable *x)
+#define QEMU_MAKE_CO_LOCKABLE_NONNULL(x) qemu_make_co_lockable_nonnull(qcml_obj_nonnull_(x))
+#define QEMU_MAKE_CO_LOCKABLE(x) qemu_make_co_lockable(qcml_obj_(x))
+
+static inline CoroutineFn<void> qemu_co_lockable_lock(const QemuCoLockable *x)
{
- x->unlock(x->object);
+ co_await x->lock(x->object);
+}
+
+static inline CoroutineFn<void> qemu_co_lockable_unlock(const QemuCoLockable *x)
+{
+ co_await x->unlock(x->object);
}
#endif
--
2.35.1
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH experiment 16/16] port test-coroutine to C++ coroutines
2022-03-14 9:31 [PATCH experiment 00/16] C++20 coroutine backend Paolo Bonzini
` (14 preceding siblings ...)
2022-03-14 9:32 ` [PATCH experiment 15/16] port QemuCoLockable to C++ coroutines Paolo Bonzini
@ 2022-03-14 9:32 ` Paolo Bonzini
2022-03-14 14:07 ` [PATCH experiment 00/16] C++20 coroutine backend Stefan Hajnoczi
` (2 subsequent siblings)
18 siblings, 0 replies; 53+ messages in thread
From: Paolo Bonzini @ 2022-03-14 9:32 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, hreitz, berrange, qemu-block, stefanha
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
tests/unit/meson.build | 6 +-
.../{test-coroutine.c => test-coroutine.cc} | 140 +++++++++---------
2 files changed, 76 insertions(+), 70 deletions(-)
rename tests/unit/{test-coroutine.c => test-coroutine.cc} (81%)
diff --git a/tests/unit/meson.build b/tests/unit/meson.build
index 4ca5fdb699..675b5323dd 100644
--- a/tests/unit/meson.build
+++ b/tests/unit/meson.build
@@ -168,7 +168,11 @@ slow_tests = {
}
foreach test_name, extra: tests
- src = [test_name + '.c']
+ if test_name == 'test-coroutine'
+ src = [test_name + '.cc']
+ else
+ src = [test_name + '.c']
+ endif
deps = [qemuutil]
if extra.length() > 0
# use a sourceset to quickly separate sources and deps
diff --git a/tests/unit/test-coroutine.c b/tests/unit/test-coroutine.cc
similarity index 81%
rename from tests/unit/test-coroutine.c
rename to tests/unit/test-coroutine.cc
index c230c2fa6e..8f9ddc50da 100644
--- a/tests/unit/test-coroutine.c
+++ b/tests/unit/test-coroutine.cc
@@ -16,14 +16,14 @@
#include "qemu/coroutine_int.h"
#include "qemu/lockable.h"
-#if 0
/*
* Check that qemu_in_coroutine() works
*/
-static void coroutine_fn verify_in_coroutine(void *opaque)
+static CoroutineFn<void> verify_in_coroutine(void *opaque)
{
g_assert(qemu_in_coroutine());
+ co_return;
}
static void test_in_coroutine(void)
@@ -40,10 +40,11 @@ static void test_in_coroutine(void)
* Check that qemu_coroutine_self() works
*/
-static void coroutine_fn verify_self(void *opaque)
+static CoroutineFn<void> verify_self(void *opaque)
{
- Coroutine **p_co = opaque;
+ Coroutine **p_co = (Coroutine **)opaque;
g_assert(qemu_coroutine_self() == *p_co);
+ co_return;
}
static void test_self(void)
@@ -58,20 +59,20 @@ static void test_self(void)
* Check that qemu_coroutine_entered() works
*/
-static void coroutine_fn verify_entered_step_2(void *opaque)
+static CoroutineFn<void> verify_entered_step_2(void *opaque)
{
Coroutine *caller = (Coroutine *)opaque;
g_assert(qemu_coroutine_entered(caller));
g_assert(qemu_coroutine_entered(qemu_coroutine_self()));
- qemu_coroutine_yield();
+ co_await qemu_coroutine_yield();
/* Once more to check it still works after yielding */
g_assert(qemu_coroutine_entered(caller));
g_assert(qemu_coroutine_entered(qemu_coroutine_self()));
}
-static void coroutine_fn verify_entered_step_1(void *opaque)
+static CoroutineFn<void> verify_entered_step_1(void *opaque)
{
Coroutine *self = qemu_coroutine_self();
Coroutine *coroutine;
@@ -83,6 +84,7 @@ static void coroutine_fn verify_entered_step_1(void *opaque)
qemu_coroutine_enter(coroutine);
g_assert(!qemu_coroutine_entered(coroutine));
qemu_coroutine_enter(coroutine);
+ co_return;
}
static void test_entered(void)
@@ -104,9 +106,9 @@ typedef struct {
unsigned int max; /* maximum level of nesting */
} NestData;
-static void coroutine_fn nest(void *opaque)
+static CoroutineFn<void> nest(void *opaque)
{
- NestData *nd = opaque;
+ NestData *nd = (NestData *)opaque;
nd->n_enter++;
@@ -118,6 +120,7 @@ static void coroutine_fn nest(void *opaque)
}
nd->n_return++;
+ co_return;
}
static void test_nesting(void)
@@ -141,13 +144,13 @@ static void test_nesting(void)
* Check that yield/enter transfer control correctly
*/
-static void coroutine_fn yield_5_times(void *opaque)
+static CoroutineFn<void> yield_5_times(void *opaque)
{
- bool *done = opaque;
+ bool *done = (bool *)opaque;
int i;
for (i = 0; i < 5; i++) {
- qemu_coroutine_yield();
+ co_await qemu_coroutine_yield();
}
*done = true;
}
@@ -166,15 +169,16 @@ static void test_yield(void)
g_assert_cmpint(i, ==, 5); /* coroutine must yield 5 times */
}
-static void coroutine_fn c2_fn(void *opaque)
+static CoroutineFn<void> c2_fn(void *opaque)
{
- qemu_coroutine_yield();
+ co_await qemu_coroutine_yield();
}
-static void coroutine_fn c1_fn(void *opaque)
+static CoroutineFn<void> c1_fn(void *opaque)
{
- Coroutine *c2 = opaque;
+ Coroutine *c2 = (Coroutine *)opaque;
qemu_coroutine_enter(c2);
+ co_return;
}
static void test_no_dangling_access(void)
@@ -200,34 +204,35 @@ static void test_no_dangling_access(void)
static bool locked;
static int done;
-static void coroutine_fn mutex_fn(void *opaque)
+static CoroutineFn<void> mutex_fn(void *opaque)
{
- CoMutex *m = opaque;
- qemu_co_mutex_lock(m);
+ CoMutex *m = (CoMutex *)opaque;
+ co_await qemu_co_mutex_lock(m);
assert(!locked);
locked = true;
- qemu_coroutine_yield();
+ co_await qemu_coroutine_yield();
locked = false;
- qemu_co_mutex_unlock(m);
+ co_await qemu_co_mutex_unlock(m);
done++;
}
-static void coroutine_fn lockable_fn(void *opaque)
+static CoroutineFn<void> lockable_fn(void *opaque)
{
- QemuCoLockable *x = opaque;
- qemu_co_lockable_lock(x);
+ QemuCoLockable *x = (QemuCoLockable *)opaque;
+ co_await qemu_co_lockable_lock(x);
assert(!locked);
locked = true;
- qemu_coroutine_yield();
+ co_await qemu_coroutine_yield();
locked = false;
- qemu_co_lockable_unlock(x);
+ co_await qemu_co_lockable_unlock(x);
done++;
}
-static void do_test_co_mutex(CoroutineEntry *entry, void *opaque)
+template<typename T>
+static void do_test_co_mutex(CoroutineEntry *entry, const T *opaque)
{
- Coroutine *c1 = qemu_coroutine_create(entry, opaque);
- Coroutine *c2 = qemu_coroutine_create(entry, opaque);
+ Coroutine *c1 = qemu_coroutine_create(entry, (void *)opaque);
+ Coroutine *c2 = qemu_coroutine_create(entry, (void *)opaque);
done = 0;
qemu_coroutine_enter(c1);
@@ -284,23 +289,23 @@ static CoRwlock rwlock;
* | unlock | |
*/
-static void coroutine_fn rwlock_yield_upgrade(void *opaque)
+static CoroutineFn<void> rwlock_yield_upgrade(void *opaque)
{
- qemu_co_rwlock_rdlock(&rwlock);
- qemu_coroutine_yield();
+ co_await qemu_co_rwlock_rdlock(&rwlock);
+ co_await qemu_coroutine_yield();
- qemu_co_rwlock_upgrade(&rwlock);
- qemu_co_rwlock_unlock(&rwlock);
+ co_await qemu_co_rwlock_upgrade(&rwlock);
+ co_await qemu_co_rwlock_unlock(&rwlock);
*(bool *)opaque = true;
}
-static void coroutine_fn rwlock_wrlock_yield(void *opaque)
+static CoroutineFn<void> rwlock_wrlock_yield(void *opaque)
{
- qemu_co_rwlock_wrlock(&rwlock);
- qemu_coroutine_yield();
+ co_await qemu_co_rwlock_wrlock(&rwlock);
+ co_await qemu_coroutine_yield();
- qemu_co_rwlock_unlock(&rwlock);
+ co_await qemu_co_rwlock_unlock(&rwlock);
*(bool *)opaque = true;
}
@@ -326,39 +331,39 @@ static void test_co_rwlock_upgrade(void)
g_assert(c2_done);
}
-static void coroutine_fn rwlock_rdlock_yield(void *opaque)
+static CoroutineFn<void> rwlock_rdlock_yield(void *opaque)
{
- qemu_co_rwlock_rdlock(&rwlock);
- qemu_coroutine_yield();
+ co_await qemu_co_rwlock_rdlock(&rwlock);
+ co_await qemu_coroutine_yield();
- qemu_co_rwlock_unlock(&rwlock);
- qemu_coroutine_yield();
+ co_await qemu_co_rwlock_unlock(&rwlock);
+ co_await qemu_coroutine_yield();
*(bool *)opaque = true;
}
-static void coroutine_fn rwlock_wrlock_downgrade(void *opaque)
+static CoroutineFn<void> rwlock_wrlock_downgrade(void *opaque)
{
- qemu_co_rwlock_wrlock(&rwlock);
+ co_await qemu_co_rwlock_wrlock(&rwlock);
- qemu_co_rwlock_downgrade(&rwlock);
- qemu_co_rwlock_unlock(&rwlock);
+ co_await qemu_co_rwlock_downgrade(&rwlock);
+ co_await qemu_co_rwlock_unlock(&rwlock);
*(bool *)opaque = true;
}
-static void coroutine_fn rwlock_rdlock(void *opaque)
+static CoroutineFn<void> rwlock_rdlock(void *opaque)
{
- qemu_co_rwlock_rdlock(&rwlock);
+ co_await qemu_co_rwlock_rdlock(&rwlock);
- qemu_co_rwlock_unlock(&rwlock);
+ co_await qemu_co_rwlock_unlock(&rwlock);
*(bool *)opaque = true;
}
-static void coroutine_fn rwlock_wrlock(void *opaque)
+static CoroutineFn<void> rwlock_wrlock(void *opaque)
{
- qemu_co_rwlock_wrlock(&rwlock);
+ co_await qemu_co_rwlock_wrlock(&rwlock);
- qemu_co_rwlock_unlock(&rwlock);
+ co_await qemu_co_rwlock_unlock(&rwlock);
*(bool *)opaque = true;
}
@@ -428,11 +433,12 @@ static void test_co_rwlock_downgrade(void)
* Check that creation, enter, and return work
*/
-static void coroutine_fn set_and_exit(void *opaque)
+static CoroutineFn<void> set_and_exit(void *opaque)
{
- bool *done = opaque;
+ bool *done = (bool *)opaque;
*done = true;
+ co_return;
}
static void test_lifecycle(void)
@@ -459,7 +465,7 @@ struct coroutine_position {
int state;
};
static struct coroutine_position records[RECORD_SIZE];
-static unsigned record_pos;
+static int record_pos;
static void record_push(int func, int state)
{
@@ -469,11 +475,11 @@ static void record_push(int func, int state)
cp->state = state;
}
-static void coroutine_fn co_order_test(void *opaque)
+static CoroutineFn<void> co_order_test(void *opaque)
{
record_push(2, 1);
g_assert(qemu_in_coroutine());
- qemu_coroutine_yield();
+ co_await qemu_coroutine_yield();
record_push(2, 2);
g_assert(qemu_in_coroutine());
}
@@ -509,9 +515,10 @@ static void test_order(void)
* Lifecycle benchmark
*/
-static void coroutine_fn empty_coroutine(void *opaque)
+static CoroutineFn<void> empty_coroutine(void *opaque)
{
/* Do nothing */
+ co_return;
}
static void perf_lifecycle(void)
@@ -561,13 +568,13 @@ static void perf_nesting(void)
* Yield benchmark
*/
-static void coroutine_fn yield_loop(void *opaque)
+static CoroutineFn<void> yield_loop(void *opaque)
{
- unsigned int *counter = opaque;
+ unsigned int *counter = (unsigned int *)opaque;
while ((*counter) > 0) {
(*counter)--;
- qemu_coroutine_yield();
+ co_await qemu_coroutine_yield();
}
}
@@ -611,9 +618,9 @@ static void perf_baseline(void)
g_test_message("Function call %u iterations: %f s", maxcycles, duration);
}
-static __attribute__((noinline)) void perf_cost_func(void *opaque)
+static __attribute__((noinline)) CoroutineFn<void> perf_cost_func(void *opaque)
{
- qemu_coroutine_yield();
+ co_await qemu_coroutine_yield();
}
static void perf_cost(void)
@@ -639,13 +646,11 @@ static void perf_cost(void)
duration, ops,
(unsigned long)(1000000000.0 * duration / maxcycles));
}
-#endif
int main(int argc, char **argv)
{
g_test_init(&argc, &argv, NULL);
-#if 0
/* This test assumes there is a freelist and marks freed coroutine memory
* with a sentinel value. If there is no freelist this would legitimately
* crash, so skip it.
@@ -653,9 +658,7 @@ int main(int argc, char **argv)
if (CONFIG_COROUTINE_POOL) {
g_test_add_func("/basic/no-dangling-access", test_no_dangling_access);
}
-#endif
-#if 0
g_test_add_func("/basic/lifecycle", test_lifecycle);
g_test_add_func("/basic/yield", test_yield);
g_test_add_func("/basic/nesting", test_nesting);
@@ -674,6 +677,5 @@ int main(int argc, char **argv)
g_test_add_func("/perf/function-call", perf_baseline);
g_test_add_func("/perf/cost", perf_cost);
}
-#endif
return g_test_run();
}
--
2.35.1
^ permalink raw reply related [flat|nested] 53+ messages in thread* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-14 9:31 [PATCH experiment 00/16] C++20 coroutine backend Paolo Bonzini
` (15 preceding siblings ...)
2022-03-14 9:32 ` [PATCH experiment 16/16] port test-coroutine " Paolo Bonzini
@ 2022-03-14 14:07 ` Stefan Hajnoczi
2022-03-14 16:21 ` Paolo Bonzini
2022-03-14 16:52 ` [PATCH experiment 00/16] C++20 coroutine backend Daniel P. Berrangé
2022-03-15 16:15 ` Daniel P. Berrangé
18 siblings, 1 reply; 53+ messages in thread
From: Stefan Hajnoczi @ 2022-03-14 14:07 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kwolf, hreitz, berrange, qemu-devel, qemu-block
[-- Attachment #1: Type: text/plain, Size: 3293 bytes --]
On Mon, Mar 14, 2022 at 10:31:47AM +0100, Paolo Bonzini wrote:
> However, there are no ramifications to actual coroutine code, except
> for the template syntax "CoroutineFn<return_type>" for the function and
> the mandatory co_await/co_return keywords... both of which are an
> improvement, really: the fact that a single function cannot run either
> inside or outside coroutines is checked by the compiler now, because
> qemu_coroutine_create accepts a function that returns CoroutineFn<void>.
Yeah, these are nice.
> One important difference is that C++ coroutines allocate frames on the
> heap, and that explains why performance is better in /perf/nesting,
> which has to do many large memory allocations for the stack in the other
> two backends (and also a makecontext/swapcontext in the ucontext case).
> C++ coroutines hardly benefit from the coroutine pool; OTOH that also
> means the coroutine pool could be removed if we went this way.
Removing the pool would be nice.
> Overall this was ~twice the amount of work of the C experiment, but
> that's because the two are very different ways to achieve the same goal:
>
> - the design work was substantially smaller in the C experiment, where
> all the backend does is allocate stack frames and do a loop that invokes
> a function pointer. Here the backend has to map between the C++ concepts
> and the QEMU API. In the C case, most of the work was really in the
> manual conversion which I had to do one function at a time.
>
> - the remaining work is also completely different: a source-to-source
> translator (and only build system work in QEMU) for the C experiment;
> making ~100 files compile in C++ for this one (and relatively little
> work as far as coroutines are concerned).
>
> This was compiled with GCC 11 only. Coroutine support was added in
> GCC 10, released in 2020, which IIRC is much newer than the most recent
> release we support.
Using C++ coroutines is likely to be lower risk than maintaining our own
C coroutine source-to-source translator. On the other hand, it exposes
QEMU developers to C++ whether they like it or not so it may not be
popular.
If we can reach a consensus about C++ language usage in QEMU then I'm in
favor of using C++ coroutines. It's probably not realistic to think we
can limit C++ language usage to just coroutines forever. Someone finds
another C++ feature they absolutely need and over time the codebase
becomes C++ - with both its advantages and disadvantages. I'm not sure
what the best solution is but this sounds like a recipe for an identity
crisis with the potential to cause friction for years. From that
perspective I feel it's better to allow C++ and get over it although
what I'd really like is just C++ coroutines and nothing else :P.
To add some more detail, although you can write C in C++, it's not
idiomatic modern C++. The language lends itself to a different style of
programming that some will embrace while others will not. It will be a
bigger impedance mismatch than anything currently in the codebase (e.g.
glib vs non-glib code).
On the other hand, a number of projects have already gone through a
transition like this (gcc, gdb, ...). Maybe we can learn from their
mistakes?
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 53+ messages in thread* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-14 14:07 ` [PATCH experiment 00/16] C++20 coroutine backend Stefan Hajnoczi
@ 2022-03-14 16:21 ` Paolo Bonzini
2022-03-14 19:51 ` Richard Henderson
2022-03-15 14:05 ` Stefan Hajnoczi
0 siblings, 2 replies; 53+ messages in thread
From: Paolo Bonzini @ 2022-03-14 16:21 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: kwolf, hreitz, berrange, qemu-devel, qemu-block
On 3/14/22 15:07, Stefan Hajnoczi wrote:
> If we can reach a consensus about C++ language usage in QEMU then I'm in
> favor of using C++ coroutines. It's probably not realistic to think we
> can limit C++ language usage to just coroutines forever. Someone finds
> another C++ feature they absolutely need and over time the codebase
> becomes C++ - with both its advantages and disadvantages.
>
> [...] although you can write C in C++, it's not idiomatic modern C++.
> The language lends itself to a different style of programming that
> some will embrace while others will not.
Yes, this is an important aspect to discuss. I think coroutines provide
a good blueprint for how QEMU might use C++.
I totally agree that, if we go this way, the genie is out of the bottle
and other uses of C++ will pop up with 100% probability. But the
important thing to note is that our dialect of C is already not standard
C, and that some of our or GLib's "innovations" are actually based on
experience with C++. We can keep on writing "QEMU's C" if we think of
C++ as a supercharged way of writing these quality-of-life improvements
that we already write. In some sense coroutines are an extreme example
of this idea.
In fact, a C API would have to remain unless all source files are
changed to C++, so QEMU would remain mostly a C project with C idioms,
but that doesn't prevent _abstracting_ the use of C++ features (written
in modern, idiomatic C++) behind an API that C programmers have no
problems learning. Again, coroutines are an example of this of keeping
the familiar create/enter/yield API and hiding the "magic" of C++
coroutines (and when they don't, that had better be an improvement).
In the end, C++ is a tool that you can use if it leads to better code.
For example, I don't see much use of C++ for devices for example, and
the storage devices in particular do not even need coroutines because
they use the callback-based interface. But perhaps someone will try to
use templates to replace repeated inclusion (which is common in
hw/display) and others will follow suit. Or perhaps not.
One example that was brought up on IRC is type-safe operations on things
such as hwaddr (i.e. hwaddr+int is allowed but hwaddr-hwaddr gives back
an int64_t and might even check for overflow). These would be opt in
(you get them just by changing a file from .c to .cc), but the actual
C++ code would still look very much like C code that uses hwaddr with no
type checking. All the operator overloading gunk would be in include/.
A different topic is what would happen if all of QEMU could be compiled
as C++, and could inform our selection of C++ idioms even long before we
get there. For example, I'm fine with GLib and our type-safe intrusive
lists, so I don't have much interest in STL containers and I would
prefer _not_ to use STL containers even in .cc files[1]. However,
perhaps QEMU's home-grown lock guard might be replaced by something that
uses C++ destructors instead of g_autoptr, so perhaps we should consider
using std::lock_guard<>, or something like that, instead of
QEMU_LOCK_GUARD. It may be interesting to pass down lock_guards as
arguments to enforce "this lock is taken" invariants.
But really, coroutines would be enough work so my dish would be full for
some time and I wouldn't really have time to look at any of this. :)
Thanks,
Paolo
[1] One big advantage of STL containers for example is that
std::vector<> automatically constructs objects instead of just filling
memory with zero (or leaving it uninitialized). But that doesn't really
matter if we don't use classes with constructors.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-14 16:21 ` Paolo Bonzini
@ 2022-03-14 19:51 ` Richard Henderson
2022-03-15 14:05 ` Stefan Hajnoczi
1 sibling, 0 replies; 53+ messages in thread
From: Richard Henderson @ 2022-03-14 19:51 UTC (permalink / raw)
To: Paolo Bonzini, Stefan Hajnoczi
Cc: kwolf, hreitz, berrange, qemu-devel, qemu-block
On 3/14/22 09:21, Paolo Bonzini wrote:
> But perhaps someone will try to use templates to replace repeated inclusion (which is
> common in hw/display) and others will follow suit.
The code in fpu/ desperately calls out for overloading and templates. At present it is a
tangle of _Generic and multiple inclusion.
r~
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-14 16:21 ` Paolo Bonzini
2022-03-14 19:51 ` Richard Henderson
@ 2022-03-15 14:05 ` Stefan Hajnoczi
2022-03-15 14:24 ` Peter Maydell
2022-03-15 14:50 ` Kevin Wolf
1 sibling, 2 replies; 53+ messages in thread
From: Stefan Hajnoczi @ 2022-03-15 14:05 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kwolf, hreitz, berrange, qemu-devel, qemu-block
[-- Attachment #1: Type: text/plain, Size: 4277 bytes --]
On Mon, Mar 14, 2022 at 05:21:22PM +0100, Paolo Bonzini wrote:
> On 3/14/22 15:07, Stefan Hajnoczi wrote:
> > If we can reach a consensus about C++ language usage in QEMU then I'm in
> > favor of using C++ coroutines. It's probably not realistic to think we
> > can limit C++ language usage to just coroutines forever. Someone finds
> > another C++ feature they absolutely need and over time the codebase
> > becomes C++ - with both its advantages and disadvantages.
> >
> > [...] although you can write C in C++, it's not idiomatic modern C++.
> > The language lends itself to a different style of programming that
> > some will embrace while others will not.
>
> Yes, this is an important aspect to discuss. I think coroutines provide a
> good blueprint for how QEMU might use C++.
>
> I totally agree that, if we go this way, the genie is out of the bottle and
> other uses of C++ will pop up with 100% probability. But the important
> thing to note is that our dialect of C is already not standard C, and that
> some of our or GLib's "innovations" are actually based on experience with
> C++. We can keep on writing "QEMU's C" if we think of C++ as a supercharged
> way of writing these quality-of-life improvements that we already write. In
> some sense coroutines are an extreme example of this idea.
>
> In fact, a C API would have to remain unless all source files are changed to
> C++, so QEMU would remain mostly a C project with C idioms, but that doesn't
> prevent _abstracting_ the use of C++ features (written in modern, idiomatic
> C++) behind an API that C programmers have no problems learning. Again,
> coroutines are an example of this of keeping the familiar create/enter/yield
> API and hiding the "magic" of C++ coroutines (and when they don't, that had
> better be an improvement).
>
> In the end, C++ is a tool that you can use if it leads to better code. For
> example, I don't see much use of C++ for devices for example, and the
> storage devices in particular do not even need coroutines because they use
> the callback-based interface. But perhaps someone will try to use templates
> to replace repeated inclusion (which is common in hw/display) and others
> will follow suit. Or perhaps not.
>
> One example that was brought up on IRC is type-safe operations on things
> such as hwaddr (i.e. hwaddr+int is allowed but hwaddr-hwaddr gives back an
> int64_t and might even check for overflow). These would be opt in (you get
> them just by changing a file from .c to .cc), but the actual C++ code would
> still look very much like C code that uses hwaddr with no type checking.
> All the operator overloading gunk would be in include/.
>
> A different topic is what would happen if all of QEMU could be compiled as
> C++, and could inform our selection of C++ idioms even long before we get
> there. For example, I'm fine with GLib and our type-safe intrusive lists,
> so I don't have much interest in STL containers and I would prefer _not_ to
> use STL containers even in .cc files[1]. However, perhaps QEMU's home-grown
> lock guard might be replaced by something that uses C++ destructors instead
> of g_autoptr, so perhaps we should consider using std::lock_guard<>, or
> something like that, instead of QEMU_LOCK_GUARD. It may be interesting to
> pass down lock_guards as arguments to enforce "this lock is taken"
> invariants.
>
> But really, coroutines would be enough work so my dish would be full for
> some time and I wouldn't really have time to look at any of this. :)
I think it will be necessary to compile QEMU with a C++ compiler quite
soon. It is possible to provide C APIs like in the case of coroutines,
but sometimes C++ features need to be exposed to the caller (like the
lock guards you mentioned). Also, once C++ is available people will
start submitting C++ patches simply because they are more comfortable
with C++ (especially one-time/infrequent contributors).
We need to agree on a policy so that people know how and when they can
use C++.
The policy needs to be simple so it doesn't create hurdles for new
contributors.
Does anyone have experience with C projects that introduced C++ and what
worked/didn't work?
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-15 14:05 ` Stefan Hajnoczi
@ 2022-03-15 14:24 ` Peter Maydell
2022-03-15 17:29 ` Paolo Bonzini
2022-03-15 14:50 ` Kevin Wolf
1 sibling, 1 reply; 53+ messages in thread
From: Peter Maydell @ 2022-03-15 14:24 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: kwolf, berrange, qemu-block, qemu-devel, hreitz, Paolo Bonzini
On Tue, 15 Mar 2022 at 14:09, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> Also, once C++ is available people will
> start submitting C++ patches simply because they are more comfortable
> with C++ (especially one-time/infrequent contributors).
This to my mind is the major argument against using C++
for coroutines...
-- PMM
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-15 14:24 ` Peter Maydell
@ 2022-03-15 17:29 ` Paolo Bonzini
2022-03-16 12:32 ` Stefan Hajnoczi
0 siblings, 1 reply; 53+ messages in thread
From: Paolo Bonzini @ 2022-03-15 17:29 UTC (permalink / raw)
To: Peter Maydell, Stefan Hajnoczi
Cc: kwolf, hreitz, berrange, qemu-devel, qemu-block
On 3/15/22 15:24, Peter Maydell wrote:
> On Tue, 15 Mar 2022 at 14:09, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>> Also, once C++ is available people will
>> start submitting C++ patches simply because they are more comfortable
>> with C++ (especially one-time/infrequent contributors).
>
> This to my mind is the major argument against using C++
> for coroutines...
I agree on the need for a policy, but _what_ C++ are they going to be
contributing that we should be scared of? We're talking about:
* major features contributed by one-time/infrequent participants (which
is already a once-in-a-year thing or so, at least for me)
* ... in an area where there are no examples of using C++ in the tree
(or presumably the maintainer would be comfortable reviewing it)
* ... but yet C++ offer killer features (right now there's only C++
coroutines and fpu/)
* ... and where the one-time contributor has put enough investment in
using these killer C++ features, that telling them to remove the
features would amount to a rewrite.
That does not seem to be a common situation, and not even a problematic
one if it were to happen.
Paolo
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-15 17:29 ` Paolo Bonzini
@ 2022-03-16 12:32 ` Stefan Hajnoczi
2022-03-16 13:06 ` Daniel P. Berrangé
2022-03-17 15:11 ` Paolo Bonzini
0 siblings, 2 replies; 53+ messages in thread
From: Stefan Hajnoczi @ 2022-03-16 12:32 UTC (permalink / raw)
To: Paolo Bonzini
Cc: kwolf, Peter Maydell, berrange, qemu-block, qemu-devel, hreitz
[-- Attachment #1: Type: text/plain, Size: 1727 bytes --]
On Tue, Mar 15, 2022 at 06:29:50PM +0100, Paolo Bonzini wrote:
> On 3/15/22 15:24, Peter Maydell wrote:
> > On Tue, 15 Mar 2022 at 14:09, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > > Also, once C++ is available people will
> > > start submitting C++ patches simply because they are more comfortable
> > > with C++ (especially one-time/infrequent contributors).
> >
> > This to my mind is the major argument against using C++
> > for coroutines...
>
> I agree on the need for a policy, but _what_ C++ are they going to be
> contributing that we should be scared of? We're talking about:
>
> * major features contributed by one-time/infrequent participants (which is
> already a once-in-a-year thing or so, at least for me)
>
> * ... in an area where there are no examples of using C++ in the tree (or
> presumably the maintainer would be comfortable reviewing it)
>
> * ... but yet C++ offer killer features (right now there's only C++
> coroutines and fpu/)
You are assuming people only choose C++ only when it offers features not
available in C. I think they might simply be more comfortable in C++.
In other words, if an existing file is compiled using a C++ compiler or
they are adding a new file, they don't need a reason to use C++, they
can just use it.
You can define rules and a way to enforce a subset of C++, but I think
over time the code will be C++. A policy that is complicated discourages
contributors.
For these reasons I think that if code runs through a C++ compiler we
should just allow C++. Either way, it will take time but that way no one
will feel betrayed when C++ creeps in.
That said, I hope we find an option that doesn't involve C++.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-16 12:32 ` Stefan Hajnoczi
@ 2022-03-16 13:06 ` Daniel P. Berrangé
2022-03-16 16:44 ` Stefan Hajnoczi
2022-03-17 15:11 ` Paolo Bonzini
1 sibling, 1 reply; 53+ messages in thread
From: Daniel P. Berrangé @ 2022-03-16 13:06 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: kwolf, Peter Maydell, qemu-block, qemu-devel, hreitz,
Paolo Bonzini
On Wed, Mar 16, 2022 at 12:32:48PM +0000, Stefan Hajnoczi wrote:
> On Tue, Mar 15, 2022 at 06:29:50PM +0100, Paolo Bonzini wrote:
> > On 3/15/22 15:24, Peter Maydell wrote:
> > > On Tue, 15 Mar 2022 at 14:09, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > > > Also, once C++ is available people will
> > > > start submitting C++ patches simply because they are more comfortable
> > > > with C++ (especially one-time/infrequent contributors).
> > >
> > > This to my mind is the major argument against using C++
> > > for coroutines...
> >
> > I agree on the need for a policy, but _what_ C++ are they going to be
> > contributing that we should be scared of? We're talking about:
> >
> > * major features contributed by one-time/infrequent participants (which is
> > already a once-in-a-year thing or so, at least for me)
> >
> > * ... in an area where there are no examples of using C++ in the tree (or
> > presumably the maintainer would be comfortable reviewing it)
> >
> > * ... but yet C++ offer killer features (right now there's only C++
> > coroutines and fpu/)
>
> You are assuming people only choose C++ only when it offers features not
> available in C. I think they might simply be more comfortable in C++.
>
> In other words, if an existing file is compiled using a C++ compiler or
> they are adding a new file, they don't need a reason to use C++, they
> can just use it.
>
> You can define rules and a way to enforce a subset of C++, but I think
> over time the code will be C++. A policy that is complicated discourages
> contributors.
>
> For these reasons I think that if code runs through a C++ compiler we
> should just allow C++. Either way, it will take time but that way no one
> will feel betrayed when C++ creeps in.
>
> That said, I hope we find an option that doesn't involve C++.
The real show stopper with our current coroutine impl IIUC, is the
undefined behaviour when we yield and restore across different threads.
Is there any relastic hope that we can change QEMU's usage, such that
each coroutine is confined to a single thread, to avoid the undefined
behaviour ?
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-16 13:06 ` Daniel P. Berrangé
@ 2022-03-16 16:44 ` Stefan Hajnoczi
0 siblings, 0 replies; 53+ messages in thread
From: Stefan Hajnoczi @ 2022-03-16 16:44 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: kwolf, Peter Maydell, qemu-block, qemu-devel, hreitz,
Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 3343 bytes --]
On Wed, Mar 16, 2022 at 01:06:02PM +0000, Daniel P. Berrangé wrote:
> On Wed, Mar 16, 2022 at 12:32:48PM +0000, Stefan Hajnoczi wrote:
> > On Tue, Mar 15, 2022 at 06:29:50PM +0100, Paolo Bonzini wrote:
> > > On 3/15/22 15:24, Peter Maydell wrote:
> > > > On Tue, 15 Mar 2022 at 14:09, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > > > > Also, once C++ is available people will
> > > > > start submitting C++ patches simply because they are more comfortable
> > > > > with C++ (especially one-time/infrequent contributors).
> > > >
> > > > This to my mind is the major argument against using C++
> > > > for coroutines...
> > >
> > > I agree on the need for a policy, but _what_ C++ are they going to be
> > > contributing that we should be scared of? We're talking about:
> > >
> > > * major features contributed by one-time/infrequent participants (which is
> > > already a once-in-a-year thing or so, at least for me)
> > >
> > > * ... in an area where there are no examples of using C++ in the tree (or
> > > presumably the maintainer would be comfortable reviewing it)
> > >
> > > * ... but yet C++ offer killer features (right now there's only C++
> > > coroutines and fpu/)
> >
> > You are assuming people only choose C++ only when it offers features not
> > available in C. I think they might simply be more comfortable in C++.
> >
> > In other words, if an existing file is compiled using a C++ compiler or
> > they are adding a new file, they don't need a reason to use C++, they
> > can just use it.
> >
> > You can define rules and a way to enforce a subset of C++, but I think
> > over time the code will be C++. A policy that is complicated discourages
> > contributors.
> >
> > For these reasons I think that if code runs through a C++ compiler we
> > should just allow C++. Either way, it will take time but that way no one
> > will feel betrayed when C++ creeps in.
> >
> > That said, I hope we find an option that doesn't involve C++.
>
> The real show stopper with our current coroutine impl IIUC, is the
> undefined behaviour when we yield and restore across different threads.
>
> Is there any relastic hope that we can change QEMU's usage, such that
> each coroutine is confined to a single thread, to avoid the undefined
> behaviour ?
I don't think so. At the point where a coroutine stops executing in the
vCPU thread the call stack is too deep. The benefit of coroutines would
be lost because it would be necessary to use callback functions (BHs).
Fixes that paper over the undefined behavior have been merged so the
bugs should be kept at bay for a while. In theory we can continue with
stack-based coroutines but we're likely to hit issues again in the
future.
Paolo also prototyped C stackless coroutines. That would require a
source-to-source translation that converts a coroutine_fn into a Duff's
device state machine. That solution doesn't require C++ but it would be
necessary to develop the source-to-source translator and maintain it.
Finally it may be possible to get coroutine in C from clang/gcc. They
have the machinery to do it for C++ so maybe they could also offer it in
C compiler mode. It would be great to have coroutines available in the
toolchain - more reliable and supported than if we roll our own.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-16 12:32 ` Stefan Hajnoczi
2022-03-16 13:06 ` Daniel P. Berrangé
@ 2022-03-17 15:11 ` Paolo Bonzini
2022-03-17 15:53 ` Hanna Reitz
1 sibling, 1 reply; 53+ messages in thread
From: Paolo Bonzini @ 2022-03-17 15:11 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: kwolf, Peter Maydell, berrange, qemu-block, qemu-devel, hreitz
On 3/16/22 13:32, Stefan Hajnoczi wrote:
> You can define rules and a way to enforce a subset of C++, but I think
> over time the code will be C++. A policy that is complicated discourages
> contributors.
>
> For these reasons I think that if code runs through a C++ compiler we
> should just allow C++. Either way, it will take time but that way no one
> will feel betrayed when C++ creeps in.
Fair enough. We even already have some conventions that will make any
C++ that creeps in less weird (for example, mandatory typedef of structs).
I don't think it would be a big deal overall. I actually agree that we
should "just allow C++", what matters more to have style rules that make
QEMU's flavors of C and C++ consistent.
Most files are leaves (i.e. they just expose an interface via
type_init/block_init/etc.), where almost always people will copy from
something that already exists and which will be C. What matters are the
core APIs, and maintainers have way more authority there to say no to
code they're not comfortable reviewing. This way, code within a
subsystem will keep some consistency.
Paolo
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-17 15:11 ` Paolo Bonzini
@ 2022-03-17 15:53 ` Hanna Reitz
2022-03-31 11:37 ` Markus Armbruster
0 siblings, 1 reply; 53+ messages in thread
From: Hanna Reitz @ 2022-03-17 15:53 UTC (permalink / raw)
To: Paolo Bonzini, Stefan Hajnoczi
Cc: kwolf, Peter Maydell, berrange, qemu-devel, qemu-block
On 17.03.22 16:11, Paolo Bonzini wrote:
> On 3/16/22 13:32, Stefan Hajnoczi wrote:
>> You can define rules and a way to enforce a subset of C++, but I think
>> over time the code will be C++. A policy that is complicated discourages
>> contributors.
>>
>> For these reasons I think that if code runs through a C++ compiler we
>> should just allow C++. Either way, it will take time but that way no one
>> will feel betrayed when C++ creeps in.
>
> Fair enough. We even already have some conventions that will make any
> C++ that creeps in less weird (for example, mandatory typedef of
> structs).
>
> I don't think it would be a big deal overall. I actually agree that
> we should "just allow C++", what matters more to have style rules that
> make QEMU's flavors of C and C++ consistent.
I just want to throw in that I personally am absolutely not confident in
reviewing C++ code. As for Rust, you keep mentioning that we don’t have
anyone who would “shepherd us through the learning experience”, but I
find the very same argument applies to C++.
C++ may start out looking like C, but if used ideally, then it is a very
different language, too. I understand the difference is that we can
incrementally convert our C code to C++, but I’m not comfortable
overseeing that process, which I would have to do as a maintainer.
Assuming our overall stance does change to “just allowing C++”, I’d feel
unjust if I were to reject C++isms just based on the fact that I don’t
know C++, so I’d be forced to learn C++. I’m not strictly opposed to
that (though from experience I’m more than hesitant), but forcing
maintainers to learn C++ is something that has a cost associated with it.
My biggest gripe about C++ is that as far as I understand there are many
antipatterns, many of which I think stem from the exact fact that it is
kind of compatible with C, and so you can easily accidentally write
really bad C++ code; but without years of experience, I’m absolutely not
confident that I could recognize them. Now, I might say I’d just
disallow complicated stuff, and keep everything C-like, but from what
I’ve heard, C-like C++ seems to be exactly one case that is considered
bad C++. I’m really under the impression that I’d need years of
experience to discern good from bad C++, and I don’t have that experience.
Hanna
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-17 15:53 ` Hanna Reitz
@ 2022-03-31 11:37 ` Markus Armbruster
0 siblings, 0 replies; 53+ messages in thread
From: Markus Armbruster @ 2022-03-31 11:37 UTC (permalink / raw)
To: Hanna Reitz
Cc: kwolf, Peter Maydell, berrange, qemu-block, qemu-devel,
Stefan Hajnoczi, Paolo Bonzini
Hanna Reitz <hreitz@redhat.com> writes:
> On 17.03.22 16:11, Paolo Bonzini wrote:
>> On 3/16/22 13:32, Stefan Hajnoczi wrote:
>>> You can define rules and a way to enforce a subset of C++, but I think
>>> over time the code will be C++. A policy that is complicated discourages
>>> contributors.
>>>
>>> For these reasons I think that if code runs through a C++ compiler we
>>> should just allow C++. Either way, it will take time but that way no one
>>> will feel betrayed when C++ creeps in.
>>
>> Fair enough. We even already have some conventions that will make
>> any C++ that creeps in less weird (for example, mandatory typedef of
>> structs).
>>
>> I don't think it would be a big deal overall. I actually agree that
>> we should "just allow C++", what matters more to have style rules
>> that make QEMU's flavors of C and C++ consistent.
>
> I just want to throw in that I personally am absolutely not confident
> in reviewing C++ code.
Me neither.
> As for Rust, you keep mentioning that we don’t
> have anyone who would “shepherd us through the learning experience”,
> but I find the very same argument applies to C++.
>
> C++ may start out looking like C, but if used ideally, then it is a
> very different language, too. I understand the difference is that we
> can incrementally convert our C code to C++,
Bad C++ in need of rework, presumably.
> but I’m not comfortable
> overseeing that process, which I would have to do as a maintainer.
That makes two of us.
The plain language version of "I'm not comfortable serving" is of course
"I do not intend to serve".
> Assuming our overall stance does change to “just allowing C++”, I’d
> feel unjust if I were to reject C++isms just based on the fact that I
> don’t know C++, so I’d be forced to learn C++. I’m not strictly
> opposed to that (though from experience I’m more than hesitant), but
> forcing maintainers to learn C++ is something that has a cost
> associated with it.
I'm a lot more interested in learning Rust than in (re-)learning C++.
> My biggest gripe about C++ is that as far as I understand there are
> many antipatterns, many of which I think stem from the exact fact that
> it is kind of compatible with C, and so you can easily accidentally
> write really bad C++ code; but without years of experience, I’m
> absolutely not confident that I could recognize them. Now, I might
> say I’d just disallow complicated stuff, and keep everything C-like,
> but from what I’ve heard, C-like C++ seems to be exactly one case that
> is considered bad C++. I’m really under the impression that I’d need
> years of experience to discern good from bad C++, and I don’t have
> that experience.
Right.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-15 14:05 ` Stefan Hajnoczi
2022-03-15 14:24 ` Peter Maydell
@ 2022-03-15 14:50 ` Kevin Wolf
2022-03-15 15:35 ` Stefan Hajnoczi
` (2 more replies)
1 sibling, 3 replies; 53+ messages in thread
From: Kevin Wolf @ 2022-03-15 14:50 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: Paolo Bonzini, hreitz, berrange, qemu-devel, qemu-block
[-- Attachment #1: Type: text/plain, Size: 5000 bytes --]
Am 15.03.2022 um 15:05 hat Stefan Hajnoczi geschrieben:
> On Mon, Mar 14, 2022 at 05:21:22PM +0100, Paolo Bonzini wrote:
> > On 3/14/22 15:07, Stefan Hajnoczi wrote:
> > > If we can reach a consensus about C++ language usage in QEMU then I'm in
> > > favor of using C++ coroutines. It's probably not realistic to think we
> > > can limit C++ language usage to just coroutines forever. Someone finds
> > > another C++ feature they absolutely need and over time the codebase
> > > becomes C++ - with both its advantages and disadvantages.
> > >
> > > [...] although you can write C in C++, it's not idiomatic modern C++.
> > > The language lends itself to a different style of programming that
> > > some will embrace while others will not.
> >
> > Yes, this is an important aspect to discuss. I think coroutines provide a
> > good blueprint for how QEMU might use C++.
> >
> > I totally agree that, if we go this way, the genie is out of the bottle and
> > other uses of C++ will pop up with 100% probability. But the important
> > thing to note is that our dialect of C is already not standard C, and that
> > some of our or GLib's "innovations" are actually based on experience with
> > C++. We can keep on writing "QEMU's C" if we think of C++ as a supercharged
> > way of writing these quality-of-life improvements that we already write. In
> > some sense coroutines are an extreme example of this idea.
> >
> > In fact, a C API would have to remain unless all source files are changed to
> > C++, so QEMU would remain mostly a C project with C idioms, but that doesn't
> > prevent _abstracting_ the use of C++ features (written in modern, idiomatic
> > C++) behind an API that C programmers have no problems learning. Again,
> > coroutines are an example of this of keeping the familiar create/enter/yield
> > API and hiding the "magic" of C++ coroutines (and when they don't, that had
> > better be an improvement).
> >
> > In the end, C++ is a tool that you can use if it leads to better code. For
> > example, I don't see much use of C++ for devices for example, and the
> > storage devices in particular do not even need coroutines because they use
> > the callback-based interface. But perhaps someone will try to use templates
> > to replace repeated inclusion (which is common in hw/display) and others
> > will follow suit. Or perhaps not.
> >
> > One example that was brought up on IRC is type-safe operations on things
> > such as hwaddr (i.e. hwaddr+int is allowed but hwaddr-hwaddr gives back an
> > int64_t and might even check for overflow). These would be opt in (you get
> > them just by changing a file from .c to .cc), but the actual C++ code would
> > still look very much like C code that uses hwaddr with no type checking.
> > All the operator overloading gunk would be in include/.
> >
> > A different topic is what would happen if all of QEMU could be compiled as
> > C++, and could inform our selection of C++ idioms even long before we get
> > there. For example, I'm fine with GLib and our type-safe intrusive lists,
> > so I don't have much interest in STL containers and I would prefer _not_ to
> > use STL containers even in .cc files[1]. However, perhaps QEMU's home-grown
> > lock guard might be replaced by something that uses C++ destructors instead
> > of g_autoptr, so perhaps we should consider using std::lock_guard<>, or
> > something like that, instead of QEMU_LOCK_GUARD. It may be interesting to
> > pass down lock_guards as arguments to enforce "this lock is taken"
> > invariants.
> >
> > But really, coroutines would be enough work so my dish would be full for
> > some time and I wouldn't really have time to look at any of this. :)
>
> I think it will be necessary to compile QEMU with a C++ compiler quite
> soon. It is possible to provide C APIs like in the case of coroutines,
> but sometimes C++ features need to be exposed to the caller (like the
> lock guards you mentioned).
I'm not sure what the C++ lock guards offer that our current lock guards
don't? Passing down lock guards makes sense to me, but why can't you do
that with QemuLockable? (Hm, or can the C++ version somehow check at
compile time that it's the _right_ lock that is held rather than just
any lock? It didn't look like it at the first sight.)
But I do see the benefit of a compiler checked CoroutineFn<> return type
compared to the coroutine_fn markers we have today. On the other hand...
> Also, once C++ is available people will start submitting C++ patches
> simply because they are more comfortable with C++ (especially
> one-time/infrequent contributors).
...using C++ in coroutine code means that all of the block layer would
suddenly become C++ and would be most affected by this effect. I'm not
sure if that's something I would like to see, at least until there is a
clear tree-wide policy (that preferably limits it to a subset that I
understand).
Kevin
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 53+ messages in thread* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-15 14:50 ` Kevin Wolf
@ 2022-03-15 15:35 ` Stefan Hajnoczi
2022-03-15 15:55 ` Daniel P. Berrangé
2022-03-15 17:23 ` When and how to use C++ (was Re: [PATCH experiment 00/16] C++20 coroutine backend) Paolo Bonzini
2 siblings, 0 replies; 53+ messages in thread
From: Stefan Hajnoczi @ 2022-03-15 15:35 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Paolo Bonzini, hreitz, berrange, qemu-devel, qemu-block
[-- Attachment #1: Type: text/plain, Size: 4893 bytes --]
On Tue, Mar 15, 2022 at 03:50:26PM +0100, Kevin Wolf wrote:
> Am 15.03.2022 um 15:05 hat Stefan Hajnoczi geschrieben:
> > On Mon, Mar 14, 2022 at 05:21:22PM +0100, Paolo Bonzini wrote:
> > > On 3/14/22 15:07, Stefan Hajnoczi wrote:
> > > > If we can reach a consensus about C++ language usage in QEMU then I'm in
> > > > favor of using C++ coroutines. It's probably not realistic to think we
> > > > can limit C++ language usage to just coroutines forever. Someone finds
> > > > another C++ feature they absolutely need and over time the codebase
> > > > becomes C++ - with both its advantages and disadvantages.
> > > >
> > > > [...] although you can write C in C++, it's not idiomatic modern C++.
> > > > The language lends itself to a different style of programming that
> > > > some will embrace while others will not.
> > >
> > > Yes, this is an important aspect to discuss. I think coroutines provide a
> > > good blueprint for how QEMU might use C++.
> > >
> > > I totally agree that, if we go this way, the genie is out of the bottle and
> > > other uses of C++ will pop up with 100% probability. But the important
> > > thing to note is that our dialect of C is already not standard C, and that
> > > some of our or GLib's "innovations" are actually based on experience with
> > > C++. We can keep on writing "QEMU's C" if we think of C++ as a supercharged
> > > way of writing these quality-of-life improvements that we already write. In
> > > some sense coroutines are an extreme example of this idea.
> > >
> > > In fact, a C API would have to remain unless all source files are changed to
> > > C++, so QEMU would remain mostly a C project with C idioms, but that doesn't
> > > prevent _abstracting_ the use of C++ features (written in modern, idiomatic
> > > C++) behind an API that C programmers have no problems learning. Again,
> > > coroutines are an example of this of keeping the familiar create/enter/yield
> > > API and hiding the "magic" of C++ coroutines (and when they don't, that had
> > > better be an improvement).
> > >
> > > In the end, C++ is a tool that you can use if it leads to better code. For
> > > example, I don't see much use of C++ for devices for example, and the
> > > storage devices in particular do not even need coroutines because they use
> > > the callback-based interface. But perhaps someone will try to use templates
> > > to replace repeated inclusion (which is common in hw/display) and others
> > > will follow suit. Or perhaps not.
> > >
> > > One example that was brought up on IRC is type-safe operations on things
> > > such as hwaddr (i.e. hwaddr+int is allowed but hwaddr-hwaddr gives back an
> > > int64_t and might even check for overflow). These would be opt in (you get
> > > them just by changing a file from .c to .cc), but the actual C++ code would
> > > still look very much like C code that uses hwaddr with no type checking.
> > > All the operator overloading gunk would be in include/.
> > >
> > > A different topic is what would happen if all of QEMU could be compiled as
> > > C++, and could inform our selection of C++ idioms even long before we get
> > > there. For example, I'm fine with GLib and our type-safe intrusive lists,
> > > so I don't have much interest in STL containers and I would prefer _not_ to
> > > use STL containers even in .cc files[1]. However, perhaps QEMU's home-grown
> > > lock guard might be replaced by something that uses C++ destructors instead
> > > of g_autoptr, so perhaps we should consider using std::lock_guard<>, or
> > > something like that, instead of QEMU_LOCK_GUARD. It may be interesting to
> > > pass down lock_guards as arguments to enforce "this lock is taken"
> > > invariants.
> > >
> > > But really, coroutines would be enough work so my dish would be full for
> > > some time and I wouldn't really have time to look at any of this. :)
> >
> > I think it will be necessary to compile QEMU with a C++ compiler quite
> > soon. It is possible to provide C APIs like in the case of coroutines,
> > but sometimes C++ features need to be exposed to the caller (like the
> > lock guards you mentioned).
>
> I'm not sure what the C++ lock guards offer that our current lock guards
> don't? Passing down lock guards makes sense to me, but why can't you do
> that with QemuLockable? (Hm, or can the C++ version somehow check at
> compile time that it's the _right_ lock that is held rather than just
> any lock? It didn't look like it at the first sight.)
>
> But I do see the benefit of a compiler checked CoroutineFn<> return type
> compared to the coroutine_fn markers we have today. On the other hand...
Sorry, I made a mistake: the C++ coroutines implementation does not hide
everything behind a C API. Coroutine functions need to be defined in C++
source units.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-15 14:50 ` Kevin Wolf
2022-03-15 15:35 ` Stefan Hajnoczi
@ 2022-03-15 15:55 ` Daniel P. Berrangé
2022-03-15 23:08 ` Paolo Bonzini
2022-03-31 11:52 ` Markus Armbruster
2022-03-15 17:23 ` When and how to use C++ (was Re: [PATCH experiment 00/16] C++20 coroutine backend) Paolo Bonzini
2 siblings, 2 replies; 53+ messages in thread
From: Daniel P. Berrangé @ 2022-03-15 15:55 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Paolo Bonzini, hreitz, qemu-block, qemu-devel, Stefan Hajnoczi
On Tue, Mar 15, 2022 at 03:50:26PM +0100, Kevin Wolf wrote:
> Am 15.03.2022 um 15:05 hat Stefan Hajnoczi geschrieben:
> > On Mon, Mar 14, 2022 at 05:21:22PM +0100, Paolo Bonzini wrote:
> > > On 3/14/22 15:07, Stefan Hajnoczi wrote:
> > > > If we can reach a consensus about C++ language usage in QEMU then I'm in
> > > > favor of using C++ coroutines. It's probably not realistic to think we
> > > > can limit C++ language usage to just coroutines forever. Someone finds
> > > > another C++ feature they absolutely need and over time the codebase
> > > > becomes C++ - with both its advantages and disadvantages.
> > > >
> > > > [...] although you can write C in C++, it's not idiomatic modern C++.
> > > > The language lends itself to a different style of programming that
> > > > some will embrace while others will not.
> > >
> > > Yes, this is an important aspect to discuss. I think coroutines provide a
> > > good blueprint for how QEMU might use C++.
> > >
> > > I totally agree that, if we go this way, the genie is out of the bottle and
> > > other uses of C++ will pop up with 100% probability. But the important
> > > thing to note is that our dialect of C is already not standard C, and that
> > > some of our or GLib's "innovations" are actually based on experience with
> > > C++. We can keep on writing "QEMU's C" if we think of C++ as a supercharged
> > > way of writing these quality-of-life improvements that we already write. In
> > > some sense coroutines are an extreme example of this idea.
> > >
> > > In fact, a C API would have to remain unless all source files are changed to
> > > C++, so QEMU would remain mostly a C project with C idioms, but that doesn't
> > > prevent _abstracting_ the use of C++ features (written in modern, idiomatic
> > > C++) behind an API that C programmers have no problems learning. Again,
> > > coroutines are an example of this of keeping the familiar create/enter/yield
> > > API and hiding the "magic" of C++ coroutines (and when they don't, that had
> > > better be an improvement).
> > >
> > > In the end, C++ is a tool that you can use if it leads to better code. For
> > > example, I don't see much use of C++ for devices for example, and the
> > > storage devices in particular do not even need coroutines because they use
> > > the callback-based interface. But perhaps someone will try to use templates
> > > to replace repeated inclusion (which is common in hw/display) and others
> > > will follow suit. Or perhaps not.
> > >
> > > One example that was brought up on IRC is type-safe operations on things
> > > such as hwaddr (i.e. hwaddr+int is allowed but hwaddr-hwaddr gives back an
> > > int64_t and might even check for overflow). These would be opt in (you get
> > > them just by changing a file from .c to .cc), but the actual C++ code would
> > > still look very much like C code that uses hwaddr with no type checking.
> > > All the operator overloading gunk would be in include/.
> > >
> > > A different topic is what would happen if all of QEMU could be compiled as
> > > C++, and could inform our selection of C++ idioms even long before we get
> > > there. For example, I'm fine with GLib and our type-safe intrusive lists,
> > > so I don't have much interest in STL containers and I would prefer _not_ to
> > > use STL containers even in .cc files[1]. However, perhaps QEMU's home-grown
> > > lock guard might be replaced by something that uses C++ destructors instead
> > > of g_autoptr, so perhaps we should consider using std::lock_guard<>, or
> > > something like that, instead of QEMU_LOCK_GUARD. It may be interesting to
> > > pass down lock_guards as arguments to enforce "this lock is taken"
> > > invariants.
> > >
> > > But really, coroutines would be enough work so my dish would be full for
> > > some time and I wouldn't really have time to look at any of this. :)
> >
> > I think it will be necessary to compile QEMU with a C++ compiler quite
> > soon. It is possible to provide C APIs like in the case of coroutines,
> > but sometimes C++ features need to be exposed to the caller (like the
> > lock guards you mentioned).
>
> I'm not sure what the C++ lock guards offer that our current lock guards
> don't? Passing down lock guards makes sense to me, but why can't you do
> that with QemuLockable? (Hm, or can the C++ version somehow check at
> compile time that it's the _right_ lock that is held rather than just
> any lock? It didn't look like it at the first sight.)
>
> But I do see the benefit of a compiler checked CoroutineFn<> return type
> compared to the coroutine_fn markers we have today. On the other hand...
>
> > Also, once C++ is available people will start submitting C++ patches
> > simply because they are more comfortable with C++ (especially
> > one-time/infrequent contributors).
>
> ...using C++ in coroutine code means that all of the block layer would
> suddenly become C++ and would be most affected by this effect. I'm not
> sure if that's something I would like to see, at least until there is a
> clear tree-wide policy (that preferably limits it to a subset that I
> understand).
Expecting maintainers to enforce a subset during code review feels
like it would be a tedious burden, that will inevitably let stuff
through because humans are fallible, especially when presented
with uninspiring, tedious, repetitive tasks.
Restricting ourselves to a subset is only viable if we have
an automated tool that can reliably enforce that subset. I'm not
sure that any such tool exists, and not convinced our time is
best served by trying to write & maintainer one either.
IOW, I fear one we allow C++ in any level, it won't be practical
to constrain it as much we desire. I fear us turning QEMU into
even more of a monster like other big C++ apps I see which take
all hours to compile while using all available RAM in Fedora RPM
build hosts.
My other question is whether adoption of C++ would complicate any
desire to make more use of Rust in QEMU ? I know Rust came out of
work by the Mozilla Firefox crew, and Firefox was C++, but I don't
have any idea how they integrated use of Rust with Firefox, so
whether there are any gotcha's for us or not ?
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-15 15:55 ` Daniel P. Berrangé
@ 2022-03-15 23:08 ` Paolo Bonzini
2022-03-16 12:40 ` Stefan Hajnoczi
` (2 more replies)
2022-03-31 11:52 ` Markus Armbruster
1 sibling, 3 replies; 53+ messages in thread
From: Paolo Bonzini @ 2022-03-15 23:08 UTC (permalink / raw)
To: Daniel P. Berrangé, Kevin Wolf
Cc: hreitz, Stefan Hajnoczi, qemu-devel, qemu-block
On 3/15/22 16:55, Daniel P. Berrangé wrote:
> Expecting maintainers to enforce a subset during code review feels
> like it would be a tedious burden, that will inevitably let stuff
> through because humans are fallible, especially when presented
> with uninspiring, tedious, repetitive tasks.
>
> Restricting ourselves to a subset is only viable if we have
> an automated tool that can reliably enforce that subset. I'm not
> sure that any such tool exists, and not convinced our time is
> best served by trying to write & maintainer one either.
We don't need to have a policy on which features are used. We need to
have goals for what to use C++ for. I won't go into further details
here, because I had already posted "When and how to use C++"[1] about an
hour before your reply.
> IOW, I fear one we allow C++ in any level, it won't be practical
> to constrain it as much we desire. I fear us turning QEMU into
> even more of a monster like other big C++ apps I see which take
> all hours to compile while using all available RAM in Fedora RPM
> build hosts.
Sorry but this is FUD. There's plenty of C++ apps and libraries that do
not "take hours to compile while using all available RAM". You're
probably thinking of the Chromium/Firefox/Libreoffice triplet but those
are an order of magnitude larger than QEMU. And in fact, QEMU is
*already* a monster that takes longer to compile than most other
packages, no matter the language they're written in.
Most of KDE and everything that uses Qt is written in C++, and so is
Inkscape in GTK+ land. LLVM and Clang are written in C++. Hotspot and
V8 are written in C++. Kodi, MAME and DolphinEmu are written in C++.
GCC and GDB have migrated to C++ and their compile times have not exploded.
> My other question is whether adoption of C++ would complicate any
> desire to make more use of Rust in QEMU ? I know Rust came out of
> work by the Mozilla Firefox crew, and Firefox was C++, but I don't
> have any idea how they integrated use of Rust with Firefox, so
> whether there are any gotcha's for us or not ?
Any Rust integration would go through C APIs. Using Rust in the block
layer would certainly be much harder, though perhaps not impossible, if
the block layer uses C++ coroutines. Rust supports something similar,
but two-direction interoperability would be hard.
For everything else, not much. Even if using C++, the fact that QEMU's
APIs are primarily C would not change. Changing "timer_mod_ns(timer,
ns)" to "timer.modify_ns(ns)" is not on the table.
But really, first of all the question should be who is doing work on
integrating Rust with QEMU. I typically hear about this topic exactly
once a year at KVM Forum, and then nothing. We have seen Marc-André's
QAPI integration experiment, but it's not clear to me what the path
would be from there to wider use in QEMU.
In particular, after ~3 years of talking about it, it is not even clear:
- what subsystems would benefit the most from the adoption of Rust, and
whether that would be feasible without a rewrite which will simply never
happen
- what the plans would be for coexistence of Rust and C code within a
subsystem
- whether maintainers would be on board with adopting a completely
different language, and who in the community has enough Rust experience
to shepherd us through the learning experience
The first two questions have answers in the other message if
s/Rust/C++/, and as to the last I think we're already further in the
discussion.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 53+ messages in thread* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-15 23:08 ` Paolo Bonzini
@ 2022-03-16 12:40 ` Stefan Hajnoczi
2022-03-16 16:15 ` Kevin Wolf
2022-03-17 12:16 ` Dr. David Alan Gilbert
2022-03-17 12:51 ` Daniel P. Berrangé
2 siblings, 1 reply; 53+ messages in thread
From: Stefan Hajnoczi @ 2022-03-16 12:40 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Kevin Wolf, hreitz, Daniel P. Berrangé, qemu-devel,
qemu-block
[-- Attachment #1: Type: text/plain, Size: 2642 bytes --]
On Wed, Mar 16, 2022 at 12:08:33AM +0100, Paolo Bonzini wrote:
> On 3/15/22 16:55, Daniel P. Berrangé wrote:
> > Expecting maintainers to enforce a subset during code review feels
> > like it would be a tedious burden, that will inevitably let stuff
> > through because humans are fallible, especially when presented
> > with uninspiring, tedious, repetitive tasks.
> >
> > Restricting ourselves to a subset is only viable if we have
> > an automated tool that can reliably enforce that subset. I'm not
> > sure that any such tool exists, and not convinced our time is
> > best served by trying to write & maintainer one either.
>
> We don't need to have a policy on which features are used. We need to have
> goals for what to use C++ for. I won't go into further details here,
> because I had already posted "When and how to use C++"[1] about an hour
> before your reply.
>
> > IOW, I fear one we allow C++ in any level, it won't be practical
> > to constrain it as much we desire. I fear us turning QEMU into
> > even more of a monster like other big C++ apps I see which take
> > all hours to compile while using all available RAM in Fedora RPM
> > build hosts.
>
> Sorry but this is FUD. There's plenty of C++ apps and libraries that do not
> "take hours to compile while using all available RAM". You're probably
> thinking of the Chromium/Firefox/Libreoffice triplet but those are an order
> of magnitude larger than QEMU. And in fact, QEMU is *already* a monster
> that takes longer to compile than most other packages, no matter the
> language they're written in.
>
> Most of KDE and everything that uses Qt is written in C++, and so is
> Inkscape in GTK+ land. LLVM and Clang are written in C++. Hotspot and V8
> are written in C++. Kodi, MAME and DolphinEmu are written in C++. GCC and
> GDB have migrated to C++ and their compile times have not exploded.
>
> > My other question is whether adoption of C++ would complicate any
> > desire to make more use of Rust in QEMU ? I know Rust came out of
> > work by the Mozilla Firefox crew, and Firefox was C++, but I don't
> > have any idea how they integrated use of Rust with Firefox, so
> > whether there are any gotcha's for us or not ?
>
> Any Rust integration would go through C APIs. Using Rust in the block layer
> would certainly be much harder, though perhaps not impossible, if the block
> layer uses C++ coroutines. Rust supports something similar, but
> two-direction interoperability would be hard.
I haven't looked at this in depth but there is a solution for Rust-C++
interop: https://cxx.rs/
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-16 12:40 ` Stefan Hajnoczi
@ 2022-03-16 16:15 ` Kevin Wolf
0 siblings, 0 replies; 53+ messages in thread
From: Kevin Wolf @ 2022-03-16 16:15 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Paolo Bonzini, hreitz, Daniel P. Berrangé, qemu-devel,
qemu-block
[-- Attachment #1: Type: text/plain, Size: 3589 bytes --]
Am 16.03.2022 um 13:40 hat Stefan Hajnoczi geschrieben:
> On Wed, Mar 16, 2022 at 12:08:33AM +0100, Paolo Bonzini wrote:
> > On 3/15/22 16:55, Daniel P. Berrangé wrote:
> > > Expecting maintainers to enforce a subset during code review feels
> > > like it would be a tedious burden, that will inevitably let stuff
> > > through because humans are fallible, especially when presented
> > > with uninspiring, tedious, repetitive tasks.
> > >
> > > Restricting ourselves to a subset is only viable if we have
> > > an automated tool that can reliably enforce that subset. I'm not
> > > sure that any such tool exists, and not convinced our time is
> > > best served by trying to write & maintainer one either.
> >
> > We don't need to have a policy on which features are used. We need to have
> > goals for what to use C++ for. I won't go into further details here,
> > because I had already posted "When and how to use C++"[1] about an hour
> > before your reply.
> >
> > > IOW, I fear one we allow C++ in any level, it won't be practical
> > > to constrain it as much we desire. I fear us turning QEMU into
> > > even more of a monster like other big C++ apps I see which take
> > > all hours to compile while using all available RAM in Fedora RPM
> > > build hosts.
> >
> > Sorry but this is FUD. There's plenty of C++ apps and libraries that do not
> > "take hours to compile while using all available RAM". You're probably
> > thinking of the Chromium/Firefox/Libreoffice triplet but those are an order
> > of magnitude larger than QEMU. And in fact, QEMU is *already* a monster
> > that takes longer to compile than most other packages, no matter the
> > language they're written in.
> >
> > Most of KDE and everything that uses Qt is written in C++, and so is
> > Inkscape in GTK+ land. LLVM and Clang are written in C++. Hotspot and V8
> > are written in C++. Kodi, MAME and DolphinEmu are written in C++. GCC and
> > GDB have migrated to C++ and their compile times have not exploded.
> >
> > > My other question is whether adoption of C++ would complicate any
> > > desire to make more use of Rust in QEMU ? I know Rust came out of
> > > work by the Mozilla Firefox crew, and Firefox was C++, but I don't
> > > have any idea how they integrated use of Rust with Firefox, so
> > > whether there are any gotcha's for us or not ?
> >
> > Any Rust integration would go through C APIs. Using Rust in the block layer
> > would certainly be much harder, though perhaps not impossible, if the block
> > layer uses C++ coroutines. Rust supports something similar, but
> > two-direction interoperability would be hard.
>
> I haven't looked at this in depth but there is a solution for Rust-C++
> interop: https://cxx.rs/
"Direct FFI of async functions is absolutely in scope for CXX (on C++20
and up) but is not implemented yet in the current release."
With the current QEMU coroutines, calling Rust async fns from C is
relatively easy, and calling C coroutine_fns from a Rust async fn is
trivial when the Rust async fn is already called from a C coroutine
(because qemu_coroutine_yield() just works, we still have a coroutine
stack from the original C caller).
I suppose calling Rust async fns from C++ could actually have the same
implementation as with C when using Paolo's wrappers, but the other
direction might be a bit harder - to be honest, I can't tell because
I've never checked how C++ coroutines work internally. Could as well be
that a Rust wrapper for them isn't that hard after all.
Kevin
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-15 23:08 ` Paolo Bonzini
2022-03-16 12:40 ` Stefan Hajnoczi
@ 2022-03-17 12:16 ` Dr. David Alan Gilbert
2022-03-17 12:51 ` Daniel P. Berrangé
2 siblings, 0 replies; 53+ messages in thread
From: Dr. David Alan Gilbert @ 2022-03-17 12:16 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Kevin Wolf, Daniel P. Berrangé, qemu-block, qemu-devel,
hreitz, Stefan Hajnoczi
* Paolo Bonzini (pbonzini@redhat.com) wrote:
> On 3/15/22 16:55, Daniel P. Berrangé wrote:
> > Expecting maintainers to enforce a subset during code review feels
> > like it would be a tedious burden, that will inevitably let stuff
> > through because humans are fallible, especially when presented
> > with uninspiring, tedious, repetitive tasks.
> >
> > Restricting ourselves to a subset is only viable if we have
> > an automated tool that can reliably enforce that subset. I'm not
> > sure that any such tool exists, and not convinced our time is
> > best served by trying to write & maintainer one either.
>
> We don't need to have a policy on which features are used. We need to have
> goals for what to use C++ for. I won't go into further details here,
> because I had already posted "When and how to use C++"[1] about an hour
> before your reply.
>
> > IOW, I fear one we allow C++ in any level, it won't be practical
> > to constrain it as much we desire. I fear us turning QEMU into
> > even more of a monster like other big C++ apps I see which take
> > all hours to compile while using all available RAM in Fedora RPM
> > build hosts.
>
> Sorry but this is FUD. There's plenty of C++ apps and libraries that do not
> "take hours to compile while using all available RAM". You're probably
> thinking of the Chromium/Firefox/Libreoffice triplet but those are an order
> of magnitude larger than QEMU. And in fact, QEMU is *already* a monster
> that takes longer to compile than most other packages, no matter the
> language they're written in.
>
> Most of KDE and everything that uses Qt is written in C++, and so is
> Inkscape in GTK+ land. LLVM and Clang are written in C++. Hotspot and V8
> are written in C++. Kodi, MAME and DolphinEmu are written in C++. GCC and
> GDB have migrated to C++ and their compile times have not exploded.
While I think it does take longer to compile, the bigger problem for
the CI setup is the amount of RAM-per-compile-process; it's not so much
the fact that those applications are huge that's the problem, it's that
a make -j ($threads) can run out of RAM.
Dave
> > My other question is whether adoption of C++ would complicate any
> > desire to make more use of Rust in QEMU ? I know Rust came out of
> > work by the Mozilla Firefox crew, and Firefox was C++, but I don't
> > have any idea how they integrated use of Rust with Firefox, so
> > whether there are any gotcha's for us or not ?
>
> Any Rust integration would go through C APIs. Using Rust in the block layer
> would certainly be much harder, though perhaps not impossible, if the block
> layer uses C++ coroutines. Rust supports something similar, but
> two-direction interoperability would be hard.
>
> For everything else, not much. Even if using C++, the fact that QEMU's APIs
> are primarily C would not change. Changing "timer_mod_ns(timer, ns)" to
> "timer.modify_ns(ns)" is not on the table.
>
> But really, first of all the question should be who is doing work on
> integrating Rust with QEMU. I typically hear about this topic exactly once
> a year at KVM Forum, and then nothing. We have seen Marc-André's QAPI
> integration experiment, but it's not clear to me what the path would be from
> there to wider use in QEMU.
>
> In particular, after ~3 years of talking about it, it is not even clear:
>
> - what subsystems would benefit the most from the adoption of Rust, and
> whether that would be feasible without a rewrite which will simply never
> happen
>
> - what the plans would be for coexistence of Rust and C code within a
> subsystem
>
> - whether maintainers would be on board with adopting a completely different
> language, and who in the community has enough Rust experience to shepherd us
> through the learning experience
>
> The first two questions have answers in the other message if s/Rust/C++/,
> and as to the last I think we're already further in the discussion.
>
> Thanks,
>
> Paolo
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-15 23:08 ` Paolo Bonzini
2022-03-16 12:40 ` Stefan Hajnoczi
2022-03-17 12:16 ` Dr. David Alan Gilbert
@ 2022-03-17 12:51 ` Daniel P. Berrangé
2 siblings, 0 replies; 53+ messages in thread
From: Daniel P. Berrangé @ 2022-03-17 12:51 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Kevin Wolf, hreitz, Stefan Hajnoczi, qemu-devel, qemu-block
On Wed, Mar 16, 2022 at 12:08:33AM +0100, Paolo Bonzini wrote:
> On 3/15/22 16:55, Daniel P. Berrangé wrote:
> > Expecting maintainers to enforce a subset during code review feels
> > like it would be a tedious burden, that will inevitably let stuff
> > through because humans are fallible, especially when presented
> > with uninspiring, tedious, repetitive tasks.
> >
> > Restricting ourselves to a subset is only viable if we have
> > an automated tool that can reliably enforce that subset. I'm not
> > sure that any such tool exists, and not convinced our time is
> > best served by trying to write & maintainer one either.
>
> We don't need to have a policy on which features are used. We need to have
> goals for what to use C++ for. I won't go into further details here,
> because I had already posted "When and how to use C++"[1] about an hour
> before your reply.
I see that mail, but I don't think it addresses my point.
We already use GLib and it provides a bunch of data structures,
objects and APIs, alot of which will overlap with C++ standard
library, not to mention QEMU's own stuff that predates GLib.
The guidelines say what we want to use C++ for which fine, but
it doesn't help maintainers/reviewers evaluating whether the
proposed C++ usage is desired.
Do we prefer GLib APIs and data structures for consistency &
interoperability with remaining C-only code, or do we prefer
the libstdc++, or do we allow both but set rules on when
to use each, or do we allow a free-for-all with author or
reviewer deciding based on their personal preference.
If we don't have a policy for C++ feature usage, then we
get the free-for-all by default. May be that is OK, maybe
not. Either way we need to declare our intent, so reviewers
know what to complain about and what to not.
Personally I'm not a fan of having a codebase with many
different APIs/data structures for doing the same job,
so would like a clear policy for what to use / prefer,
ideally one we can enforce with tools rather than rely
on humans to spot.
> > IOW, I fear one we allow C++ in any level, it won't be practical
> > to constrain it as much we desire. I fear us turning QEMU into
> > even more of a monster like other big C++ apps I see which take
> > all hours to compile while using all available RAM in Fedora RPM
> > build hosts.
>
> Sorry but this is FUD. There's plenty of C++ apps and libraries that do not
> "take hours to compile while using all available RAM". You're probably
> thinking of the Chromium/Firefox/Libreoffice triplet but those are an order
> of magnitude larger than QEMU. And in fact, QEMU is *already* a monster
> that takes longer to compile than most other packages, no matter the
> language they're written in.
I was particularly considering Ceph when I wrote the above, which
is bigger, but still a similar order of magnutude in size compared
to QEMU, but in Fedora Ceph takes 12 hours to compile on the slower
arch, vs QEMU's 3 hrs, or 3 hrs on x86_64 vs QEMU's 1 hr. Maybe I'm
being mistaken in blaming C++ in general, and it is something else
Or maybe certain features trigger this slowness and we can consider
if we should stay away from them
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-15 15:55 ` Daniel P. Berrangé
2022-03-15 23:08 ` Paolo Bonzini
@ 2022-03-31 11:52 ` Markus Armbruster
1 sibling, 0 replies; 53+ messages in thread
From: Markus Armbruster @ 2022-03-31 11:52 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Kevin Wolf, qemu-block, qemu-devel, hreitz, Stefan Hajnoczi,
Paolo Bonzini
Daniel P. Berrangé <berrange@redhat.com> writes:
> On Tue, Mar 15, 2022 at 03:50:26PM +0100, Kevin Wolf wrote:
[...]
>> ...using C++ in coroutine code means that all of the block layer would
>> suddenly become C++ and would be most affected by this effect. I'm not
>> sure if that's something I would like to see, at least until there is a
>> clear tree-wide policy (that preferably limits it to a subset that I
>> understand).
>
> Expecting maintainers to enforce a subset during code review feels
> like it would be a tedious burden, that will inevitably let stuff
> through because humans are fallible, especially when presented
> with uninspiring, tedious, repetitive tasks.
>
> Restricting ourselves to a subset is only viable if we have
> an automated tool that can reliably enforce that subset.
Concur. We're talking about a complex subset of an even more complex
whole, where few to none of us will fully understand either.
> I'm not
> sure that any such tool exists, and not convinced our time is
> best served by trying to write & maintainer one either.
>
> IOW, I fear one we allow C++ in any level, it won't be practical
> to constrain it as much we desire. I fear us turning QEMU into
> even more of a monster like other big C++ apps I see which take
> all hours to compile while using all available RAM in Fedora RPM
> build hosts.
While I'm certainly concerned about the often poor ergonomics of
compiling C++ code, I'm even more concerned about the poor ergonomics of
*grokking* C++ code.
> My other question is whether adoption of C++ would complicate any
> desire to make more use of Rust in QEMU ? I know Rust came out of
> work by the Mozilla Firefox crew, and Firefox was C++, but I don't
> have any idea how they integrated use of Rust with Firefox, so
> whether there are any gotcha's for us or not ?
Good question.
^ permalink raw reply [flat|nested] 53+ messages in thread
* When and how to use C++ (was Re: [PATCH experiment 00/16] C++20 coroutine backend)
2022-03-15 14:50 ` Kevin Wolf
2022-03-15 15:35 ` Stefan Hajnoczi
2022-03-15 15:55 ` Daniel P. Berrangé
@ 2022-03-15 17:23 ` Paolo Bonzini
2 siblings, 0 replies; 53+ messages in thread
From: Paolo Bonzini @ 2022-03-15 17:23 UTC (permalink / raw)
To: Kevin Wolf, Stefan Hajnoczi; +Cc: hreitz, berrange, qemu-devel, qemu-block
On 3/15/22 15:50, Kevin Wolf wrote:
> I'm not sure what the C++ lock guards offer that our current lock guards
> don't? Passing down lock guards makes sense to me, but why can't you do
> that with QemuLockable?
Passing a QemuLockable alone doesn't ensure that the lock has been
taken. I guess we could build a QemuLockGuard on top that ensures that.
> (Hm, or can the C++ version somehow check at
> compile time that it's the _right_ lock that is held rather than just
> any lock? It didn't look like it at the first sight.)
It's not related to lock guards but clang++ has thread-safety analysis
that checks for the right lock. It is in theory there for C as well but
it's mostly a joke. Ironically, the only good use that I've seen of it
is Marc-André's never committed work to check coroutine_fn markers at
compile-time[1].
> But I do see the benefit of a compiler checked CoroutineFn<> return type
> compared to the coroutine_fn markers we have today. On the other hand...
>
>> Also, once C++ is available people will start submitting C++ patches
>> simply because they are more comfortable with C++ (especially
>> one-time/infrequent contributors).
>
> ...using C++ in coroutine code means that all of the block layer would
> suddenly become C++ and would be most affected by this effect. I'm not
> sure if that's something I would like to see, at least until there is a
> clear tree-wide policy (that preferably limits it to a subset that I
> understand).
You are the maintainer of the block layer, so you would have the biggest
impact and obviously have input in such a policy. I obviously don't
have a policy ready[2], but I do have some goals:
1) whenever C++ is used to build an API for use in C++ source files:
Those source files and their debugging experience should look
like C, and not any more "foreign" than the "QEMU C" dialect
that we already have.
Rationale: we might have funky code in headers, but
"QEMU_WITH_LOCK_GUARD(x) { }" is understandable; the same would
hold for "CoroutineFn<void>", "co_await fn();" or if hwaddr
internally used operator overloading.
2) whenever C and C++ provide two different implementations of an API,
for use in C and C++ source files respectively:
If the same API is implemented in different ways for C and C++,
the semantics should be equivalent, and covered by both
C and C++ testcases.
Rationale: this should be a rare occasion, but there are
features such as _Generic that are C only, so it should be
catered for; QemuLockable came out already in this proof of
concept. The implementation is completely different for C
and C++, but it works the same (as proved to some extent
by testcases).
Looking again at the hwaddr example, it would be okay to forbid
"hwaddr - hwaddr" subtraction in C++ code. But it would not be
okay to have it return a signed integer in C++, while it
returns an unsigned integer in C. Could there be a C++-only
method "hwaddr.diff(hwaddr)" that does return a signed integer?
That would have to be discussed, right now my opinion is
"probably not" but see the next point too.
3) whenever C++ features (templates, classes, etc.) are used in .cc files:
The features should have a clear benefit over equivalent
C-only code (e.g. using the preprocessor instead of templates)
and if applicable they should publish a C API[3] that can be
consumed outside a particular subsystem.
If in doubt, err on the side of moderation, i.e. less C++ and
fewer C++ features.
Example: I can imagine that in some cases the semantic
replacement abilities provided by templates will for example
improve compile-time checking and reduce code duplication
compared to gcc. However, QEMU already has a wide range of
data structures and auxiliary APIs for C, and those should be
used for C++ code to avoid splitting the code base.
In turn, this should limit the usage of other C++ features.
For example, constructors (as opposed to just being able to
zero-initialize memory) can be more of a pain than a benefit
if you can't use "new", "delete" or "std::vector<>". Likewise,
it's unlikely that we'll have reasons to use anonymous
namespaces instead of "static", because that's mostly useful
for classes defined in .cc files and their member functions[5].
And VMState will prevent the usage of "private:", thus lowering
the benefit from methods.
Would this kind of formulation be enough? Maybe, maybe not, but either
way I don't really believe in prohibiting features by policy. The QEMU
community and especially its maintainers won't change overnight if C++
is allowed in the codebase; but they should be allowed to evolve their
opinions as their experience grows. If some day the block layer
maintainers want to extend coroutines to variable arguments rather than
a single void *, they shouldn't be forbidden from doing so just because
(I guess) it entails using variadic templates. They should show that
there is a benefit, though.
In any case, Stefan's suggestion to look into what other projects did,
especially GCC/GDB is a good one:
- https://gcc.gnu.org/wiki/CppConventions probably is a good start[4],
but we would probably cut on the "explicitly permitted" parts.
https://gcc.gnu.org/codingrationale.html also focuses a lot on things
that QEMU would likely not use.
- the discussion at
https://gcc.gnu.org/legacy-ml/gcc/2010-05/msg00705.html has some quotes
that capture interesting the point. It's possible that I have
subconsciously introduced it in my two messages on the topic:
The goal is not to reimplement GCC from the ground up using
modern OO/C++ techniques. The goal is simply to permit
ourselves to use C++ features where appropriate in the codebase.
---
The right subset is dependent on context. We're not in a
safety-critical environment, we have a large existing C
codebase, and we have a developer team made up of experienced
C programmers, not all of whom are used to programming in C++.
So, we need to take those factors into account.
---
Switching to C++ should never be excuse to bring more more
brittle codes or more obscurities. Rather, it should be
opportunity to write simpler and better code.
Also of interest is the replies to less enthusiastic developers starting
at https://gcc.gnu.org/legacy-ml/gcc/2010-05/msg00744.html at
https://gcc.gnu.org/legacy-ml/gcc/2010-06/msg00174.html.
- Richard might have some experience here, and his insight is valuable.
Paolo
[1] https://lists.gnu.org/archive/html/qemu-devel/2017-07/msg00793.html
[2] in fact, even if we decided to go with C++ coroutines, it would take
quite some time to split the coroutine/non-coroutine cases, which is a
prerequisite for that, so we'd have a lot of time to develop a policy.
Once the policy is there, there will be even more time before a
C++-using patch that does something useful gets submitted and approved.
[3] or at least C-like, for example qemu_coroutine_create technically is
a C++-only API because it takes a CoroutineFn<void>(*)(void *) argument.
But it *looks* like C.
[4] lol, there's even a remark fom me in there, but I stopped working on
GCC about at the time that the document was being written.
[5]
https://stackoverflow.com/questions/154469/unnamed-anonymous-namespaces-vs-static-functions
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-14 9:31 [PATCH experiment 00/16] C++20 coroutine backend Paolo Bonzini
` (16 preceding siblings ...)
2022-03-14 14:07 ` [PATCH experiment 00/16] C++20 coroutine backend Stefan Hajnoczi
@ 2022-03-14 16:52 ` Daniel P. Berrangé
2022-03-15 9:05 ` Paolo Bonzini
2022-03-15 16:15 ` Daniel P. Berrangé
18 siblings, 1 reply; 53+ messages in thread
From: Daniel P. Berrangé @ 2022-03-14 16:52 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kwolf, hreitz, qemu-block, qemu-devel, stefanha
On Mon, Mar 14, 2022 at 10:31:47AM +0100, Paolo Bonzini wrote:
> This was compiled with GCC 11 only. Coroutine support was added in
> GCC 10, released in 2020, which IIRC is much newer than the most recent
> release we support.
Currrently we target 7.4:
commit 2a85a08c998e418a46a308095893f223642f6fc9
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: Fri May 14 13:04:15 2021 +0100
configure: bump min required CLang to 6.0 / XCode 10.0
Several distros have been dropped since the last time we bumped the
minimum required CLang version.
Per repology, currently shipping versions are:
RHEL-8: 10.0.1
Debian Buster: 7.0.1
openSUSE Leap 15.2: 9.0.1
Ubuntu LTS 18.04: 6.0.0
Ubuntu LTS 20.04: 10.0.0
FreeBSD 12: 8.0.1
Fedora 33: 11.0.0
Fedora 34: 11.1.0
Ubuntu 18.04 drops off our list after 7.0 comes out
Buster is already off our list as that hit the 2 year
mark in AUg 2021.
OpenSUSE Leap 15.2 was EOL'd by SUSE themselves in Jan 2022,
We use it as a proxy for SLES, but I think we can required
SLES 15 sp3.
FreeBSD 12 is something we still support until April 2023,
but arguably we only care about CLang there.
NetBSD 9 wasn't listed, but it was reported to required
GCC 7.4 (commit 3830df5f83b9b52d9496763ce1a50afb9231c998)
and that is still the latest release of NetBSD.
So NetBSD is our biggest constraint on requiring GCC 10
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 53+ messages in thread* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-14 16:52 ` [PATCH experiment 00/16] C++20 coroutine backend Daniel P. Berrangé
@ 2022-03-15 9:05 ` Paolo Bonzini
2022-03-15 9:32 ` Daniel P. Berrangé
0 siblings, 1 reply; 53+ messages in thread
From: Paolo Bonzini @ 2022-03-15 9:05 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: kwolf, hreitz, stefanha, qemu-devel, qemu-block
On 3/14/22 17:52, Daniel P. Berrangé wrote:
> RHEL-8: 10.0.1
> openSUSE Leap 15.3: 9.0.1
> Ubuntu LTS 18.04: 6.0.0
> FreeBSD 12: 8.0.1
>
> Ubuntu 18.04 drops off our list after 7.0 comes out
>
> OpenSUSE Leap 15.2 was EOL'd by SUSE themselves in Jan 2022,
> We use it as a proxy for SLES, but I think we can required
> SLES 15 sp3.
(FTR, OpenSUSE Leap 15.3 has GCC 10.3.0).
> FreeBSD 12 is something we still support until April 2023,
> but arguably we only care about CLang there.
>
> NetBSD 9 wasn't listed, but it was reported to required
> GCC 7.4 (commit 3830df5f83b9b52d9496763ce1a50afb9231c998)
> and that is still the latest release of NetBSD.
>
> So NetBSD is our biggest constraint on requiring GCC 10
Do we care about the BSDs since they have newer compilers (including
gcc10) available in pkgsrc? If you go by the base system, then RHEL8
has 8.5.0 and newer version are only available with packages such as
gcc-toolset-10 and gcc-toolset-11.
Paolo
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-15 9:05 ` Paolo Bonzini
@ 2022-03-15 9:32 ` Daniel P. Berrangé
2022-03-15 17:27 ` Paolo Bonzini
0 siblings, 1 reply; 53+ messages in thread
From: Daniel P. Berrangé @ 2022-03-15 9:32 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kwolf, hreitz, stefanha, qemu-devel, qemu-block
On Tue, Mar 15, 2022 at 10:05:32AM +0100, Paolo Bonzini wrote:
> On 3/14/22 17:52, Daniel P. Berrangé wrote:
> > RHEL-8: 10.0.1
> > openSUSE Leap 15.3: 9.0.1
> > Ubuntu LTS 18.04: 6.0.0
> > FreeBSD 12: 8.0.1
> >
> > Ubuntu 18.04 drops off our list after 7.0 comes out
> >
> > OpenSUSE Leap 15.2 was EOL'd by SUSE themselves in Jan 2022,
> > We use it as a proxy for SLES, but I think we can required
> > SLES 15 sp3.
>
> (FTR, OpenSUSE Leap 15.3 has GCC 10.3.0).
>
> > FreeBSD 12 is something we still support until April 2023,
> > but arguably we only care about CLang there.
> >
> > NetBSD 9 wasn't listed, but it was reported to required
> > GCC 7.4 (commit 3830df5f83b9b52d9496763ce1a50afb9231c998)
> > and that is still the latest release of NetBSD.
> >
> > So NetBSD is our biggest constraint on requiring GCC 10
>
> Do we care about the BSDs since they have newer compilers (including gcc10)
> available in pkgsrc? If you go by the base system, then RHEL8 has 8.5.0 and
> newer version are only available with packages such as gcc-toolset-10 and
> gcc-toolset-11.
I mention NetBSD because we had an explicit request to support
7.4 GCC from there, as it was the current system compiler.
That's a mistake in my original commit logs then wrt RHEL8, as I
only ever intended to consider the standard base repos, not random
addon repos.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-15 9:32 ` Daniel P. Berrangé
@ 2022-03-15 17:27 ` Paolo Bonzini
2022-03-15 18:12 ` Daniel P. Berrangé
0 siblings, 1 reply; 53+ messages in thread
From: Paolo Bonzini @ 2022-03-15 17:27 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: kwolf, qemu-block, qemu-devel, hreitz, stefanha, Warner Losh
On 3/15/22 10:32, Daniel P. Berrangé wrote:
>>> So NetBSD is our biggest constraint on requiring GCC 10
>>
>> Do we care about the BSDs since they have newer compilers (including gcc10)
>> available in pkgsrc? If you go by the base system, then RHEL8 has 8.5.0 and
>> newer version are only available with packages such as gcc-toolset-10 and
>> gcc-toolset-11.
>
> I mention NetBSD because we had an explicit request to support
> 7.4 GCC from there, as it was the current system compiler.
Thanks, do you have a reference? I would like to understand the reason
for that (adding Werner too).
For RHEL8, considering that the switch would be several months away
anyway, I don't think it would be an issue to require AppStream toolset
packages. It's unlikely that RHEL8 would rebase QEMU in 2023 and beyond.
Paolo
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-15 17:27 ` Paolo Bonzini
@ 2022-03-15 18:12 ` Daniel P. Berrangé
0 siblings, 0 replies; 53+ messages in thread
From: Daniel P. Berrangé @ 2022-03-15 18:12 UTC (permalink / raw)
To: Paolo Bonzini
Cc: kwolf, qemu-block, qemu-devel, hreitz, stefanha, Warner Losh
On Tue, Mar 15, 2022 at 06:27:57PM +0100, Paolo Bonzini wrote:
> On 3/15/22 10:32, Daniel P. Berrangé wrote:
> > > > So NetBSD is our biggest constraint on requiring GCC 10
> > >
> > > Do we care about the BSDs since they have newer compilers (including gcc10)
> > > available in pkgsrc? If you go by the base system, then RHEL8 has 8.5.0 and
> > > newer version are only available with packages such as gcc-toolset-10 and
> > > gcc-toolset-11.
> >
> > I mention NetBSD because we had an explicit request to support
> > 7.4 GCC from there, as it was the current system compiler.
>
> Thanks, do you have a reference? I would like to understand the reason for
> that (adding Werner too).
It was commit 3830df5f83b9b52d9496763ce1a50afb9231c998
> For RHEL8, considering that the switch would be several months away anyway,
> I don't think it would be an issue to require AppStream toolset packages.
> It's unlikely that RHEL8 would rebase QEMU in 2023 and beyond.
It isn't so much RHEL8 rebases I'm thinking of wrt the policy, but
rather our corporate contributors who can be constrained by annoying
corporate policies to only use RHEL-8 for their work.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-14 9:31 [PATCH experiment 00/16] C++20 coroutine backend Paolo Bonzini
` (17 preceding siblings ...)
2022-03-14 16:52 ` [PATCH experiment 00/16] C++20 coroutine backend Daniel P. Berrangé
@ 2022-03-15 16:15 ` Daniel P. Berrangé
2022-03-15 17:50 ` Paolo Bonzini
18 siblings, 1 reply; 53+ messages in thread
From: Daniel P. Berrangé @ 2022-03-15 16:15 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kwolf, hreitz, qemu-block, qemu-devel, stefanha
On Mon, Mar 14, 2022 at 10:31:47AM +0100, Paolo Bonzini wrote:
> However, there are no ramifications to actual coroutine code, except
> for the template syntax "CoroutineFn<return_type>" for the function and
> the mandatory co_await/co_return keywords... both of which are an
> improvement, really: the fact that a single function cannot run either
> inside or outside coroutines is checked by the compiler now, because
> qemu_coroutine_create accepts a function that returns CoroutineFn<void>.
> Therefore I had to disable some more code in util/ and qapi/ that used
> qemu_in_coroutine() or coroutine_fn.
Bear with me as I suggest something potentially/probably silly
given my limited knowledge of C++ coroutines.
Given a function I know about:
void coroutine_fn qio_channel_yield(QIOChannel *ioc,
GIOCondition condition);
IIUC, you previously indicated that the header file declaration,
the implementation and any callers of this would need to be in
C++ source files.
The caller is what I'm most curious about, because I feel that
is where the big ripple effects come into play that cause large
parts of QEMU to become C++ code.
In general it is possible to call C++ functions from C.
I presume there is something special about the CoroutineFn<void>
prototype preventing that from working as needed, thus requiring
the caller to be compiled as C++ ? IIUC compiling as C++ though
is not neccessarily the same as using C++ linkage.
So I'm assuming the caller as C++ requirement is not recursive,
otherwise it would immediately mean all of QEMU needs to be C++.
This leads me to wonder if we can workaround this problem with
a wrapper function. eg in a io/channel.hpp file can be declare
something like:
CoroutineFn<void> qio_channel_yield_impl(QIOChannel *ioc,
GIOCondition condition);
extern "C" {
static inline void qio_chanel_yield(QIOChannel *ioc,
GIOCondition condition) {
qio_channel_yield_impl(ioc, condition)
}
}
With this qio_channel_yield_impl and qio_channel_yield are both
compiled as C++, but qio_channel_yield is exposed with C linkage
semantics. Thus enabling callers of qio_channel_yield can carry
on being compiled as C, since the invokation of the CoroutineFn
is in the inline C++ function ?
This would mean an extra function call, but perhaps this gets
optimized away, espeically with LTO, such that it doesn't impact
performance negatively ?
The impl of qio_channel_yield_impl() could also call from C++
back to C functions as much as possible.
IOW, can we get it such that the C++ bit is just a thin shim
"C -> C++ wrapper -> C++ CoroutineFn -> C", enabling all the
C++ bits to be well encapsulated and thus prevent arbitrary
usage of C++ features leaking all across the codebase ?
With Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 53+ messages in thread* Re: [PATCH experiment 00/16] C++20 coroutine backend
2022-03-15 16:15 ` Daniel P. Berrangé
@ 2022-03-15 17:50 ` Paolo Bonzini
0 siblings, 0 replies; 53+ messages in thread
From: Paolo Bonzini @ 2022-03-15 17:50 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: kwolf, hreitz, stefanha, qemu-devel, qemu-block
On 3/15/22 17:15, Daniel P. Berrangé wrote:
> Bear with me as I suggest something potentially/probably silly
> given my limited knowledge of C++ coroutines.
>
> Given a function I know about:
>
> void coroutine_fn qio_channel_yield(QIOChannel *ioc,
> GIOCondition condition);
>
> IIUC, you previously indicated that the header file declaration,
> the implementation and any callers of this would need to be in
> C++ source files.
>
> The caller is what I'm most curious about, because I feel that
> is where the big ripple effects come into play that cause large
> parts of QEMU to become C++ code. [...]
> I presume there is something special about the CoroutineFn<void>
> prototype preventing that from working as needed, thus requiring
> the caller to be compiled as C++ ? IIUC compiling as C++ though
> is not neccessarily the same as using C++ linkage.
Yes, the CoroutineFn<void> function must either be passed to
qemu_coroutine_create() or called as "co_await f()". If you call it as
"f()" it does nothing except leak the memory needed by its stack frame,
so that only leaves passing the function to qemu_coroutine_create().
I suppose you could do some games with typedefs, like
#ifdef __cplusplus
typedef CoroutineFn<void> VoidCoroutine
#else
typedef struct VoidCoroutine VoidCoroutine;
#endif
to be able to declare a function that returns CoroutineFn<void> but I'm
not sure of the advantage.
> So I'm assuming the caller as C++ requirement is not recursive,
> otherwise it would immediately mean all of QEMU needs to be C++.
Right, qemu_coroutine_create() must be called from C++ but the caller of
qemu_coroutine_create() can be extern "C". In the particular case of
the block layer, callers of qemu_coroutine_create() include
callback-based functions such as bdrv_aio_readv(), and synchronous
functions such as bdrv_flush(). Both of these can be called from C.
> IOW, can we get it such that the C++ bit is just a thin shim
> "C -> C++ wrapper -> C++ CoroutineFn -> C", enabling all the
> C++ bits to be well encapsulated and thus prevent arbitrary
> usage of C++ features leaking all across the codebase ?
No, unfortunately not. But in particular, even though the block layer
would be C++, device models that use it would not.
Paolo
^ permalink raw reply [flat|nested] 53+ messages in thread