[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 00/22] block: Graph locking part 5 (protect children/parent l
From: |
Stefan Hajnoczi |
Subject: |
Re: [PATCH 00/22] block: Graph locking part 5 (protect children/parent links) |
Date: |
Tue, 10 Oct 2023 16:48:50 -0400 |
On Fri, Sep 29, 2023 at 04:51:35PM +0200, Kevin Wolf wrote:
> After all the preparation in previous series, this series reaches an
> important milestone for the graph locking work: TSA can now verify that
> all accesses to the children and parent lists of nodes happen under the
> graph lock.
>
> However, this is not the end of the graph locking work yet. On the one
> hand, graph locking annotations aren't consistently present on all
> functions and having an unannotated function in the middle of the call
> chain means that TSA doesn't check if the locking is consistent (in
> fact, we know that functions like bdrv_unref() are still called in
> places where they strictly speaking must not be called).
>
> On the other hand, the graph consists of more than just the children and
> parent lists. While it might be possible to convince me that the global
> node lists (graph_bdrv_states/all_bdrv_states) are safe because
> iothreads shouldn't access them, at least BlockDriverState.file/backing
> still need proper locking.
>
> There may be other fields in BlockDriverState that need to be covered
> by the lock, too. For example, Stefan said that he would look into
> annotating BlockLimits accesses to be protected by the graph lock, too.
>
> Emanuele Giuseppe Esposito (1):
> block: Mark drain related functions GRAPH_RDLOCK
>
> Kevin Wolf (21):
> test-bdrv-drain: Don't call bdrv_graph_wrlock() in coroutine context
> block-coroutine-wrapper: Add no_co_wrapper_bdrv_rdlock functions
> block: Take graph rdlock in bdrv_inactivate_all()
> block: Mark bdrv_first_blk() and bdrv_is_root_node() GRAPH_RDLOCK
> block: Mark bdrv_parent_cb_resize() and callers GRAPH_RDLOCK
> block: Mark bdrv_snapshot_fallback() and callers GRAPH_RDLOCK
> block: Take graph rdlock in parts of reopen
> block: Mark bdrv_get_xdbg_block_graph() and callers GRAPH_RDLOCK
> block: Mark bdrv_refresh_filename() and callers GRAPH_RDLOCK
> block: Mark bdrv_primary_child() and callers GRAPH_RDLOCK
> block: Mark bdrv_get_parent_name() and callers GRAPH_RDLOCK
> block: Mark bdrv_amend_options() and callers GRAPH_RDLOCK
> qcow2: Mark qcow2_signal_corruption() and callers GRAPH_RDLOCK
> qcow2: Mark qcow2_inactivate() and callers GRAPH_RDLOCK
> qcow2: Mark check_constraints_on_bitmap() GRAPH_RDLOCK
> block: Mark bdrv_op_is_blocked() and callers GRAPH_RDLOCK
> block: Mark bdrv_apply_auto_read_only() and callers GRAPH_RDLOCK
> block: Mark bdrv_get_specific_info() and callers GRAPH_RDLOCK
> block: Protect bs->parents with graph_lock
> block: Protect bs->children with graph_lock
> block: Add assertion for bdrv_graph_wrlock()
>
> block/qcow2.h | 187 ++++++++++++--------
> block/vhdx.h | 5 +-
> include/block/block-common.h | 7 +-
> include/block/block-global-state.h | 34 ++--
> include/block/block-io.h | 42 +++--
> include/block/block_int-common.h | 69 ++++----
> include/block/block_int-io.h | 7 +-
> include/block/graph-lock.h | 3 +-
> include/block/qapi.h | 23 ++-
> include/block/snapshot.h | 24 +--
> include/sysemu/block-backend-global-state.h | 4 +-
> block.c | 120 +++++++++----
> block/backup.c | 1 +
> block/block-backend.c | 9 +-
> block/bochs.c | 2 +
> block/cloop.c | 2 +
> block/commit.c | 1 +
> block/crypto.c | 4 +-
> block/curl.c | 2 +
> block/dmg.c | 2 +
> block/export/export.c | 4 +
> block/gluster.c | 2 +
> block/graph-lock.c | 3 +-
> block/io.c | 45 ++++-
> block/iscsi.c | 2 +
> block/monitor/block-hmp-cmds.c | 5 +
> block/nbd.c | 3 +-
> block/nfs.c | 2 +-
> block/parallels.c | 3 +
> block/qapi-sysemu.c | 11 +-
> block/qapi.c | 11 +-
> block/qcow.c | 3 +
> block/qcow2-bitmap.c | 38 ++--
> block/qcow2-cache.c | 11 +-
> block/qcow2-cluster.c | 62 +++----
> block/qcow2-refcount.c | 80 +++++----
> block/qcow2.c | 72 +++++---
> block/quorum.c | 4 +-
> block/raw-format.c | 2 +
> block/rbd.c | 4 +
> block/replication.c | 21 ++-
> block/snapshot.c | 54 +++++-
> block/vdi.c | 3 +
> block/vhdx.c | 4 +
> block/vmdk.c | 53 +++---
> block/vpc.c | 3 +
> block/vvfat.c | 2 +
> blockdev.c | 44 +++++
> blockjob.c | 1 +
> migration/block.c | 2 +
> migration/migration-hmp-cmds.c | 2 +
> qemu-img.c | 16 ++
> qemu-io-cmds.c | 3 +
> tests/unit/test-bdrv-drain.c | 15 +-
> tests/unit/test-block-iothread.c | 8 +
> scripts/block-coroutine-wrapper.py | 10 +-
> 56 files changed, 769 insertions(+), 387 deletions(-)
>
> --
> 2.41.0
>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
signature.asc
Description: PGP signature
- Re: [PATCH 00/22] block: Graph locking part 5 (protect children/parent links),
Stefan Hajnoczi <=