[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/6] system/cpus: rename qemu_mutex_lock_iothread() to qemu_bql_lock()
- To: Stefan Hajnoczi <stefanha@xxxxxxxxxx>
- From: BALATON Zoltan <balaton@xxxxxxxxxx>
- Date: Thu, 30 Nov 2023 22:41:09 +0100 (CET)
- Cc: Peter Xu <peterx@xxxxxxxxxx>, qemu-devel@xxxxxxxxxx, Jean-Christophe Dubois <jcd@xxxxxxxxxxxxxxx>, Fabiano Rosas <farosas@xxxxxxx>, qemu-s390x@xxxxxxxxxx, Song Gao <gaosong@xxxxxxxxxxx>, Marcel Apfelbaum <marcel.apfelbaum@xxxxxxxxx>, Thomas Huth <thuth@xxxxxxxxxx>, Hyman Huang <yong.huang@xxxxxxxxxx>, Marcelo Tosatti <mtosatti@xxxxxxxxxx>, David Woodhouse <dwmw2@xxxxxxxxxxxxx>, Andrey Smirnov <andrew.smirnov@xxxxxxxxx>, Peter Maydell <peter.maydell@xxxxxxxxxx>, Kevin Wolf <kwolf@xxxxxxxxxx>, Ilya Leoshkevich <iii@xxxxxxxxxxxxx>, Artyom Tarasenko <atar4qemu@xxxxxxxxx>, Mark Cave-Ayland <mark.cave-ayland@xxxxxxxxxxxx>, Max Filippov <jcmvbkbc@xxxxxxxxx>, Alistair Francis <alistair.francis@xxxxxxx>, Paul Durrant <paul@xxxxxxx>, Jagannathan Raman <jag.raman@xxxxxxxxxx>, Juan Quintela <quintela@xxxxxxxxxx>, Daniel P. Berrangé <berrange@xxxxxxxxxx>, qemu-arm@xxxxxxxxxx, Jason Wang <jasowang@xxxxxxxxxx>, Gerd Hoffmann <kraxel@xxxxxxxxxx>, Hanna Reitz <hreitz@xxxxxxxxxx>, Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>, Daniel Henrique Barboza <danielhb413@xxxxxxxxx>, Elena Ufimtseva <elena.ufimtseva@xxxxxxxxxx>, Aurelien Jarno <aurelien@xxxxxxxxxxx>, Hailiang Zhang <zhanghailiang@xxxxxxxxxxx>, Roman Bolshakov <rbolshakov@xxxxxxx>, Huacai Chen <chenhuacai@xxxxxxxxxx>, Fam Zheng <fam@xxxxxxxxxx>, Eric Blake <eblake@xxxxxxxxxx>, Jiri Slaby <jslaby@xxxxxxx>, Alexander Graf <agraf@xxxxxxxxx>, Liu Zhiwei <zhiwei_liu@xxxxxxxxxxxxxxxxx>, Weiwei Li <liwei1518@xxxxxxxxx>, Eric Farman <farman@xxxxxxxxxxxxx>, Stafford Horne <shorne@xxxxxxxxx>, David Hildenbrand <david@xxxxxxxxxx>, Markus Armbruster <armbru@xxxxxxxxxx>, Reinoud Zandijk <reinoud@xxxxxxxxxx>, Palmer Dabbelt <palmer@xxxxxxxxxxx>, Cameron Esfahani <dirty@xxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx, Pavel Dovgalyuk <pavel.dovgaluk@xxxxxxxxx>, qemu-riscv@xxxxxxxxxx, Aleksandar Rikalo <aleksandar.rikalo@xxxxxxxxxx>, John Snow <jsnow@xxxxxxxxxx>, Sunil Muthuswamy <sunilmut@xxxxxxxxxxxxx>, Michael Roth <michael.roth@xxxxxxx>, David Gibson <david@xxxxxxxxxxxxxxxxxxxxx>, "Michael S. Tsirkin" <mst@xxxxxxxxxx>, Richard Henderson <richard.henderson@xxxxxxxxxx>, Bin Meng <bin.meng@xxxxxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, kvm@xxxxxxxxxxxxxxx, qemu-block@xxxxxxxxxx, Halil Pasic <pasic@xxxxxxxxxxxxx>, Anthony Perard <anthony.perard@xxxxxxxxxx>, Harsh Prateek Bora <harshpb@xxxxxxxxxxxxx>, Alex Bennée <alex.bennee@xxxxxxxxxx>, Eduardo Habkost <eduardo@xxxxxxxxxxx>, Paolo Bonzini <pbonzini@xxxxxxxxxx>, Vladimir Sementsov-Ogievskiy <vsementsov@xxxxxxxxxxxxxx>, Cédric Le Goater <clg@xxxxxxxx>, qemu-ppc@xxxxxxxxxx, Philippe Mathieu-Daudé <philmd@xxxxxxxxxx>, Christian Borntraeger <borntraeger@xxxxxxxxxxxxx>, Akihiko Odaki <akihiko.odaki@xxxxxxxxxx>, Leonardo Bras <leobras@xxxxxxxxxx>, Nicholas Piggin <npiggin@xxxxxxxxx>, Jiaxun Yang <jiaxun.yang@xxxxxxxxxxx>
- Delivery-date: Thu, 30 Nov 2023 21:41:16 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On Thu, 30 Nov 2023, Stefan Hajnoczi wrote:
On Thu, Nov 30, 2023 at 03:08:49PM -0500, Peter Xu wrote:
On Wed, Nov 29, 2023 at 04:26:20PM -0500, Stefan Hajnoczi wrote:
The Big QEMU Lock (BQL) has many names and they are confusing. The
actual QemuMutex variable is called qemu_global_mutex but it's commonly
referred to as the BQL in discussions and some code comments. The
locking APIs, however, are called qemu_mutex_lock_iothread() and
qemu_mutex_unlock_iothread().
The "iothread" name is historic and comes from when the main thread was
split into into KVM vcpu threads and the "iothread" (now called the main
loop thread). I have contributed to the confusion myself by introducing
a separate --object iothread, a separate concept unrelated to the BQL.
The "iothread" name is no longer appropriate for the BQL. Rename the
locking APIs to:
- void qemu_bql_lock(void)
- void qemu_bql_unlock(void)
- bool qemu_bql_locked(void)
There are more APIs with "iothread" in their names. Subsequent patches
will rename them. There are also comments and documentation that will be
updated in later patches.
Signed-off-by: Stefan Hajnoczi <stefanha@xxxxxxxxxx>
Acked-by: Peter Xu <peterx@xxxxxxxxxx>
Two nickpicks:
- BQL contains "QEMU" as the 2nd character, so maybe easier to further
rename qemu_bql into bql_?
Philippe wondered whether the variable name should end with _mutex (or
_lock is common too), so an alternative might be big_qemu_lock. That's
imperfect because it doesn't start with the usual qemu_ prefix.
qemu_big_lock is better in that regard but inconsistent with our BQL
abbreviation.
BQL isn't very specific for those unfamiliar with the code but it's short
and already used and known by people so I'm OK with qemu_bql with some
comments and docs explainig here and there what bql stands for should be
enough for new people to quickly find out. If we want to be more verbose
how about "qemu_global_mutex" which is self describing but longer and does
not resemble BQL so then comments may be needed to explain this is what
was called BQL as well. I don't mind either way though.
Regards,
BALATON Zoltan
I don't like putting an underscore at the end. It's unusual and would
make me wonder what that means.
Naming is hard, but please discuss and I'm open to change to BQL
variable's name to whatever we all agree on.
- Could we keep the full spell of BQL at some places, so people can still
reference it if not familiar? IIUC most of the BQL helpers will root
back to the major three functions (_lock, _unlock, _locked), perhaps
add a comment of "BQL stands for..." over these three functions as
comment?
Yes, I'll update the doc comments to say "Big QEMU Lock (BQL)" for each
of these functions.
Stefan
|