Re: [PULL 11/13] coroutine-ucontext: use QEMU_DEFINE_STATIC_CO_TLS()

[Peter Maydell]

On Wed, 4 May 2022 at 15:34, Kevin Wolf <kwolf@redhat.com> wrote:
>
> From: Stefan Hajnoczi <stefanha@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.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> Message-Id: <20220307153853.602859-2-stefanha@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  util/coroutine-ucontext.c | 38 ++++++++++++++++++++++++--------------
>  1 file changed, 24 insertions(+), 14 deletions(-)
>
> diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
> index ed368e1a3e..ddc98fb4f8 100644
> --- a/util/coroutine-ucontext.c
> +++ b/util/coroutine-ucontext.c
> @@ -25,6 +25,7 @@
>  #include "qemu/osdep.h"
>  #include <ucontext.h>
>  #include "qemu/coroutine_int.h"
> +#include "qemu/coroutine-tls.h"
>
>  #ifdef CONFIG_VALGRIND_H
>  #include <valgrind/valgrind.h>
> @@ -66,8 +67,8 @@ typedef struct {
>  /**
>   * Per-thread coroutine bookkeeping
>   */
> -static __thread CoroutineUContext leader;
> -static __thread Coroutine *current;
> +QEMU_DEFINE_STATIC_CO_TLS(Coroutine *, current);
> +QEMU_DEFINE_STATIC_CO_TLS(CoroutineUContext, leader);

Hi; Coverity complains about this change (CID 1488745):

# Big parameter passed by value (PASS_BY_VALUE)
# pass_by_value: Passing parameter v of type CoroutineUContext
# (size 304 bytes) by value, which exceeds the medium threshold
# of 256 bytes.

thanks
-- PMM