qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé" <philmd@linaro.org>
To: qemu-devel@nongnu.org
Subject: [PULL 4/9] bql: Fix bql_locked status with condvar APIs
Date: Fri, 31 Oct 2025 22:15:13 +0100	[thread overview]
Message-ID: <20251031211518.38503-5-philmd@linaro.org> (raw)
In-Reply-To: <20251031211518.38503-1-philmd@linaro.org>

From: Peter Xu <peterx@redhat.com>

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>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20250904223158.1276992-1-peterx@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 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 4e2436b1968..0d55c636b21 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 2af6b120853..09331843ba9 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 6050c081f53..c89c9c7228f 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 74f5a5bd4e2..ef2d2f241fa 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.51.0



  parent reply	other threads:[~2025-10-31 21:18 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-31 21:15 [PULL 0/9] Accelerators & CPU patches for Halloween 2025 Philippe Mathieu-Daudé
2025-10-31 21:15 ` [PULL 1/9] cpus: Access CPUState::thread_kicked atomically Philippe Mathieu-Daudé
2025-10-31 21:15 ` [PULL 2/9] accel/hvf: Make async_safe_run_on_cpu() safe Philippe Mathieu-Daudé
2025-10-31 21:15 ` [PULL 3/9] accel/tcg: Use cpu_is_stopped() helper to access CPUState::stopped Philippe Mathieu-Daudé
2025-10-31 21:15 ` Philippe Mathieu-Daudé [this message]
2025-10-31 21:15 ` [PULL 5/9] exec/cpu: Declare cpu_memory_rw_debug() in 'hw/core/cpu.h' and document Philippe Mathieu-Daudé
2025-10-31 21:15 ` [PULL 6/9] timers: properly prefix init_clocks() Philippe Mathieu-Daudé
2025-10-31 21:15 ` [PULL 7/9] util/hexdump: fix QEMU_HEXDUMP_LINE_WIDTH logic Philippe Mathieu-Daudé
2025-10-31 21:15 ` [PULL 8/9] tests/unit: add unit test for qemu_hexdump() Philippe Mathieu-Daudé
2025-10-31 21:15 ` [PULL 9/9] rx: cpu: fix interrupts check in rx_cpu_do_interrupt() Philippe Mathieu-Daudé
2025-11-01 11:13 ` [PULL 0/9] Accelerators & CPU patches for Halloween 2025 Richard Henderson

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=20251031211518.38503-5-philmd@linaro.org \
    --to=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).