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 <pet...@redhat.com>
---
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..44fb430f5b 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);
+
+/**
+ * set_bql_locked:
+ *
+ * @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 256723558d..0bf677c4a2 100644
--- a/system/cpus.c
+++ b/system/cpus.c
@@ -517,6 +517,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)
@@ -557,14 +569,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
