[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 04/24] block: Mark bdrv_root_attach_child() GRAPH_WRLOCK
From: |
Eric Blake |
Subject: |
Re: [PATCH 04/24] block: Mark bdrv_root_attach_child() GRAPH_WRLOCK |
Date: |
Fri, 27 Oct 2023 15:22:56 -0500 |
User-agent: |
NeoMutt/20231023 |
On Fri, Oct 27, 2023 at 05:53:13PM +0200, Kevin Wolf wrote:
> Instead of taking the writer lock internally, require callers to already
> hold it when calling bdrv_root_attach_child(). These callers will
> typically already hold the graph lock once the locking work is
> completed, which means that they can't call functions that take it
> internally.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> include/block/block_int-global-state.h | 13 +++++++------
> block.c | 5 +----
> block/block-backend.c | 2 ++
> blockjob.c | 2 ++
> 4 files changed, 12 insertions(+), 10 deletions(-)
>
> +++ b/block.c
> @@ -3214,8 +3214,6 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState
> *child_bs,
>
> GLOBAL_STATE_CODE();
>
> - bdrv_graph_wrlock(child_bs);
> -
> child = bdrv_attach_child_common(child_bs, child_name, child_class,
Do we need some sort of assertion that the caller did indeed grab the
lock at this point? Or is that redundant with assertions made in all
helper functions we are calling where the lock already matters?
> child_role, perm, shared_perm, opaque,
> tran, errp);
> @@ -3228,9 +3226,8 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState
> *child_bs,
>
> out:
> tran_finalize(tran, ret);
> - bdrv_graph_wrunlock();
>
> - bdrv_unref(child_bs);
> + bdrv_schedule_unref(child_bs);
The change to unref matches the change to the slightly longer lock
scope; makes sense.
Assuming I figured out the answer to my question above,
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
- Re: [PATCH 05/24] block: Mark block_job_add_bdrv() GRAPH_WRLOCK, (continued)
- [PATCH 15/24] block: Mark bdrv_replace_node_common() GRAPH_WRLOCK, Kevin Wolf, 2023/10/27
- [PATCH 10/24] block: Mark bdrv_chain_contains() and callers GRAPH_RDLOCK, Kevin Wolf, 2023/10/27
- [PATCH 09/24] block: Mark bdrv_(un)freeze_backing_chain() and callers GRAPH_RDLOCK, Kevin Wolf, 2023/10/27
- [PATCH 14/24] block: Inline bdrv_set_backing_noperm(), Kevin Wolf, 2023/10/27
- [PATCH 04/24] block: Mark bdrv_root_attach_child() GRAPH_WRLOCK, Kevin Wolf, 2023/10/27
- Re: [PATCH 04/24] block: Mark bdrv_root_attach_child() GRAPH_WRLOCK,
Eric Blake <=
- [PATCH 21/24] qcow2: Take locks for accessing bs->file, Kevin Wolf, 2023/10/27
- [PATCH 06/24] block: Mark bdrv_filter_or_cow_bs() and callers GRAPH_RDLOCK, Kevin Wolf, 2023/10/27
- [PATCH 07/24] block: Mark bdrv_skip_implicit_filters() and callers GRAPH_RDLOCK, Kevin Wolf, 2023/10/27
- [PATCH 17/24] block: Protect bs->backing with graph_lock, Kevin Wolf, 2023/10/27
- [PATCH 16/24] block: Mark bdrv_replace_node() GRAPH_WRLOCK, Kevin Wolf, 2023/10/27