[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH 083/108] coroutine-win32.c: Add noinline attribute t
From: |
Michael Roth |
Subject: |
[Qemu-devel] [PATCH 083/108] coroutine-win32.c: Add noinline attribute to work around gcc bug |
Date: |
Wed, 6 Aug 2014 15:39:33 -0500 |
From: Peter Maydell <address@hidden>
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 <address@hidden>
Message-id: address@hidden
Reviewed-by: Paolo Bonzini <address@hidden>
Reviewed-by: Richard Henderson <address@hidden>
(cherry picked from commit ff4873cb8c81db89668d8b56e19e57b852edb5f5)
Signed-off-by: Michael Roth <address@hidden>
---
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_);
--
1.9.1
- [Qemu-devel] [PATCH 069/108] qga: Fix handle fd leak in acquire_privilege(), (continued)
- [Qemu-devel] [PATCH 069/108] qga: Fix handle fd leak in acquire_privilege(), Michael Roth, 2014/08/06
- [Qemu-devel] [PATCH 071/108] arch_init: Be sure of only one exit entry with DPRINTF() for ram_load(), Michael Roth, 2014/08/06
- [Qemu-devel] [PATCH 072/108] migration: catch unknown flags in ram_load, Michael Roth, 2014/08/06
- [Qemu-devel] [PATCH 073/108] rdma: bug fixes, Michael Roth, 2014/08/06
- [Qemu-devel] [PATCH 075/108] qdev: reorganize error reporting in bus_set_realized, Michael Roth, 2014/08/06
- [Qemu-devel] [PATCH 076/108] qdev: recursively unrealize devices when unrealizing bus, Michael Roth, 2014/08/06
- [Qemu-devel] [PATCH 078/108] vhost: fix resource leak in error handling, Michael Roth, 2014/08/06
- [Qemu-devel] [PATCH 077/108] scsi-disk: fix bug in scsi_block_new_request() introduced by commit 137745c, Michael Roth, 2014/08/06
- [Qemu-devel] [PATCH 079/108] virtio-scsi: define dummy handle_output for vhost-scsi vqs, Michael Roth, 2014/08/06
- [Qemu-devel] [PATCH 080/108] usb: Fix usb-bt-dongle initialization., Michael Roth, 2014/08/06
- [Qemu-devel] [PATCH 083/108] coroutine-win32.c: Add noinline attribute to work around gcc bug,
Michael Roth <=
- [Qemu-devel] [PATCH 086/108] target-i386: Filter FEAT_7_0_EBX TCG features too, Michael Roth, 2014/08/06
- [Qemu-devel] [PATCH 082/108] q35: Use PC_Q35_COMPAT_1_4 on pc-q35-1.4 compat_props, Michael Roth, 2014/08/06
- [Qemu-devel] [PATCH 084/108] hw/xtensa/xtfpga: fix FLASH mapping to boot region for KC705, Michael Roth, 2014/08/06
- [Qemu-devel] [PATCH 085/108] target-i386: Make TCG feature filtering more readable, Michael Roth, 2014/08/06
- [Qemu-devel] [PATCH 088/108] virtio-serial: don't migrate the config space, Michael Roth, 2014/08/06
- [Qemu-devel] [PATCH 087/108] virtio-net: byteswap virtio-net header, Michael Roth, 2014/08/06
- [Qemu-devel] [PATCH 089/108] nbd: Don't export a block device with no medium., Michael Roth, 2014/08/06
- [Qemu-devel] [PATCH 074/108] hw: Consistently name Error ** objects errp, and not err, Michael Roth, 2014/08/06
- [Qemu-devel] [PATCH 090/108] nbd: Don't validate from and len in NBD_CMD_DISC., Michael Roth, 2014/08/06
- [Qemu-devel] [PATCH 094/108] pc: make isapc and pc-0.10 to pc-0.13 have 1.7.0 memory layout, Michael Roth, 2014/08/06