From patchwork Wed Aug 6 20:39:33 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Roth X-Patchwork-Id: 35003 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-oa0-f72.google.com (mail-oa0-f72.google.com [209.85.219.72]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 923AD20523 for ; Wed, 6 Aug 2014 21:45:34 +0000 (UTC) Received: by mail-oa0-f72.google.com with SMTP id m1sf12678088oag.11 for ; Wed, 06 Aug 2014 14:45:34 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:delivered-to:from:to:date :message-id:in-reply-to:references:cc:subject:precedence:list-id :list-unsubscribe:list-archive:list-post:list-help:list-subscribe :errors-to:sender:x-original-sender :x-original-authentication-results:mailing-list; bh=6MNvdqr7MfQI+ZdlYg6SeINxKeQu2O3IONqjTJ+4O/I=; b=VQBKyQCuXnrV93lpMcgDtJIzBgmg1i+E/YlofC6fA6ruRmOWqThYvzEK+dfHOEhioQ eXCiTJkrm7xWG25qLY3U6WkCUar6NDW/gkEl3pQevNwJRqrwAsdkMpa8xaYj2hkYiFrg 4qWHFr3nEAVhfwXLh+pUhP9tGakdeekVtThJ9VuSauIJZJwmesB+uiQPcxjVl7AVDDLw HbVVq9T6NNprCRcm8MJk+9HdY8NeTpsBKIwMa6Zw0T6JroDdE6m2638VEU2qFX7VVKLH lK9uiB31YAIOV6bw8HGrZgQVQY7Xj0ue7eOYuoaq64TBKVqsZt3mHaxY/Zj38WhzlyJM N2zw== X-Gm-Message-State: ALoCoQmoW8T7lg0VhCQ22MBLBHN14AxI2j3o59qqSRsmzv3P3R9iPzM+hsw8mBDU8Ocp5sdu9vUG X-Received: by 10.50.153.98 with SMTP id vf2mr7162416igb.5.1407361533951; Wed, 06 Aug 2014 14:45:33 -0700 (PDT) MIME-Version: 1.0 X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.37.6 with SMTP id q6ls385742qgq.49.gmail; Wed, 06 Aug 2014 14:45:33 -0700 (PDT) X-Received: by 10.53.7.204 with SMTP id de12mr10779306vdd.41.1407361533804; Wed, 06 Aug 2014 14:45:33 -0700 (PDT) Received: from mail-vc0-f177.google.com (mail-vc0-f177.google.com [209.85.220.177]) by mx.google.com with ESMTPS id oh5si1003826vcb.6.2014.08.06.14.45.33 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 06 Aug 2014 14:45:33 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.220.177 as permitted sender) client-ip=209.85.220.177; Received: by mail-vc0-f177.google.com with SMTP id hy4so4953333vcb.8 for ; Wed, 06 Aug 2014 14:45:33 -0700 (PDT) X-Received: by 10.220.114.5 with SMTP id c5mr13072929vcq.28.1407361533687; Wed, 06 Aug 2014 14:45:33 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.221.37.5 with SMTP id tc5csp60148vcb; Wed, 6 Aug 2014 14:45:33 -0700 (PDT) X-Received: by 10.224.128.9 with SMTP id i9mr21554269qas.50.1407361532984; Wed, 06 Aug 2014 14:45:32 -0700 (PDT) Received: from lists.gnu.org (lists.gnu.org. [2001:4830:134:3::11]) by mx.google.com with ESMTPS id j49si3580024qga.126.2014.08.06.14.45.32 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Wed, 06 Aug 2014 14:45:32 -0700 (PDT) Received-SPF: pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) client-ip=2001:4830:134:3::11; Received: from localhost ([::1]:41652 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XF91Y-0007NM-EO for patch@linaro.org; Wed, 06 Aug 2014 17:45:32 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39346) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XF84I-0002A0-NA for qemu-devel@nongnu.org; Wed, 06 Aug 2014 16:44:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XF83v-0005kI-0V for qemu-devel@nongnu.org; Wed, 06 Aug 2014 16:44:18 -0400 Received: from e9.ny.us.ibm.com ([32.97.182.139]:39980) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XF83u-0005jt-Jl for qemu-devel@nongnu.org; Wed, 06 Aug 2014 16:43:54 -0400 Received: from /spool/local by e9.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 6 Aug 2014 16:43:54 -0400 Received: from d01dlp01.pok.ibm.com (9.56.250.166) by e9.ny.us.ibm.com (192.168.1.109) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Wed, 6 Aug 2014 16:43:51 -0400 Received: from b01cxnp23032.gho.pok.ibm.com (b01cxnp23032.gho.pok.ibm.com [9.57.198.27]) by d01dlp01.pok.ibm.com (Postfix) with ESMTP id 5613038C803B; Wed, 6 Aug 2014 16:43:51 -0400 (EDT) Received: from d01av01.pok.ibm.com (d01av01.pok.ibm.com [9.56.224.215]) by b01cxnp23032.gho.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id s76Khplq10224026; Wed, 6 Aug 2014 20:43:51 GMT Received: from d01av01.pok.ibm.com (localhost [127.0.0.1]) by d01av01.pok.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s76KhpI7026840; Wed, 6 Aug 2014 16:43:51 -0400 Received: from localhost ([9.80.101.111]) by d01av01.pok.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id s76KhoQ4026820; Wed, 6 Aug 2014 16:43:50 -0400 From: Michael Roth To: qemu-devel@nongnu.org Date: Wed, 6 Aug 2014 15:39:33 -0500 Message-Id: <1407357598-21541-84-git-send-email-mdroth@linux.vnet.ibm.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1407357598-21541-1-git-send-email-mdroth@linux.vnet.ibm.com> References: <1407357598-21541-1-git-send-email-mdroth@linux.vnet.ibm.com> X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 14080620-7182-0000-0000-00000024D4FB X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 32.97.182.139 Cc: qemu-stable@nongnu.org Subject: [Qemu-devel] [PATCH 083/108] coroutine-win32.c: Add noinline attribute to work around gcc bug X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: , List-Help: , List-Subscribe: , Errors-To: qemu-devel-bounces+patch=linaro.org@nongnu.org Sender: qemu-devel-bounces+patch=linaro.org@nongnu.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: mdroth@linux.vnet.ibm.com X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.220.177 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 From: Peter Maydell A gcc codegen bug in x86_64-w64-mingw32-gcc (GCC) 4.6.3 means that non-debug builds of QEMU for Windows tend to assert when using coroutines. Work around this by marking qemu_coroutine_switch as noinline. If we allow gcc to inline qemu_coroutine_switch into coroutine_trampoline, then it hoists the code to get the address of the TLS variable "current" out of the while() loop. This is an invalid transformation because the SwitchToFiber() call may be called when running thread A but return in thread B, and so we might be in a different thread context each time round the loop. This can happen quite often. Typically. a coroutine is started when a VCPU thread does bdrv_aio_readv: VCPU thread main VCPU thread coroutine I/O coroutine bdrv_aio_readv -----> start I/O operation thread_pool_submit_co <------------ yields back to emulation Then I/O finishes and the thread-pool.c event notifier triggers in the I/O thread. event_notifier_ready calls thread_pool_co_cb, and the I/O coroutine now restarts *in another thread*: iothread main iothread coroutine I/O coroutine (formerly in VCPU thread) event_notifier_ready thread_pool_co_cb -----> current = I/O coroutine; call AIO callback But on Win32, because of the bug, the "current" being set here the current coroutine of the VCPU thread, not the iothread. noinline is a good-enough workaround, and quite unlikely to break in the future. (Thanks to Paolo Bonzini for assistance in diagnosing the problem and providing the detailed example/ascii art quoted above.) Signed-off-by: Peter Maydell Message-id: 1403535303-14939-1-git-send-email-peter.maydell@linaro.org Reviewed-by: Paolo Bonzini Reviewed-by: Richard Henderson (cherry picked from commit ff4873cb8c81db89668d8b56e19e57b852edb5f5) Signed-off-by: Michael Roth --- coroutine-win32.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/coroutine-win32.c b/coroutine-win32.c index edc1f72..17ace37 100644 --- a/coroutine-win32.c +++ b/coroutine-win32.c @@ -36,8 +36,17 @@ typedef struct static __thread CoroutineWin32 leader; static __thread Coroutine *current; -CoroutineAction qemu_coroutine_switch(Coroutine *from_, Coroutine *to_, - CoroutineAction action) +/* This function is marked noinline to prevent GCC from inlining it + * into coroutine_trampoline(). If we allow it to do that then it + * hoists the code to get the address of the TLS variable "current" + * out of the while() loop. This is an invalid transformation because + * the SwitchToFiber() call may be called when running thread A but + * return in thread B, and so we might be in a different thread + * context each time round the loop. + */ +CoroutineAction __attribute__((noinline)) +qemu_coroutine_switch(Coroutine *from_, Coroutine *to_, + CoroutineAction action) { CoroutineWin32 *from = DO_UPCAST(CoroutineWin32, base, from_); CoroutineWin32 *to = DO_UPCAST(CoroutineWin32, base, to_);