Skip to content

Commit

Permalink
coroutine: use QEMU_DEFINE_STATIC_CO_TLS()
Browse files Browse the repository at this point in the history
Thread-Local Storage variables cannot be used directly from coroutine
code because the compiler may optimize TLS variable accesses across
qemu_coroutine_yield() calls. When the coroutine is re-entered from
another thread the TLS variables from the old thread must no longer be
used.

Use QEMU_DEFINE_STATIC_CO_TLS() for the current and leader variables.
The alloc_pool QSLIST needs a typedef so the return value of
get_ptr_alloc_pool() can be stored in a local variable.

One example of why this code is necessary: a coroutine that yields
before calling qemu_coroutine_create() to create another coroutine is
affected by the TLS issue.

Signed-off-by: Stefan Hajnoczi <[email protected]>
Message-Id: <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Signed-off-by: Kevin Wolf <[email protected]>
(cherry picked from commit ac387a08a9c9f6b36757da912f0339c25f421f90)
  • Loading branch information
stefanhaRH authored and arichardson committed Jul 27, 2024
1 parent 25e5a1d commit 23b5b08
Showing 1 changed file with 24 additions and 17 deletions.
41 changes: 24 additions & 17 deletions util/qemu-coroutine.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "qemu/atomic.h"
#include "qemu/coroutine.h"
#include "qemu/coroutine_int.h"
#include "qemu/coroutine-tls.h"
#include "block/aio.h"

enum {
Expand All @@ -27,17 +28,20 @@ enum {
/** Free list to speed up creation */
static QSLIST_HEAD(, Coroutine) release_pool = QSLIST_HEAD_INITIALIZER(pool);
static unsigned int release_pool_size;
static __thread QSLIST_HEAD(, Coroutine) alloc_pool = QSLIST_HEAD_INITIALIZER(pool);
static __thread unsigned int alloc_pool_size;
static __thread Notifier coroutine_pool_cleanup_notifier;

typedef QSLIST_HEAD(, Coroutine) CoroutineQSList;
QEMU_DEFINE_STATIC_CO_TLS(CoroutineQSList, alloc_pool);
QEMU_DEFINE_STATIC_CO_TLS(unsigned int, alloc_pool_size);
QEMU_DEFINE_STATIC_CO_TLS(Notifier, coroutine_pool_cleanup_notifier);

static void coroutine_pool_cleanup(Notifier *n, void *value)
{
Coroutine *co;
Coroutine *tmp;
CoroutineQSList *alloc_pool = get_ptr_alloc_pool();

QSLIST_FOREACH_SAFE(co, &alloc_pool, pool_next, tmp) {
QSLIST_REMOVE_HEAD(&alloc_pool, pool_next);
QSLIST_FOREACH_SAFE(co, alloc_pool, pool_next, tmp) {
QSLIST_REMOVE_HEAD(alloc_pool, pool_next);
qemu_coroutine_delete(co);
}
}
Expand All @@ -47,27 +51,30 @@ Coroutine *qemu_coroutine_create(CoroutineEntry *entry, void *opaque)
Coroutine *co = NULL;

if (CONFIG_COROUTINE_POOL) {
co = QSLIST_FIRST(&alloc_pool);
CoroutineQSList *alloc_pool = get_ptr_alloc_pool();

co = QSLIST_FIRST(alloc_pool);
if (!co) {
if (release_pool_size > POOL_BATCH_SIZE) {
/* Slow path; a good place to register the destructor, too. */
if (!coroutine_pool_cleanup_notifier.notify) {
coroutine_pool_cleanup_notifier.notify = coroutine_pool_cleanup;
qemu_thread_atexit_add(&coroutine_pool_cleanup_notifier);
Notifier *notifier = get_ptr_coroutine_pool_cleanup_notifier();
if (!notifier->notify) {
notifier->notify = coroutine_pool_cleanup;
qemu_thread_atexit_add(notifier);
}

/* This is not exact; there could be a little skew between
* release_pool_size and the actual size of release_pool. But
* it is just a heuristic, it does not need to be perfect.
*/
alloc_pool_size = qatomic_xchg(&release_pool_size, 0);
QSLIST_MOVE_ATOMIC(&alloc_pool, &release_pool);
co = QSLIST_FIRST(&alloc_pool);
set_alloc_pool_size(qatomic_xchg(&release_pool_size, 0));
QSLIST_MOVE_ATOMIC(alloc_pool, &release_pool);
co = QSLIST_FIRST(alloc_pool);
}
}
if (co) {
QSLIST_REMOVE_HEAD(&alloc_pool, pool_next);
alloc_pool_size--;
QSLIST_REMOVE_HEAD(alloc_pool, pool_next);
set_alloc_pool_size(get_alloc_pool_size() - 1);
}
}

Expand All @@ -91,9 +98,9 @@ static void coroutine_delete(Coroutine *co)
qatomic_inc(&release_pool_size);
return;
}
if (alloc_pool_size < POOL_BATCH_SIZE) {
QSLIST_INSERT_HEAD(&alloc_pool, co, pool_next);
alloc_pool_size++;
if (get_alloc_pool_size() < POOL_BATCH_SIZE) {
QSLIST_INSERT_HEAD(get_ptr_alloc_pool(), co, pool_next);
set_alloc_pool_size(get_alloc_pool_size() + 1);
return;
}
}
Expand Down

0 comments on commit 23b5b08

Please sign in to comment.