[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PULL 11/13] coroutine-ucontext: use QEMU_DEFINE_STATIC_CO_TLS()
From: |
Kevin Wolf |
Subject: |
Re: [PULL 11/13] coroutine-ucontext: use QEMU_DEFINE_STATIC_CO_TLS() |
Date: |
Fri, 13 May 2022 17:54:28 +0200 |
Am 13.05.2022 um 17:27 hat Peter Maydell geschrieben:
> 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.
The macro defines functions get_leader()/set_leader(), which would
indeed have this problem, but they are never called. Not sure if it's
worth having a separate macro that avoids generating these unused
functions for cases like this, but in practice it's a false positive.
Kevin
- [PULL 09/13] block/vmdk: Fix reopening bs->file, (continued)
- [PULL 09/13] block/vmdk: Fix reopening bs->file, Kevin Wolf, 2022/05/04
- [PULL 01/13] qemu-img: properly list formats which have consistency check implemented, Kevin Wolf, 2022/05/04
- [PULL 07/13] Revert "main-loop: Disable GLOBAL_STATE_CODE() assertions", Kevin Wolf, 2022/05/04
- [PULL 13/13] coroutine-win32: use QEMU_DEFINE_STATIC_CO_TLS(), Kevin Wolf, 2022/05/04
- [PULL 05/13] block: Classify bdrv_get_flags() as I/O function, Kevin Wolf, 2022/05/04
- [PULL 10/13] iotests/reopen-file: Test reopening file child, Kevin Wolf, 2022/05/04
- [PULL 12/13] coroutine: use QEMU_DEFINE_STATIC_CO_TLS(), Kevin Wolf, 2022/05/04
- [PULL 08/13] iotests: Add regression test for issue 945, Kevin Wolf, 2022/05/04
- [PULL 11/13] coroutine-ucontext: use QEMU_DEFINE_STATIC_CO_TLS(), Kevin Wolf, 2022/05/04
- Re: [PULL 00/13] Block layer patches, Richard Henderson, 2022/05/05