From patchwork Wed Dec 25 05:44:16 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Akihiko Odaki X-Patchwork-Id: 13920619 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id A7F7BE7718D for ; Wed, 25 Dec 2024 05:45:36 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1tQKCF-000737-It; Wed, 25 Dec 2024 00:44:39 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1tQKCE-00072s-4S for qemu-devel@nongnu.org; Wed, 25 Dec 2024 00:44:38 -0500 Received: from mail-pl1-x62a.google.com ([2607:f8b0:4864:20::62a]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1tQKCB-0008Mn-KB for qemu-devel@nongnu.org; Wed, 25 Dec 2024 00:44:37 -0500 Received: by mail-pl1-x62a.google.com with SMTP id d9443c01a7336-2164b1f05caso59523035ad.3 for ; Tue, 24 Dec 2024 21:44:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=daynix-com.20230601.gappssmtp.com; s=20230601; t=1735105474; x=1735710274; darn=nongnu.org; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:from:to:cc:subject:date:message-id :reply-to; bh=y0dJk2V+rE4PujnuLWGLjchvT5Ur3ihlFfD3AC8SUs8=; b=Ax2MAgmFAWuM9MzAZGn0cc2y5TAy8PcLbA4CZL4RdUbw3cICwxMOqhSaCBG65Hq3yq inA/VQjF35/Olt39rEUEs+p/EK3TTVmCg/UKLUSVWA9hD+7ivXYiOdP0tRWNM+0eiZCx yVW0DE2h3D81lCGC0B4kkmIDFpbzeyxM9BKuLddPXG0kN8YLmXZ2/Vy+NMYYtUx/Nfxz n2lRV00PC73vjwnx0Pi9sN/gO2LKeF+hKCqA7UnaP7lacRvQbzEhaU5HJL4+AZ2SjguK e0+EjmTqiLe0Wo5Y/2s270bQE9dW2WZoy0aWuE/ciXdFJH5tLdwYiq9UNEk4IJqZ0CCt DhJg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1735105474; x=1735710274; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=y0dJk2V+rE4PujnuLWGLjchvT5Ur3ihlFfD3AC8SUs8=; b=nLQoMP0PUcBi1GzIAHtOzTk/71YREHfI/6A0nc4fsVyWRH0yz73oFNsPzLHlp6iain F2Tb3JT3MxGGzLYVVOUXPkIT6tucPCNwYJdWkifDK8O5artwgzu5NCeSxaLvRg8it3SO T7yKlcgZ3p4mbf13V2O0tu+VpR1zQ731fTbjcQm+SSa5z0qp/VLbmmBmUvFPOd0qKdJy 9M796XRvS+8fzGFUaPAt9CwY99b24XF7DbqZBFXZxDstUGj3dymoDJgny8tiKOCwpGx1 mjJXDlu7IvAwW6Q8U5zU1Mi6mVYgE1h8f5hEZiDmpE1/fQylU4iEfaVguRYlvbW3NDxY kMpw== X-Forwarded-Encrypted: i=1; AJvYcCUIMHUh04DgESM4bBDrqWMkmx/p0Qz8vC7T4BkR3BmXdZy33GrY1PSrqOhK5XsksEyp3qIV/RNf+wIl@nongnu.org X-Gm-Message-State: AOJu0YzqV8m+x7wtPfzmzNdbNYx296CpByBYAXwtnK+UjaAcEkXuq4uX LWMpXUnn6E4NsIAhjQCYQgbkjSoNDF+zT9rdoUkxLVmgb8rxCpWA981+vimHaX4= X-Gm-Gg: ASbGncsV2UFgxaFBVe8qb7luLWw9m2JnUueU1X3viQekdDN0zDVr8QGbSeN6ch7oY+G vV2QNdljIf9PmKOUZS5hcPw1UeKeAtnrV64aepRJMhP6Bdsff3fYCiwfhEbhdn47mWfwYz/GmKT uAxUh8tKtNoGbrb/ghitKagRkwFewu4jRm+SUoDNNxl9PvVqr3upx7oeQgeeHtYu6RRHm6cHkdo KiV6bS59aAXSUPRPwaTiuVgCwFXpFAwtdYAdN6FhO/056HMUKwDg15ik7qg X-Google-Smtp-Source: AGHT+IG9LWbwE54hM1Et3LXyIJywSwZz3wR5TVBhCUKKhwcINmJKOb3eQpD3N9ByDWlzhgbzfjjYLA== X-Received: by 2002:a17:902:d582:b0:216:5e6e:68ca with SMTP id d9443c01a7336-219e6f26685mr311413835ad.49.1735105474184; Tue, 24 Dec 2024 21:44:34 -0800 (PST) Received: from localhost ([157.82.207.107]) by smtp.gmail.com with UTF8SMTPSA id d9443c01a7336-219dc962d5fsm98473145ad.38.2024.12.24.21.44.31 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 24 Dec 2024 21:44:33 -0800 (PST) From: Akihiko Odaki Date: Wed, 25 Dec 2024 14:44:16 +0900 Subject: [PATCH 4/8] qemu-thread: Use futex for QemuEvent on Windows MIME-Version: 1.0 Message-Id: <20241225-event-v1-4-a58c8d63eb70@daynix.com> References: <20241225-event-v1-0-a58c8d63eb70@daynix.com> In-Reply-To: <20241225-event-v1-0-a58c8d63eb70@daynix.com> To: Paolo Bonzini , Stefan Weil , Peter Xu , Fabiano Rosas , Hailiang Zhang Cc: Phil Dennis-Jordan , qemu-devel@nongnu.org, devel@daynix.com, Akihiko Odaki X-Mailer: b4 0.14-dev-fd6e3 Received-SPF: pass client-ip=2607:f8b0:4864:20::62a; envelope-from=akihiko.odaki@daynix.com; helo=mail-pl1-x62a.google.com X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Use the futex-based implementation of QemuEvent on Windows to remove code duplication and remove the overhead of event object construction and destruction. Signed-off-by: Akihiko Odaki --- include/qemu/thread-posix.h | 9 --- include/qemu/thread-win32.h | 6 -- include/qemu/thread.h | 11 +++- util/event.c | 144 ++++++++++++++++++++++++++++++++++++++++++++ util/qemu-thread-posix.c | 143 ------------------------------------------- util/qemu-thread-win32.c | 129 --------------------------------------- util/meson.build | 1 + 7 files changed, 155 insertions(+), 288 deletions(-) diff --git a/include/qemu/thread-posix.h b/include/qemu/thread-posix.h index 5f2f3d1386bc..758808b705e4 100644 --- a/include/qemu/thread-posix.h +++ b/include/qemu/thread-posix.h @@ -32,15 +32,6 @@ struct QemuSemaphore { unsigned int count; }; -struct QemuEvent { -#ifndef __linux__ - pthread_mutex_t lock; - pthread_cond_t cond; -#endif - unsigned value; - bool initialized; -}; - struct QemuThread { pthread_t thread; }; diff --git a/include/qemu/thread-win32.h b/include/qemu/thread-win32.h index d95af4498fc9..da9e7322990c 100644 --- a/include/qemu/thread-win32.h +++ b/include/qemu/thread-win32.h @@ -28,12 +28,6 @@ struct QemuSemaphore { bool initialized; }; -struct QemuEvent { - int value; - HANDLE event; - bool initialized; -}; - typedef struct QemuThreadData QemuThreadData; struct QemuThread { QemuThreadData *data; diff --git a/include/qemu/thread.h b/include/qemu/thread.h index 7eba27a70493..cab58a14404d 100644 --- a/include/qemu/thread.h +++ b/include/qemu/thread.h @@ -4,13 +4,22 @@ #include "qemu/processor.h" #include "qemu/atomic.h" #include "qemu/clang-tsa.h" +#include "qemu/futex.h" typedef struct QemuCond QemuCond; typedef struct QemuSemaphore QemuSemaphore; -typedef struct QemuEvent QemuEvent; typedef struct QemuLockCnt QemuLockCnt; typedef struct QemuThread QemuThread; +typedef struct QemuEvent { +#ifndef HAVE_FUTEX + pthread_mutex_t lock; + pthread_cond_t cond; +#endif + unsigned value; + bool initialized; +} QemuEvent; + #ifdef _WIN32 #include "qemu/thread-win32.h" #else diff --git a/util/event.c b/util/event.c new file mode 100644 index 000000000000..34ac7a08d0bf --- /dev/null +++ b/util/event.c @@ -0,0 +1,144 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include "qemu/osdep.h" +#include "qemu/thread.h" + +/* + * Valid transitions: + * - free->set, when setting the event + * - busy->set, when setting the event, followed by qemu_futex_wake_all + * - set->free, when resetting the event + * - free->busy, when waiting + * + * set->busy does not happen (it can be observed from the outside but + * it really is set->free->busy). + * + * busy->free provably cannot happen; to enforce it, the set->free transition + * is done with an OR, which becomes a no-op if the event has concurrently + * transitioned to free or busy. + */ + +#define EV_SET 0 +#define EV_FREE 1 +#define EV_BUSY -1 + +void qemu_event_init(QemuEvent *ev, bool init) +{ +#ifndef HAVE_FUTEX + pthread_mutex_init(&ev->lock, NULL); + pthread_cond_init(&ev->cond, NULL); +#endif + + ev->value = (init ? EV_SET : EV_FREE); + ev->initialized = true; +} + +void qemu_event_destroy(QemuEvent *ev) +{ + assert(ev->initialized); + ev->initialized = false; +#ifndef HAVE_FUTEX + pthread_mutex_destroy(&ev->lock); + pthread_cond_destroy(&ev->cond); +#endif +} + +void qemu_event_set(QemuEvent *ev) +{ + assert(ev->initialized); + +#ifdef HAVE_FUTEX + /* + * Pairs with both qemu_event_reset() and qemu_event_wait(). + * + * qemu_event_set has release semantics, but because it *loads* + * ev->value we need a full memory barrier here. + */ + smp_mb(); + if (qatomic_read(&ev->value) != EV_SET) { + int old = qatomic_xchg(&ev->value, EV_SET); + + /* Pairs with memory barrier in kernel futex_wait system call. */ + smp_mb__after_rmw(); + if (old == EV_BUSY) { + /* There were waiters, wake them up. */ + qemu_futex_wake_all(ev); + } + } +#else + pthread_mutex_lock(&ev->lock); + qatomic_set(&ev->value, EV_SET); + pthread_cond_broadcast(&ev->cond); + pthread_mutex_unlock(&ev->lock); +#endif +} + +void qemu_event_reset(QemuEvent *ev) +{ + assert(ev->initialized); + + /* + * If there was a concurrent reset (or even reset+wait), + * do nothing. Otherwise change EV_SET->EV_FREE. + */ + qatomic_or(&ev->value, EV_FREE); + + /* + * Order reset before checking the condition in the caller. + * Pairs with the first memory barrier in qemu_event_set(). + */ + smp_mb__after_rmw(); +} + +void qemu_event_wait(QemuEvent *ev) +{ + assert(ev->initialized); + +#ifdef HAVE_FUTEX + while (true) { + /* + * qemu_event_wait must synchronize with qemu_event_set even if it does + * not go down the slow path, so this load-acquire is needed that + * synchronizes with the first memory barrier in qemu_event_set(). + * + * If we do go down the slow path, there is no requirement at all: we + * might miss a qemu_event_set() here but ultimately the memory barrier + * in qemu_futex_wait() will ensure the check is done correctly. + */ + unsigned value = qatomic_load_acquire(&ev->value); + if (value == EV_SET) { + break; + } + + if (value == EV_FREE) { + /* + * Leave the event reset and tell qemu_event_set that there are + * waiters. No need to retry, because there cannot be a concurrent + * busy->free transition. After the CAS, the event will be either + * set or busy. + * + * This cmpxchg doesn't have particular ordering requirements if it + * succeeds (moving the store earlier can only cause + * qemu_event_set() to issue _more_ wakeups), the failing case needs + * acquire semantics like the load above. + */ + if (qatomic_cmpxchg(&ev->value, EV_FREE, EV_BUSY) == EV_SET) { + break; + } + } + + /* + * This is the final check for a concurrent set, so it does need + * a smp_mb() pairing with the second barrier of qemu_event_set(). + * The barrier is inside the FUTEX_WAIT system call. + */ + qemu_futex_wait(ev, EV_BUSY); + } +#else + pthread_mutex_lock(&ev->lock); + if (qatomic_read(&ev->value) != EV_SET) { + pthread_cond_wait(&ev->cond, &ev->lock); + } + pthread_mutex_unlock(&ev->lock); +#endif +} diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c index 805cac444f15..ba725444ba63 100644 --- a/util/qemu-thread-posix.c +++ b/util/qemu-thread-posix.c @@ -317,149 +317,6 @@ void qemu_sem_wait(QemuSemaphore *sem) qemu_mutex_unlock(&sem->mutex); } -#ifdef __linux__ -#include "qemu/futex.h" -#endif - -/* Valid transitions: - * - free->set, when setting the event - * - busy->set, when setting the event, followed by qemu_futex_wake_all - * - set->free, when resetting the event - * - free->busy, when waiting - * - * set->busy does not happen (it can be observed from the outside but - * it really is set->free->busy). - * - * busy->free provably cannot happen; to enforce it, the set->free transition - * is done with an OR, which becomes a no-op if the event has concurrently - * transitioned to free or busy. - */ - -#define EV_SET 0 -#define EV_FREE 1 -#define EV_BUSY -1 - -void qemu_event_init(QemuEvent *ev, bool init) -{ -#ifndef CONFIG_LINUX - pthread_mutex_init(&ev->lock, NULL); - pthread_cond_init(&ev->cond, NULL); -#endif - - ev->value = (init ? EV_SET : EV_FREE); - ev->initialized = true; -} - -void qemu_event_destroy(QemuEvent *ev) -{ - assert(ev->initialized); - ev->initialized = false; -#ifndef CONFIG_LINUX - pthread_mutex_destroy(&ev->lock); - pthread_cond_destroy(&ev->cond); -#endif -} - -void qemu_event_set(QemuEvent *ev) -{ - assert(ev->initialized); - -#ifdef CONFIG_LINUX - /* - * Pairs with both qemu_event_reset() and qemu_event_wait(). - * - * qemu_event_set has release semantics, but because it *loads* - * ev->value we need a full memory barrier here. - */ - smp_mb(); - if (qatomic_read(&ev->value) != EV_SET) { - int old = qatomic_xchg(&ev->value, EV_SET); - - /* Pairs with memory barrier in kernel futex_wait system call. */ - smp_mb__after_rmw(); - if (old == EV_BUSY) { - /* There were waiters, wake them up. */ - qemu_futex_wake_all(ev); - } - } -#else - pthread_mutex_lock(&ev->lock); - qatomic_set(&ev->value, EV_SET); - pthread_cond_broadcast(&ev->cond); - pthread_mutex_unlock(&ev->lock); -#endif -} - -void qemu_event_reset(QemuEvent *ev) -{ - assert(ev->initialized); - - /* - * If there was a concurrent reset (or even reset+wait), - * do nothing. Otherwise change EV_SET->EV_FREE. - */ - qatomic_or(&ev->value, EV_FREE); - - /* - * Order reset before checking the condition in the caller. - * Pairs with the first memory barrier in qemu_event_set(). - */ - smp_mb__after_rmw(); -} - -void qemu_event_wait(QemuEvent *ev) -{ - assert(ev->initialized); - -#ifdef CONFIG_LINUX - while (true) { - /* - * qemu_event_wait must synchronize with qemu_event_set even if it does - * not go down the slow path, so this load-acquire is needed that - * synchronizes with the first memory barrier in qemu_event_set(). - * - * If we do go down the slow path, there is no requirement at all: we - * might miss a qemu_event_set() here but ultimately the memory barrier - * in qemu_futex_wait() will ensure the check is done correctly. - */ - unsigned value = qatomic_load_acquire(&ev->value); - if (value == EV_SET) { - break; - } - - if (value == EV_FREE) { - /* - * Leave the event reset and tell qemu_event_set that there are - * waiters. No need to retry, because there cannot be a concurrent - * busy->free transition. After the CAS, the event will be either - * set or busy. - * - * This cmpxchg doesn't have particular ordering requirements if it - * succeeds (moving the store earlier can only cause qemu_event_set() - * to issue _more_ wakeups), the failing case needs acquire semantics - * like the load above. - */ - if (qatomic_cmpxchg(&ev->value, EV_FREE, EV_BUSY) == EV_SET) { - break; - } - } - - /* - * This is the final check for a concurrent set, so it does need - * a smp_mb() pairing with the second barrier of qemu_event_set(). - * The barrier is inside the FUTEX_WAIT system call. - */ - qemu_futex_wait(ev, EV_BUSY); - } -#else - pthread_mutex_lock(&ev->lock); - if (qatomic_read(&ev->value) != EV_SET) { - pthread_cond_wait(&ev->cond, &ev->lock); - } - pthread_mutex_unlock(&ev->lock); -#endif -} - static __thread NotifierList thread_exit; /* diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c index a7fe3cc345f0..ca2e0b512e26 100644 --- a/util/qemu-thread-win32.c +++ b/util/qemu-thread-win32.c @@ -231,135 +231,6 @@ void qemu_sem_wait(QemuSemaphore *sem) } } -/* Wrap a Win32 manual-reset event with a fast userspace path. The idea - * is to reset the Win32 event lazily, as part of a test-reset-test-wait - * sequence. Such a sequence is, indeed, how QemuEvents are used by - * RCU and other subsystems! - * - * Valid transitions: - * - free->set, when setting the event - * - busy->set, when setting the event, followed by SetEvent - * - set->free, when resetting the event - * - free->busy, when waiting - * - * set->busy does not happen (it can be observed from the outside but - * it really is set->free->busy). - * - * busy->free provably cannot happen; to enforce it, the set->free transition - * is done with an OR, which becomes a no-op if the event has concurrently - * transitioned to free or busy (and is faster than cmpxchg). - */ - -#define EV_SET 0 -#define EV_FREE 1 -#define EV_BUSY -1 - -void qemu_event_init(QemuEvent *ev, bool init) -{ - /* Manual reset. */ - ev->event = CreateEvent(NULL, TRUE, TRUE, NULL); - ev->value = (init ? EV_SET : EV_FREE); - ev->initialized = true; -} - -void qemu_event_destroy(QemuEvent *ev) -{ - assert(ev->initialized); - ev->initialized = false; - CloseHandle(ev->event); -} - -void qemu_event_set(QemuEvent *ev) -{ - assert(ev->initialized); - - /* - * Pairs with both qemu_event_reset() and qemu_event_wait(). - * - * qemu_event_set has release semantics, but because it *loads* - * ev->value we need a full memory barrier here. - */ - smp_mb(); - if (qatomic_read(&ev->value) != EV_SET) { - int old = qatomic_xchg(&ev->value, EV_SET); - - /* Pairs with memory barrier after ResetEvent. */ - smp_mb__after_rmw(); - if (old == EV_BUSY) { - /* There were waiters, wake them up. */ - SetEvent(ev->event); - } - } -} - -void qemu_event_reset(QemuEvent *ev) -{ - assert(ev->initialized); - - /* - * If there was a concurrent reset (or even reset+wait), - * do nothing. Otherwise change EV_SET->EV_FREE. - */ - qatomic_or(&ev->value, EV_FREE); - - /* - * Order reset before checking the condition in the caller. - * Pairs with the first memory barrier in qemu_event_set(). - */ - smp_mb__after_rmw(); -} - -void qemu_event_wait(QemuEvent *ev) -{ - unsigned value; - - assert(ev->initialized); - - /* - * qemu_event_wait must synchronize with qemu_event_set even if it does - * not go down the slow path, so this load-acquire is needed that - * synchronizes with the first memory barrier in qemu_event_set(). - * - * If we do go down the slow path, there is no requirement at all: we - * might miss a qemu_event_set() here but ultimately the memory barrier in - * qemu_futex_wait() will ensure the check is done correctly. - */ - value = qatomic_load_acquire(&ev->value); - if (value != EV_SET) { - if (value == EV_FREE) { - /* - * Here the underlying kernel event is reset, but qemu_event_set is - * not yet going to call SetEvent. However, there will be another - * check for EV_SET below when setting EV_BUSY. At that point it - * is safe to call WaitForSingleObject. - */ - ResetEvent(ev->event); - - /* - * It is not clear whether ResetEvent provides this barrier; kernel - * APIs (KeResetEvent/KeClearEvent) do not. Better safe than sorry! - */ - smp_mb(); - - /* - * Leave the event reset and tell qemu_event_set that there are - * waiters. No need to retry, because there cannot be a concurrent - * busy->free transition. After the CAS, the event will be either - * set or busy. - */ - if (qatomic_cmpxchg(&ev->value, EV_FREE, EV_BUSY) == EV_SET) { - return; - } - } - - /* - * ev->value is now EV_BUSY. Since we didn't observe EV_SET, - * qemu_event_set() must observe EV_BUSY and call SetEvent(). - */ - WaitForSingleObject(ev->event, INFINITE); - } -} - struct QemuThreadData { /* Passed to win32_start_routine. */ void *(*start_routine)(void *); diff --git a/util/meson.build b/util/meson.build index da5b49f1ea82..6d23df3d25d2 100644 --- a/util/meson.build +++ b/util/meson.build @@ -33,6 +33,7 @@ if glib_has_gslice endif util_ss.add(files('defer-call.c')) util_ss.add(files('envlist.c', 'path.c', 'module.c')) +util_ss.add(files('event.c')) util_ss.add(files('host-utils.c')) util_ss.add(files('bitmap.c', 'bitops.c')) util_ss.add(files('fifo8.c'))