[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 00/18] block: Introduce a block graph rwlock
From: |
Emanuele Giuseppe Esposito |
Subject: |
Re: [PATCH 00/18] block: Introduce a block graph rwlock |
Date: |
Wed, 7 Dec 2022 15:12:49 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 |
Am 07/12/2022 um 14:18 schrieb Kevin Wolf:
> 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
>
Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
^ I am curious to see if I am allowed to have my r-b also on my patches :)
> 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
>
- [PATCH 12/18] block: remove unnecessary assert_bdrv_graph_writable(), (continued)
- [PATCH 12/18] block: remove unnecessary assert_bdrv_graph_writable(), Kevin Wolf, 2022/12/07
- [PATCH 11/18] block: wrlock in bdrv_replace_child_noperm, Kevin Wolf, 2022/12/07
- [PATCH 06/18] clang-tsa: Add TSA_ASSERT() macro, Kevin Wolf, 2022/12/07
- [PATCH 14/18] graph-lock: TSA annotations for lock/unlock functions, Kevin Wolf, 2022/12/07
- [PATCH 13/18] block: assert that graph read and writes are performed correctly, Kevin Wolf, 2022/12/07
- [PATCH 16/18] block-coroutine-wrapper.py: introduce annotations that take the graph rdlock, Kevin Wolf, 2022/12/07
- [PATCH 15/18] Mark assert_bdrv_graph_readable/writable() GRAPH_RD/WRLOCK, Kevin Wolf, 2022/12/07
- [PATCH 17/18] block: use co_wrapper_mixed_bdrv_rdlock in functions taking the rdlock, Kevin Wolf, 2022/12/07
- [PATCH 18/18] block: GRAPH_RDLOCK for functions only called by co_wrappers, Kevin Wolf, 2022/12/07
- [PATCH 09/18] test-bdrv-drain: Fix incorrrect drain assumptions, Kevin Wolf, 2022/12/07
- Re: [PATCH 00/18] block: Introduce a block graph rwlock,
Emanuele Giuseppe Esposito <=