[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: |
Kevin Wolf |
|
Subject: |
Re: [PATCH 00/22] block: Graph locking part 5 (protect children/parent links) |
|
Date: |
Wed, 11 Oct 2023 13:05:26 +0200 |
Am 10.10.2023 um 22:48 hat Stefan Hajnoczi geschrieben:
> 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>
Thanks, applied to the block branch.
Kevin
signature.asc
Description: PGP signature