From: Stefan Hajnoczi <stefanha@redhat.com>
To: Peter Xu <peterx@redhat.com>
Cc: qemu-devel@nongnu.org,
"Daniel P . Berrangé" <berrange@redhat.com>,
"Philippe Mathieu-Daudé" <philmd@linaro.org>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Juraj Marcin" <jmarcin@redhat.com>,
"Peter Maydell" <peter.maydell@linaro.org>,
"Fabiano Rosas" <farosas@suse.de>
Subject: Re: [PATCH] bql: Fix bql_locked status with condvar APIs
Date: Mon, 8 Sep 2025 14:24:53 -0400 [thread overview]
Message-ID: <20250908182453.GD167158@fedora> (raw)
In-Reply-To: <20250904223158.1276992-1-peterx@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1464 bytes --]
On Thu, Sep 04, 2025 at 06:31:58PM -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 <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(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
next prev parent reply other threads:[~2025-09-09 14:25 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-04 22:31 [PATCH] bql: Fix bql_locked status with condvar APIs Peter Xu
2025-09-08 18:24 ` Stefan Hajnoczi [this message]
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=20250908182453.GD167158@fedora \
--to=stefanha@redhat.com \
--cc=berrange@redhat.com \
--cc=farosas@suse.de \
--cc=jmarcin@redhat.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=peterx@redhat.com \
--cc=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 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.