From: Kevin Wolf <kwolf@redhat.com>
To: qemu-block@nongnu.org
Cc: kwolf@redhat.com, hreitz@redhat.com, eesposit@redhat.com,
pbonzini@redhat.com, vsementsov@yandex-team.ru,
qemu-devel@nongnu.org
Subject: [PATCH 00/18] block: Introduce a block graph rwlock
Date: Wed, 7 Dec 2022 14:18:20 +0100 [thread overview]
Message-ID: <20221207131838.239125-1-kwolf@redhat.com> (raw)
This series supersedes the first half of Emanuele's "Protect the block
layer with a rwlock: part 1". It introduces the basic infrastructure for
protecting the block graph (specifically parent/child links) with a
rwlock. Actually taking the reader lock in all necessary places is left
for future series.
Compared to Emanuele's series, this one adds patches to make use of
clang's Thread Safety Analysis (TSA) feature in order to statically
check at compile time that the places where we assert that we hold the
lock actually do hold it. Once we cover all relevant places, the check
can be extended to verify that all accesses of bs->children and
bs->parents hold the lock.
For reference, here is the more detailed version of our plan in
Emanuele's words from his series:
The aim is to replace the current AioContext lock with much
fine-grained locks, aimed to protect only specific data. Currently
the AioContext lock is used pretty much everywhere, and it's not
even clear what it is protecting exactly.
The aim of the rwlock is to cover graph modifications: more
precisely, when a BlockDriverState parent or child list is modified
or read, since it can be concurrently accessed by the main loop and
iothreads.
The main assumption is that the main loop is the only one allowed to
perform graph modifications, and so far this has always been held by
the current code.
The rwlock is inspired from cpus-common.c implementation, and aims
to reduce cacheline bouncing by having per-aiocontext counter of
readers. All details and implementation of the lock are in patch 2.
We distinguish between writer (main loop, under BQL) that modifies
the graph, and readers (all other coroutines running in various
AioContext), that go through the graph edges, reading ->parents
and->children. The writer (main loop) has an "exclusive" access,
so it first waits for current read to finish, and then prevents
incoming ones from entering while it has the exclusive access. The
readers (coroutines in multiple AioContext) are free to access the
graph as long the writer is not modifying the graph. In case it is,
they go in a CoQueue and sleep until the writer is done.
In this and following series, we try to follow the following locking
pattern:
- bdrv_co_* functions that call BlockDriver callbacks always expect
the lock to be taken, therefore they assert.
- blk_co_* functions are called from external code outside the block
layer, which should not have to care about the block layer's
internal locking. Usually they also call blk_wait_while_drained().
Therefore they take the lock internally.
The long term goal of this series is to eventually replace the
AioContext lock, so that we can get rid of it once and for all.
Emanuele Giuseppe Esposito (7):
graph-lock: Implement guard macros
async: Register/unregister aiocontext in graph lock list
block: wrlock in bdrv_replace_child_noperm
block: remove unnecessary assert_bdrv_graph_writable()
block: assert that graph read and writes are performed correctly
block-coroutine-wrapper.py: introduce annotations that take the graph
rdlock
block: use co_wrapper_mixed_bdrv_rdlock in functions taking the rdlock
Kevin Wolf (10):
block: Factor out bdrv_drain_all_begin_nopoll()
Import clang-tsa.h
clang-tsa: Add TSA_ASSERT() macro
clang-tsa: Add macros for shared locks
configure: Enable -Wthread-safety if present
test-bdrv-drain: Fix incorrrect drain assumptions
block: Fix locking in external_snapshot_prepare()
graph-lock: TSA annotations for lock/unlock functions
Mark assert_bdrv_graph_readable/writable() GRAPH_RD/WRLOCK
block: GRAPH_RDLOCK for functions only called by co_wrappers
Paolo Bonzini (1):
graph-lock: Introduce a lock to protect block graph operations
configure | 1 +
block/coroutines.h | 19 +-
include/block/aio.h | 9 +
include/block/block-common.h | 9 +-
include/block/block-global-state.h | 1 +
include/block/block-io.h | 53 +++--
include/block/block_int-common.h | 24 +--
include/block/block_int-global-state.h | 17 --
include/block/block_int.h | 1 +
include/block/graph-lock.h | 280 +++++++++++++++++++++++++
include/qemu/clang-tsa.h | 114 ++++++++++
block.c | 24 ++-
block/graph-lock.c | 275 ++++++++++++++++++++++++
block/io.c | 21 +-
blockdev.c | 4 +
stubs/graph-lock.c | 10 +
tests/unit/test-bdrv-drain.c | 18 ++
util/async.c | 4 +
scripts/block-coroutine-wrapper.py | 12 ++
block/meson.build | 1 +
stubs/meson.build | 1 +
21 files changed, 820 insertions(+), 78 deletions(-)
create mode 100644 include/block/graph-lock.h
create mode 100644 include/qemu/clang-tsa.h
create mode 100644 block/graph-lock.c
create mode 100644 stubs/graph-lock.c
--
2.38.1
next reply other threads:[~2022-12-07 13:20 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-07 13:18 Kevin Wolf [this message]
2022-12-07 13:18 ` [PATCH 01/18] block: Factor out bdrv_drain_all_begin_nopoll() Kevin Wolf
2022-12-07 13:18 ` [PATCH 02/18] graph-lock: Introduce a lock to protect block graph operations Kevin Wolf
2022-12-07 13:18 ` [PATCH 03/18] graph-lock: Implement guard macros Kevin Wolf
2022-12-07 13:18 ` [PATCH 04/18] async: Register/unregister aiocontext in graph lock list Kevin Wolf
2022-12-07 13:18 ` [PATCH 05/18] Import clang-tsa.h Kevin Wolf
2022-12-07 13:18 ` [PATCH 06/18] clang-tsa: Add TSA_ASSERT() macro Kevin Wolf
2022-12-07 13:18 ` [PATCH 07/18] clang-tsa: Add macros for shared locks Kevin Wolf
2022-12-07 13:18 ` [PATCH 08/18] configure: Enable -Wthread-safety if present Kevin Wolf
2022-12-07 13:18 ` [PATCH 09/18] test-bdrv-drain: Fix incorrrect drain assumptions Kevin Wolf
2022-12-07 13:18 ` [PATCH 10/18] block: Fix locking in external_snapshot_prepare() Kevin Wolf
2022-12-07 13:18 ` [PATCH 11/18] block: wrlock in bdrv_replace_child_noperm Kevin Wolf
2022-12-07 13:18 ` [PATCH 12/18] block: remove unnecessary assert_bdrv_graph_writable() Kevin Wolf
2022-12-07 13:18 ` [PATCH 13/18] block: assert that graph read and writes are performed correctly Kevin Wolf
2022-12-07 13:18 ` [PATCH 14/18] graph-lock: TSA annotations for lock/unlock functions Kevin Wolf
2022-12-07 13:18 ` [PATCH 15/18] Mark assert_bdrv_graph_readable/writable() GRAPH_RD/WRLOCK Kevin Wolf
2022-12-07 13:18 ` [PATCH 16/18] block-coroutine-wrapper.py: introduce annotations that take the graph rdlock Kevin Wolf
2022-12-07 13:18 ` [PATCH 17/18] block: use co_wrapper_mixed_bdrv_rdlock in functions taking the rdlock Kevin Wolf
2022-12-07 13:18 ` [PATCH 18/18] block: GRAPH_RDLOCK for functions only called by co_wrappers Kevin Wolf
2022-12-07 14:12 ` [PATCH 00/18] block: Introduce a block graph rwlock Emanuele Giuseppe Esposito
2022-12-07 16:08 ` Kevin Wolf
2022-12-12 17:14 ` Kevin Wolf
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=20221207131838.239125-1-kwolf@redhat.com \
--to=kwolf@redhat.com \
--cc=eesposit@redhat.com \
--cc=hreitz@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=vsementsov@yandex-team.ru \
/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).