From: Peter Xu <peterx@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Daniel P . Berrangé" <berrange@redhat.com>,
"Philippe Mathieu-Daudé" <philmd@linaro.org>,
"Stefan Hajnoczi" <stefanha@redhat.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Juraj Marcin" <jmarcin@redhat.com>,
peterx@redhat.com, "Peter Maydell" <peter.maydell@linaro.org>,
"Fabiano Rosas" <farosas@suse.de>
Subject: [PATCH] bql: Fix bql_locked status with condvar APIs
Date: Thu, 4 Sep 2025 18:31:58 -0400 [thread overview]
Message-ID: <20250904223158.1276992-1-peterx@redhat.com> (raw)
QEMU has a per-thread "bql_locked" variable stored in TLS section, showing
whether the current thread is holding the BQL lock.
It's a pretty handy variable. Function-wise, QEMU have codes trying to
conditionally take bql, relying on the var reflecting the locking status
(e.g. BQL_LOCK_GUARD), or in a GDB debugging session, we could also look at
the variable (in reality, co_tls_bql_locked), to see which thread is
currently holding the bql.
When using that as a debugging facility, sometimes we can observe multiple
threads holding bql at the same time. It's because QEMU's condvar APIs
bypassed the bql_*() API, hence they do not update bql_locked even if they
have released the mutex while waiting.
It can cause confusion if one does "thread apply all p co_tls_bql_locked"
and see multiple threads reporting true.
Fix this by moving the bql status updates into the mutex debug hooks. Now
the variable should always reflect the reality.
Signed-off-by: Peter Xu <peterx@redhat.com>
---
rfc->v1
- Fix comment [Stefan]
---
include/qemu/main-loop.h | 18 ++++++++++++++++++
util/qemu-thread-common.h | 7 +++++++
stubs/iothread-lock.c | 9 +++++++++
system/cpus.c | 14 ++++++++++++--
4 files changed, 46 insertions(+), 2 deletions(-)
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index 4e2436b196..0d55c636b2 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -270,6 +270,24 @@ void rust_bql_mock_lock(void);
*/
bool bql_locked(void);
+/**
+ * mutex_is_bql:
+ *
+ * @mutex: the mutex pointer
+ *
+ * Returns whether the mutex is the BQL.
+ */
+bool mutex_is_bql(QemuMutex *mutex);
+
+/**
+ * bql_update_status:
+ *
+ * @locked: update status on whether the BQL is locked
+ *
+ * NOTE: this should normally only be invoked when the status changed.
+ */
+void bql_update_status(bool locked);
+
/**
* bql_block: Allow/deny releasing the BQL
*
diff --git a/util/qemu-thread-common.h b/util/qemu-thread-common.h
index 2af6b12085..09331843ba 100644
--- a/util/qemu-thread-common.h
+++ b/util/qemu-thread-common.h
@@ -14,6 +14,7 @@
#define QEMU_THREAD_COMMON_H
#include "qemu/thread.h"
+#include "qemu/main-loop.h"
#include "trace.h"
static inline void qemu_mutex_post_init(QemuMutex *mutex)
@@ -39,6 +40,9 @@ static inline void qemu_mutex_post_lock(QemuMutex *mutex,
mutex->line = line;
#endif
trace_qemu_mutex_locked(mutex, file, line);
+ if (mutex_is_bql(mutex)) {
+ bql_update_status(true);
+ }
}
static inline void qemu_mutex_pre_unlock(QemuMutex *mutex,
@@ -49,6 +53,9 @@ static inline void qemu_mutex_pre_unlock(QemuMutex *mutex,
mutex->line = 0;
#endif
trace_qemu_mutex_unlock(mutex, file, line);
+ if (mutex_is_bql(mutex)) {
+ bql_update_status(false);
+ }
}
#endif
diff --git a/stubs/iothread-lock.c b/stubs/iothread-lock.c
index 6050c081f5..c89c9c7228 100644
--- a/stubs/iothread-lock.c
+++ b/stubs/iothread-lock.c
@@ -34,3 +34,12 @@ void bql_block_unlock(bool increase)
assert((new_value > bql_unlock_blocked) == increase);
bql_unlock_blocked = new_value;
}
+
+bool mutex_is_bql(QemuMutex *mutex)
+{
+ return false;
+}
+
+void bql_update_status(bool locked)
+{
+}
diff --git a/system/cpus.c b/system/cpus.c
index 437848b5eb..9c066e1c08 100644
--- a/system/cpus.c
+++ b/system/cpus.c
@@ -524,6 +524,18 @@ bool qemu_in_vcpu_thread(void)
QEMU_DEFINE_STATIC_CO_TLS(bool, bql_locked)
+bool mutex_is_bql(QemuMutex *mutex)
+{
+ return mutex == &bql;
+}
+
+void bql_update_status(bool locked)
+{
+ /* This function should only be used when an update happened.. */
+ assert(bql_locked() != locked);
+ set_bql_locked(locked);
+}
+
static uint32_t bql_unlock_blocked;
void bql_block_unlock(bool increase)
@@ -564,14 +576,12 @@ void bql_lock_impl(const char *file, int line)
g_assert(!bql_locked());
bql_lock_fn(&bql, file, line);
- set_bql_locked(true);
}
void bql_unlock(void)
{
g_assert(bql_locked());
g_assert(!bql_unlock_blocked);
- set_bql_locked(false);
qemu_mutex_unlock(&bql);
}
--
2.50.1
next reply other threads:[~2025-09-04 22:34 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-04 22:31 Peter Xu [this message]
2025-09-08 18:24 ` [PATCH] bql: Fix bql_locked status with condvar APIs Stefan Hajnoczi
2025-09-22 7:19 ` Philippe Mathieu-Daudé
2025-10-09 9:21 ` Philippe Mathieu-Daudé
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250904223158.1276992-1-peterx@redhat.com \
--to=peterx@redhat.com \
--cc=berrange@redhat.com \
--cc=farosas@suse.de \
--cc=jmarcin@redhat.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=philmd@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.