From 3d69f821616a578aeed569b3b56e157922819602 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Fri, 17 Mar 2017 14:14:46 +0800 Subject: virtio-scsi: Make virtio_scsi_acquire/release public They will be used in virtio-scsi-dataplane.c as well, so move them to header. Signed-off-by: Fam Zheng Message-Id: <20170317061447.16243-2-famz@redhat.com> Signed-off-by: Paolo Bonzini --- include/hw/virtio/virtio-scsi.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'include') diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h index f536f77e68..8ae0acaa1f 100644 --- a/include/hw/virtio/virtio-scsi.h +++ b/include/hw/virtio/virtio-scsi.h @@ -121,6 +121,20 @@ typedef struct VirtIOSCSIReq { } req; } VirtIOSCSIReq; +static inline void virtio_scsi_acquire(VirtIOSCSI *s) +{ + if (s->ctx) { + aio_context_acquire(s->ctx); + } +} + +static inline void virtio_scsi_release(VirtIOSCSI *s) +{ + if (s->ctx) { + aio_context_release(s->ctx); + } +} + void virtio_scsi_common_realize(DeviceState *dev, Error **errp, VirtIOHandleOutput ctrl, VirtIOHandleOutput evt, VirtIOHandleOutput cmd); -- cgit v1.2.3-55-g7522 From 12f8def0e02232d7c6416ad9b66640f973c531d1 Mon Sep 17 00:00:00 2001 From: Andrey Shedel Date: Fri, 24 Mar 2017 15:01:41 -0700 Subject: win32: replace custom mutex and condition variable with native primitives The multithreaded TCG implementation exposed deadlocks in the win32 condition variables: as implemented, qemu_cond_broadcast waited on receivers, whereas the pthreads API it was intended to emulate does not. This was causing a deadlock because broadcast was called while holding the IO lock, as well as all possible waiters blocked on the same lock. This patch replaces all the custom synchronisation code for mutexes and condition variables with native Windows primitives (SRWlocks and condition variables) with the same semantics as their POSIX equivalents. To enable that, it requires a Windows Vista or newer host OS. Signed-off-by: Andrey Shedel [AB: edited commit message] Signed-off-by: Andrew Baumann Message-Id: <20170324220141.10104-1-Andrew.Baumann@microsoft.com> Signed-off-by: Paolo Bonzini --- include/qemu/thread-win32.h | 7 +-- util/qemu-thread-win32.c | 136 +++++--------------------------------------- 2 files changed, 17 insertions(+), 126 deletions(-) (limited to 'include') diff --git a/include/qemu/thread-win32.h b/include/qemu/thread-win32.h index 5fb6541ae9..4c4a261cf4 100644 --- a/include/qemu/thread-win32.h +++ b/include/qemu/thread-win32.h @@ -4,8 +4,7 @@ #include struct QemuMutex { - CRITICAL_SECTION lock; - LONG owner; + SRWLOCK lock; }; typedef struct QemuRecMutex QemuRecMutex; @@ -19,9 +18,7 @@ int qemu_rec_mutex_trylock(QemuRecMutex *mutex); void qemu_rec_mutex_unlock(QemuRecMutex *mutex); struct QemuCond { - LONG waiters, target; - HANDLE sema; - HANDLE continue_event; + CONDITION_VARIABLE var; }; struct QemuSemaphore { diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c index 29c3e4dd85..59befd5202 100644 --- a/util/qemu-thread-win32.c +++ b/util/qemu-thread-win32.c @@ -10,6 +10,11 @@ * See the COPYING file in the top-level directory. * */ + +#ifndef _WIN32_WINNT +#define _WIN32_WINNT 0x0600 +#endif + #include "qemu/osdep.h" #include "qemu-common.h" #include "qemu/thread.h" @@ -39,44 +44,30 @@ static void error_exit(int err, const char *msg) void qemu_mutex_init(QemuMutex *mutex) { - mutex->owner = 0; - InitializeCriticalSection(&mutex->lock); + InitializeSRWLock(&mutex->lock); } void qemu_mutex_destroy(QemuMutex *mutex) { - assert(mutex->owner == 0); - DeleteCriticalSection(&mutex->lock); + InitializeSRWLock(&mutex->lock); } void qemu_mutex_lock(QemuMutex *mutex) { - EnterCriticalSection(&mutex->lock); - - /* Win32 CRITICAL_SECTIONs are recursive. Assert that we're not - * using them as such. - */ - assert(mutex->owner == 0); - mutex->owner = GetCurrentThreadId(); + AcquireSRWLockExclusive(&mutex->lock); } int qemu_mutex_trylock(QemuMutex *mutex) { int owned; - owned = TryEnterCriticalSection(&mutex->lock); - if (owned) { - assert(mutex->owner == 0); - mutex->owner = GetCurrentThreadId(); - } + owned = TryAcquireSRWLockExclusive(&mutex->lock); return !owned; } void qemu_mutex_unlock(QemuMutex *mutex) { - assert(mutex->owner == GetCurrentThreadId()); - mutex->owner = 0; - LeaveCriticalSection(&mutex->lock); + ReleaseSRWLockExclusive(&mutex->lock); } void qemu_rec_mutex_init(QemuRecMutex *mutex) @@ -107,124 +98,27 @@ void qemu_rec_mutex_unlock(QemuRecMutex *mutex) void qemu_cond_init(QemuCond *cond) { memset(cond, 0, sizeof(*cond)); - - cond->sema = CreateSemaphore(NULL, 0, LONG_MAX, NULL); - if (!cond->sema) { - error_exit(GetLastError(), __func__); - } - cond->continue_event = CreateEvent(NULL, /* security */ - FALSE, /* auto-reset */ - FALSE, /* not signaled */ - NULL); /* name */ - if (!cond->continue_event) { - error_exit(GetLastError(), __func__); - } + InitializeConditionVariable(&cond->var); } void qemu_cond_destroy(QemuCond *cond) { - BOOL result; - result = CloseHandle(cond->continue_event); - if (!result) { - error_exit(GetLastError(), __func__); - } - cond->continue_event = 0; - result = CloseHandle(cond->sema); - if (!result) { - error_exit(GetLastError(), __func__); - } - cond->sema = 0; + InitializeConditionVariable(&cond->var); } void qemu_cond_signal(QemuCond *cond) { - DWORD result; - - /* - * Signal only when there are waiters. cond->waiters is - * incremented by pthread_cond_wait under the external lock, - * so we are safe about that. - */ - if (cond->waiters == 0) { - return; - } - - /* - * Waiting threads decrement it outside the external lock, but - * only if another thread is executing pthread_cond_broadcast and - * has the mutex. So, it also cannot be decremented concurrently - * with this particular access. - */ - cond->target = cond->waiters - 1; - result = SignalObjectAndWait(cond->sema, cond->continue_event, - INFINITE, FALSE); - if (result == WAIT_ABANDONED || result == WAIT_FAILED) { - error_exit(GetLastError(), __func__); - } + WakeConditionVariable(&cond->var); } void qemu_cond_broadcast(QemuCond *cond) { - BOOLEAN result; - /* - * As in pthread_cond_signal, access to cond->waiters and - * cond->target is locked via the external mutex. - */ - if (cond->waiters == 0) { - return; - } - - cond->target = 0; - result = ReleaseSemaphore(cond->sema, cond->waiters, NULL); - if (!result) { - error_exit(GetLastError(), __func__); - } - - /* - * At this point all waiters continue. Each one takes its - * slice of the semaphore. Now it's our turn to wait: Since - * the external mutex is held, no thread can leave cond_wait, - * yet. For this reason, we can be sure that no thread gets - * a chance to eat *more* than one slice. OTOH, it means - * that the last waiter must send us a wake-up. - */ - WaitForSingleObject(cond->continue_event, INFINITE); + WakeAllConditionVariable(&cond->var); } void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex) { - /* - * This access is protected under the mutex. - */ - cond->waiters++; - - /* - * Unlock external mutex and wait for signal. - * NOTE: we've held mutex locked long enough to increment - * waiters count above, so there's no problem with - * leaving mutex unlocked before we wait on semaphore. - */ - qemu_mutex_unlock(mutex); - WaitForSingleObject(cond->sema, INFINITE); - - /* Now waiters must rendez-vous with the signaling thread and - * let it continue. For cond_broadcast this has heavy contention - * and triggers thundering herd. So goes life. - * - * Decrease waiters count. The mutex is not taken, so we have - * to do this atomically. - * - * All waiters contend for the mutex at the end of this function - * until the signaling thread relinquishes it. To ensure - * each waiter consumes exactly one slice of the semaphore, - * the signaling thread stops until it is told by the last - * waiter that it can go on. - */ - if (InterlockedDecrement(&cond->waiters) == cond->target) { - SetEvent(cond->continue_event); - } - - qemu_mutex_lock(mutex); + SleepConditionVariableSRW(&cond->var, &mutex->lock, INFINITE, 0); } void qemu_sem_init(QemuSemaphore *sem, int init) -- cgit v1.2.3-55-g7522 From 5354edd286ea6e956b81da66c9dcc5e47028198e Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 22 Mar 2017 13:12:18 +0100 Subject: Revert "apic: save apic_delivered flag" This reverts commit 07bfa354772f2de67008dc66c201b627acff0106. The global variable is only read as part of a apic_reset_irq_delivered(); qemu_irq_raise(s->irq); if (!apic_get_irq_delivered()) { sequence, so the value never matters at migration time. Reported-by: Dr. David Alan Gilbert Cc: Pavel Dovgalyuk Signed-off-by: Paolo Bonzini --- hw/intc/apic_common.c | 33 --------------------------------- include/hw/i386/apic_internal.h | 2 -- 2 files changed, 35 deletions(-) (limited to 'include') diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c index 7a6e771ed1..c3829e31b5 100644 --- a/hw/intc/apic_common.c +++ b/hw/intc/apic_common.c @@ -387,25 +387,6 @@ static bool apic_common_sipi_needed(void *opaque) return s->wait_for_sipi != 0; } -static bool apic_irq_delivered_needed(void *opaque) -{ - APICCommonState *s = APIC_COMMON(opaque); - return s->cpu == X86_CPU(first_cpu) && apic_irq_delivered != 0; -} - -static void apic_irq_delivered_pre_save(void *opaque) -{ - APICCommonState *s = APIC_COMMON(opaque); - s->apic_irq_delivered = apic_irq_delivered; -} - -static int apic_irq_delivered_post_load(void *opaque, int version_id) -{ - APICCommonState *s = APIC_COMMON(opaque); - apic_irq_delivered = s->apic_irq_delivered; - return 0; -} - static const VMStateDescription vmstate_apic_common_sipi = { .name = "apic_sipi", .version_id = 1, @@ -418,19 +399,6 @@ static const VMStateDescription vmstate_apic_common_sipi = { } }; -static const VMStateDescription vmstate_apic_irq_delivered = { - .name = "apic_irq_delivered", - .version_id = 1, - .minimum_version_id = 1, - .needed = apic_irq_delivered_needed, - .pre_save = apic_irq_delivered_pre_save, - .post_load = apic_irq_delivered_post_load, - .fields = (VMStateField[]) { - VMSTATE_INT32(apic_irq_delivered, APICCommonState), - VMSTATE_END_OF_LIST() - } -}; - static const VMStateDescription vmstate_apic_common = { .name = "apic", .version_id = 3, @@ -465,7 +433,6 @@ static const VMStateDescription vmstate_apic_common = { }, .subsections = (const VMStateDescription*[]) { &vmstate_apic_common_sipi, - &vmstate_apic_irq_delivered, NULL } }; diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h index 20ad28c95b..1209eb483a 100644 --- a/include/hw/i386/apic_internal.h +++ b/include/hw/i386/apic_internal.h @@ -189,8 +189,6 @@ struct APICCommonState { DeviceState *vapic; hwaddr vapic_paddr; /* note: persistence via kvmvapic */ bool legacy_instance_id; - - int apic_irq_delivered; /* for saving static variable */ }; typedef struct VAPICState { -- cgit v1.2.3-55-g7522