On Wed, Aug 20, 2025 at 04:50:51PM -0400, Peter Xu wrote: > 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:
This does not match the actual function name (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 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
>
signature.asc
Description: PGP signature
