From 9c2e55d25fec6ffb21e344513b7dbeed7e21f641 Mon Sep 17 00:00:00 2001

From: Stefan Hajnoczi <stefanha@redhat.com>

Date: Tue, 17 May 2022 12:08:04 +0100

Subject: [PATCH 2/6] coroutine: use QEMU_DEFINE_STATIC_CO_TLS()

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

RH-Author: Stefan Hajnoczi <stefanha@redhat.com>

RH-MergeRequest: 89: coroutine: use coroutine TLS macros to protect thread-local variables

RH-Commit: [2/3] 68a8847e406e2eace6ddc31b0c5676a60600d606 (stefanha/centos-stream-qemu-kvm)

RH-Bugzilla: 1952483

RH-Acked-by: Hanna Reitz <hreitz@redhat.com>

RH-Acked-by: Eric Blake <eblake@redhat.com>

RH-Acked-by: Kevin Wolf <kwolf@redhat.com>

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 <stefanha@redhat.com>

Message-Id: <20220307153853.602859-3-stefanha@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

(cherry picked from commit ac387a08a9c9f6b36757da912f0339c25f421f90)

Conflicts:

- Context conflicts due to commit 5411171c3ef4 ("coroutine: Revert to

  constant batch size").

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

---

 util/qemu-coroutine.c | 41 ++++++++++++++++++++++++-----------------

 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c

index 804f672e0a..4a8bd63ef0 100644

--- a/util/qemu-coroutine.c

+++ b/util/qemu-coroutine.c

@@ -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"

 /**

@@ -35,17 +36,20 @@ enum {

 static QSLIST_HEAD(, Coroutine) release_pool = QSLIST_HEAD_INITIALIZER(pool);

 static unsigned int pool_max_size = POOL_INITIAL_MAX_SIZE;

 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);

     }

 }

@@ -55,27 +59,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_MIN_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);

         }

     }

@@ -99,9 +106,9 @@ static void coroutine_delete(Coroutine *co)

             qatomic_inc(&release_pool_size);

             return;

         }

-        if (alloc_pool_size < qatomic_read(&pool_max_size)) {

-            QSLIST_INSERT_HEAD(&alloc_pool, co, pool_next);

-            alloc_pool_size++;

+        if (get_alloc_pool_size() < qatomic_read(&pool_max_size)) {

+            QSLIST_INSERT_HEAD(get_ptr_alloc_pool(), co, pool_next);

+            set_alloc_pool_size(get_alloc_pool_size() + 1);

             return;

         }

     }

-- 

2.31.1