From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38498) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c1bol-00033j-Sl for qemu-devel@nongnu.org; Tue, 01 Nov 2016 12:21:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c1boh-0004Ju-Tf for qemu-devel@nongnu.org; Tue, 01 Nov 2016 12:21:43 -0400 Received: from mail-wm0-x235.google.com ([2a00:1450:400c:c09::235]:37924) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1c1boh-0004Jo-F1 for qemu-devel@nongnu.org; Tue, 01 Nov 2016 12:21:39 -0400 Received: by mail-wm0-x235.google.com with SMTP id n67so295680713wme.1 for ; Tue, 01 Nov 2016 09:21:39 -0700 (PDT) References: <20161021115442.18420-1-alex.bennee@linaro.org> <1776cd73-bfa5-8af8-5d76-34d9760a1664@redhat.com> From: Alex =?utf-8?Q?Benn=C3=A9e?= In-reply-to: <1776cd73-bfa5-8af8-5d76-34d9760a1664@redhat.com> Date: Tue, 01 Nov 2016 16:21:36 +0000 Message-ID: <871syvxjan.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH] cpus: make qemu_mutex_iothread_locked() understand co-routines List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: stefanha@redhat.com, qemu-devel@nongnu.org, Peter Crosthwaite , Richard Henderson Paolo Bonzini writes: > 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 >> Cc: Stefan Hajnoczi > > 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? The current production stuff is due to missing support for new stacks with setcontext. However I have built the latest tsan support library and that seems happy without the gthread co-routines. Currently I'm dealing with glib's racy gthread support however. > > 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) >> -- Alex Bennée