From patchwork Mon Jun 23 14:55:03 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Maydell X-Patchwork-Id: 32372 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-qc0-f198.google.com (mail-qc0-f198.google.com [209.85.216.198]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id B257520540 for ; Mon, 23 Jun 2014 14:55:08 +0000 (UTC) Received: by mail-qc0-f198.google.com with SMTP id m20sf22303599qcx.1 for ; Mon, 23 Jun 2014 07:55:08 -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:cc:subject :date:message-id:x-original-sender:x-original-authentication-results :precedence:mailing-list:list-id:list-post:list-help:list-archive :list-unsubscribe; bh=8eSOH7P99vcFxZZMN94RMibxYLb4NNjcYnhUxazAZ1w=; b=Ov0M/Lhhpid1t18KiF5BC7Bl8Eikoy+yM5mI8fvg1xxvU06hADoeU471M5WqscRVWK 8bgMjX7uoctXrnUUELS3mb+v+eSt2msbA42D4tOBH1lRKMwYw0wrgskL4v4pkx8M3+bY QbYpYpX6QgmAdbhlxBoHxKJS520RHhyd1rmHbYQCTu/dYKtWmEe6xfrCQk5shu5Lfld5 uKpAgwRvlkekNMK+VhiJWCVRe88bzYKe48zVtJ18NhPfVjT1pA/LWK/b7QMv6QjsDGuT 1ve3yX995xoGUmuM7vFsh90dgDqffhEhgECBDDILDx9K9OCkseb91nnD1X4qWVdZCHCy n0Vg== X-Gm-Message-State: ALoCoQkUtJ5SozKX0jD3/7JJsCV0gAhMJNO6HjBuEboIfVBpXCiykr+hawSHlt8f2FltkU3jmLG/ X-Received: by 10.236.101.148 with SMTP id b20mr8798643yhg.46.1403535308545; Mon, 23 Jun 2014 07:55:08 -0700 (PDT) MIME-Version: 1.0 X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.37.211 with SMTP id r77ls1963181qgr.10.gmail; Mon, 23 Jun 2014 07:55:08 -0700 (PDT) X-Received: by 10.52.248.209 with SMTP id yo17mr1120118vdc.60.1403535308479; Mon, 23 Jun 2014 07:55:08 -0700 (PDT) Received: from mail-ve0-f182.google.com (mail-ve0-f182.google.com [209.85.128.182]) by mx.google.com with ESMTPS id xt7si9138891veb.104.2014.06.23.07.55.08 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 23 Jun 2014 07:55:08 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.128.182 as permitted sender) client-ip=209.85.128.182; Received: by mail-ve0-f182.google.com with SMTP id oy12so6205959veb.41 for ; Mon, 23 Jun 2014 07:55:08 -0700 (PDT) X-Received: by 10.52.52.168 with SMTP id u8mr16642594vdo.25.1403535308209; Mon, 23 Jun 2014 07:55:08 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patches@linaro.org Received: by 10.221.37.5 with SMTP id tc5csp136571vcb; Mon, 23 Jun 2014 07:55:07 -0700 (PDT) X-Received: by 10.180.126.8 with SMTP id mu8mr27195629wib.10.1403535307235; Mon, 23 Jun 2014 07:55:07 -0700 (PDT) Received: from mnementh.archaic.org.uk (mnementh.archaic.org.uk. [2001:8b0:1d0::1]) by mx.google.com with ESMTPS id j2si4836423wiz.79.2014.06.23.07.55.06 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Mon, 23 Jun 2014 07:55:06 -0700 (PDT) Received-SPF: none (google.com: pm215@archaic.org.uk does not designate permitted sender hosts) client-ip=2001:8b0:1d0::1; Received: from pm215 by mnementh.archaic.org.uk with local (Exim 4.80) (envelope-from ) id 1Wz5eB-0003tQ-38; Mon, 23 Jun 2014 15:55:03 +0100 From: Peter Maydell To: qemu-devel@nongnu.org Cc: patches@linaro.org, Paolo Bonzini , Stefan Weil Subject: [PATCH] coroutine-win32.c: Add noinline attribute to work around gcc bug Date: Mon, 23 Jun 2014 15:55:03 +0100 Message-Id: <1403535303-14939-1-git-send-email-peter.maydell@linaro.org> X-Mailer: git-send-email 1.7.10.4 X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: peter.maydell@linaro.org 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.182 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Precedence: list Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org List-ID: X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , 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 Reviewed-by: Paolo Bonzini Reviewed-by: Richard Henderson --- 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_);