From patchwork Tue Jul 8 17:18:55 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Roth X-Patchwork-Id: 33255 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-qg0-f69.google.com (mail-qg0-f69.google.com [209.85.192.69]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 5B82C20969 for ; Wed, 9 Jul 2014 00:05:00 +0000 (UTC) Received: by mail-qg0-f69.google.com with SMTP id j107sf15553206qga.8 for ; Tue, 08 Jul 2014 17:04:59 -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=JqDWMztdvKJDQT/2P2maVWWa54+aZ3uXv0RHpRfgh0yGD5bS7XNpGJ3J4TuTZ5pCQo 60Q1wv0ZvVBe3ten9BBeeXN8BHLOxzDiAvs8Al77V3NagiWL0DLYt4ld5iUOqh895zjS FsZCHf3JKgjd8WiG5JF+zjsJtTSDq3wPsIEnvLqH9X+ufu7K+kLZ15MOmk0wmmSwdcaw IqAPbj0XS6YCdcpqkEM9JevcmceureF5yUChF/a67lTPu7e6BrW5iWWX7zfynHHP3m2/ hiqa1akfg1XNxdk0CoNTiIDc0rK31an4XxFGLtDZpvDt5br5Nc6AG4NutjfvCd/CB9zH DoUg== X-Gm-Message-State: ALoCoQmtlw1C71iPUDUHco+4Xk8pPHbFeMYC6HizQQPaN74eTqWOPuHtLv/QJmNQMZboMEFxJx37 X-Received: by 10.58.46.79 with SMTP id t15mr18246395vem.35.1404864299883; Tue, 08 Jul 2014 17:04:59 -0700 (PDT) MIME-Version: 1.0 X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.46.99 with SMTP id j90ls2446148qga.49.gmail; Tue, 08 Jul 2014 17:04:59 -0700 (PDT) X-Received: by 10.221.9.72 with SMTP id ov8mr36132811vcb.27.1404864299808; Tue, 08 Jul 2014 17:04:59 -0700 (PDT) Received: from mail-ve0-f179.google.com (mail-ve0-f179.google.com [209.85.128.179]) by mx.google.com with ESMTPS id xv6si74151vdc.77.2014.07.08.17.04.59 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 08 Jul 2014 17:04:59 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.128.179 as permitted sender) client-ip=209.85.128.179; Received: by mail-ve0-f179.google.com with SMTP id jz11so202769veb.24 for ; Tue, 08 Jul 2014 17:04:59 -0700 (PDT) X-Received: by 10.220.136.8 with SMTP id p8mr90155vct.42.1404864299687; Tue, 08 Jul 2014 17:04:59 -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 tc5csp819002vcb; Tue, 8 Jul 2014 17:04:59 -0700 (PDT) X-Received: by 10.224.130.201 with SMTP id u9mr64066153qas.98.1404864298804; Tue, 08 Jul 2014 17:04:58 -0700 (PDT) Received: from lists.gnu.org (lists.gnu.org. [2001:4830:134:3::11]) by mx.google.com with ESMTPS id n6si25642930qak.51.2014.07.08.17.04.58 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Tue, 08 Jul 2014 17:04:58 -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]:57500 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X4ZhA-0001CQ-ND for patch@linaro.org; Tue, 08 Jul 2014 14:00:48 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42667) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X4Z8G-0005hd-1Z for qemu-devel@nongnu.org; Tue, 08 Jul 2014 13:25:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X4Z7J-0001V5-P8 for qemu-devel@nongnu.org; Tue, 08 Jul 2014 13:24:43 -0400 Received: from e39.co.us.ibm.com ([32.97.110.160]:52326) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X4Z7J-0001Un-HM for qemu-devel@nongnu.org; Tue, 08 Jul 2014 13:23:45 -0400 Received: from /spool/local by e39.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 8 Jul 2014 11:23:45 -0600 Received: from d01dlp03.pok.ibm.com (9.56.250.168) by e39.co.us.ibm.com (192.168.1.139) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Tue, 8 Jul 2014 11:23:43 -0600 Received: from b01cxnp23034.gho.pok.ibm.com (b01cxnp23034.gho.pok.ibm.com [9.57.198.29]) by d01dlp03.pok.ibm.com (Postfix) with ESMTP id 32A78C90042; Tue, 8 Jul 2014 13:23:36 -0400 (EDT) Received: from d01av05.pok.ibm.com (d01av05.pok.ibm.com [9.56.224.195]) by b01cxnp23034.gho.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id s68HNYBC1376694; Tue, 8 Jul 2014 17:23:42 GMT Received: from d01av05.pok.ibm.com (localhost [127.0.0.1]) by d01av05.pok.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s68HN95W006950; Tue, 8 Jul 2014 13:23:09 -0400 Received: from localhost ([9.41.105.211]) by d01av05.pok.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id s68HN8Kj006532; Tue, 8 Jul 2014 13:23:09 -0400 From: Michael Roth To: qemu-devel@nongnu.org Date: Tue, 8 Jul 2014 12:18:55 -0500 Message-Id: <1404839947-1086-145-git-send-email-mdroth@linux.vnet.ibm.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1404839947-1086-1-git-send-email-mdroth@linux.vnet.ibm.com> References: <1404839947-1086-1-git-send-email-mdroth@linux.vnet.ibm.com> X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 14070817-9332-0000-0000-000001506A04 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 32.97.110.160 Cc: qemu-stable@nongnu.org Subject: [Qemu-devel] [PATCH 144/156] 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.128.179 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_);