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