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 <stefa...@redhat.com>
signature.asc
Description: PGP signature