On 21/10/2016 13:54, Alex Bennée wrote:
> There is a slight wart when checking for the state of the BQL when using
> GThread base co-routines (which we keep for ThreadSanitizer runs). While
> the main-loop holds the BQL it is suspended until the co-routine
> completes however the co-routines run in a separate thread so checking
> the TLS variable could be wrong.
>
> We fix this by expanding the check to include qemu_in_coroutine() for
> GThread based builds. As it is not used for production builds I'm not
> overly worried about any performance impact which should be negligible
> anyway.
>
> Signed-off-by: Alex Bennée <[email protected]>
> Cc: Stefan Hajnoczi <[email protected]>
This is wrong unfortunately. It is possible to run coroutines outside
the BQL (e.g. with -device virtio-blk,iothread=foo).
Do you know exactly why TSAN has no love for coroutines?
Paolo
> ---
> configure | 3 +++
> cpus.c | 13 +++++++++++++
> 2 files changed, 16 insertions(+)
>
> diff --git a/configure b/configure
> index 91a14c1..97b89fb 100755
> --- a/configure
> +++ b/configure
> @@ -5461,6 +5461,9 @@ if test "$rbd" = "yes" ; then
> fi
>
> echo "CONFIG_COROUTINE_BACKEND=$coroutine" >> $config_host_mak
> +if test "$coroutine" = "gthread" ; then
> + echo "CONFIG_COROUTINE_GTHREAD=1" >> $config_host_mak
> +fi
> if test "$coroutine_pool" = "yes" ; then
> echo "CONFIG_COROUTINE_POOL=1" >> $config_host_mak
> else
> diff --git a/cpus.c b/cpus.c
> index 0c18a9f..a3e189a 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -49,6 +49,10 @@
> #include "hw/nmi.h"
> #include "sysemu/replay.h"
>
> +#ifdef CONFIG_COROUTINE_GTHREAD
> +#include "qemu/coroutine.h"
> +#endif
> +
> #ifndef _WIN32
> #include "qemu/compatfd.h"
> #endif
> @@ -1422,9 +1426,18 @@ bool qemu_in_vcpu_thread(void)
>
> static __thread bool iothread_locked = false;
>
> +/*
> + * There is a slight wart when using gthread based co-routines. Here
> + * the BQL is held by the main-thread which is suspended until the
> + * co-routines complete.
> + */
> bool qemu_mutex_iothread_locked(void)
> {
> +#ifdef CONFIG_COROUTINE_GTHREAD
> + return iothread_locked || qemu_in_coroutine();
> +#else
> return iothread_locked;
> +#endif
> }
>
> void qemu_mutex_lock_iothread(void)
>
