qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: qemu-block@nongnu.org
Cc: kwolf@redhat.com, stefanha@redhat.com, eblake@redhat.com,
	eesposit@redhat.com, pbonzini@redhat.com,
	vsementsov@yandex-team.ru, qemu-devel@nongnu.org
Subject: [PATCH 08/21] block: Mark bdrv_replace_child_noperm() GRAPH_WRLOCK
Date: Thu, 17 Aug 2023 14:50:07 +0200	[thread overview]
Message-ID: <20230817125020.208339-9-kwolf@redhat.com> (raw)
In-Reply-To: <20230817125020.208339-1-kwolf@redhat.com>

Instead of taking the writer lock internally, require callers to already
hold it when calling bdrv_replace_child_noperm(). 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>
---
 block.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/block.c b/block.c
index 9c4f24f4b9..500a08b26c 100644
--- a/block.c
+++ b/block.c
@@ -91,8 +91,9 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
 static bool bdrv_recurse_has_child(BlockDriverState *bs,
                                    BlockDriverState *child);
 
-static void bdrv_replace_child_noperm(BdrvChild *child,
-                                      BlockDriverState *new_bs);
+static void GRAPH_WRLOCK
+bdrv_replace_child_noperm(BdrvChild *child, BlockDriverState *new_bs);
+
 static void bdrv_remove_child(BdrvChild *child, Transaction *tran);
 
 static int bdrv_reopen_prepare(BDRVReopenState *reopen_state,
@@ -2385,6 +2386,8 @@ static void bdrv_replace_child_abort(void *opaque)
     BlockDriverState *new_bs = s->child->bs;
 
     GLOBAL_STATE_CODE();
+    bdrv_graph_wrlock(s->old_bs);
+
     /* old_bs reference is transparently moved from @s to @s->child */
     if (!s->child->bs) {
         /*
@@ -2401,6 +2404,8 @@ static void bdrv_replace_child_abort(void *opaque)
     }
     assert(s->child->quiesced_parent);
     bdrv_replace_child_noperm(s->child, s->old_bs);
+
+    bdrv_graph_wrunlock();
     bdrv_unref(new_bs);
 }
 
@@ -2437,7 +2442,10 @@ static void bdrv_replace_child_tran(BdrvChild *child, BlockDriverState *new_bs,
     if (new_bs) {
         bdrv_ref(new_bs);
     }
+
+    bdrv_graph_wrlock(new_bs);
     bdrv_replace_child_noperm(child, new_bs);
+    bdrv_graph_wrunlock();
     /* old_bs reference is transparently moved from @child to @s */
 }
 
@@ -2856,8 +2864,8 @@ uint64_t bdrv_qapi_perm_to_blk_perm(BlockPermission qapi_perm)
  * If @new_bs is non-NULL, the parent of @child must already be drained through
  * @child and the caller must hold the AioContext lock for @new_bs.
  */
-static void bdrv_replace_child_noperm(BdrvChild *child,
-                                      BlockDriverState *new_bs)
+static void GRAPH_WRLOCK
+bdrv_replace_child_noperm(BdrvChild *child, BlockDriverState *new_bs)
 {
     BlockDriverState *old_bs = child->bs;
     int new_bs_quiesce_counter;
@@ -2892,8 +2900,6 @@ static void bdrv_replace_child_noperm(BdrvChild *child,
         assert(bdrv_get_aio_context(old_bs) == bdrv_get_aio_context(new_bs));
     }
 
-    /* TODO Pull this up into the callers to avoid polling here */
-    bdrv_graph_wrlock(new_bs);
     if (old_bs) {
         if (child->klass->detach) {
             child->klass->detach(child);
@@ -2909,7 +2915,6 @@ static void bdrv_replace_child_noperm(BdrvChild *child,
             child->klass->attach(child);
         }
     }
-    bdrv_graph_wrunlock();
 
     /*
      * If the parent was drained through this BdrvChild previously, but new_bs
@@ -2950,7 +2955,10 @@ static void bdrv_attach_child_common_abort(void *opaque)
     BlockDriverState *bs = s->child->bs;
 
     GLOBAL_STATE_CODE();
+
+    bdrv_graph_wrlock(NULL);
     bdrv_replace_child_noperm(s->child, NULL);
+    bdrv_graph_wrunlock();
 
     if (bdrv_get_aio_context(bs) != s->old_child_ctx) {
         bdrv_try_change_aio_context(bs, s->old_child_ctx, NULL, &error_abort);
@@ -3078,8 +3086,10 @@ static BdrvChild *bdrv_attach_child_common(BlockDriverState *child_bs,
      * a problem, we already did this), but it will still poll until the parent
      * is fully quiesced, so it will not be negatively affected either.
      */
+    bdrv_graph_wrlock(child_bs);
     bdrv_parent_drained_begin_single(new_child);
     bdrv_replace_child_noperm(new_child, child_bs);
+    bdrv_graph_wrunlock();
 
     BdrvAttachChildCommonState *s = g_new(BdrvAttachChildCommonState, 1);
     *s = (BdrvAttachChildCommonState) {
@@ -3223,7 +3233,9 @@ void bdrv_root_unref_child(BdrvChild *child)
     BlockDriverState *child_bs = child->bs;
 
     GLOBAL_STATE_CODE();
+    bdrv_graph_wrlock(NULL);
     bdrv_replace_child_noperm(child, NULL);
+    bdrv_graph_wrunlock();
     bdrv_child_free(child);
 
     if (child_bs) {
-- 
2.41.0



  parent reply	other threads:[~2023-08-17 12:52 UTC|newest]

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-17 12:49 [PATCH 00/21] Graph locking part 4 (node management) Kevin Wolf
2023-08-17 12:50 ` [PATCH 01/21] block: Remove unused BlockReopenQueueEntry.perms_checked Kevin Wolf
2023-08-17 19:18   ` Eric Blake
2023-08-21  7:34   ` Emanuele Giuseppe Esposito
2023-08-22 18:23   ` Stefan Hajnoczi
2023-08-17 12:50 ` [PATCH 02/21] preallocate: Factor out preallocate_truncate_to_real_size() Kevin Wolf
2023-08-18 15:23   ` Eric Blake
2023-08-21  7:34   ` Emanuele Giuseppe Esposito
2023-08-22 18:26   ` Stefan Hajnoczi
2023-08-17 12:50 ` [PATCH 03/21] preallocate: Don't poll during permission updates Kevin Wolf
2023-08-18 15:34   ` Eric Blake
2023-08-22 18:41   ` Stefan Hajnoczi
2023-08-17 12:50 ` [PATCH 04/21] block: Take AioContext lock for bdrv_append() more consistently Kevin Wolf
2023-08-18 16:07   ` Eric Blake
2023-08-21  7:34   ` Emanuele Giuseppe Esposito
2023-08-22 18:49   ` Stefan Hajnoczi
2023-08-17 12:50 ` [PATCH 05/21] block: Introduce bdrv_schedule_unref() Kevin Wolf
2023-08-18 16:23   ` Eric Blake
2023-08-18 16:26     ` Eric Blake
2023-08-21 16:59       ` Kevin Wolf
2023-08-22 19:01   ` Stefan Hajnoczi
2023-09-05 16:39     ` Kevin Wolf
2023-09-06  9:17       ` Kevin Wolf
2023-08-17 12:50 ` [PATCH 06/21] block-coroutine-wrapper: Add no_co_wrapper_bdrv_wrlock functions Kevin Wolf
2023-08-21  7:34   ` Emanuele Giuseppe Esposito
2023-08-22 19:03   ` Stefan Hajnoczi
2023-08-17 12:50 ` [PATCH 07/21] block-coroutine-wrapper: Allow arbitrary parameter names Kevin Wolf
2023-08-21  7:35   ` Emanuele Giuseppe Esposito
2023-08-22 19:03   ` Stefan Hajnoczi
2023-08-17 12:50 ` Kevin Wolf [this message]
2023-08-21  7:35   ` [PATCH 08/21] block: Mark bdrv_replace_child_noperm() GRAPH_WRLOCK Emanuele Giuseppe Esposito
2023-08-22 19:05   ` Stefan Hajnoczi
2023-08-17 12:50 ` [PATCH 09/21] block: Mark bdrv_replace_child_tran() GRAPH_WRLOCK Kevin Wolf
2023-08-21  7:35   ` Emanuele Giuseppe Esposito
2023-08-22 19:14   ` Stefan Hajnoczi
2023-08-17 12:50 ` [PATCH 10/21] block: Mark bdrv_attach_child_common() GRAPH_WRLOCK Kevin Wolf
2023-08-22 19:16   ` Stefan Hajnoczi
2023-08-17 12:50 ` [PATCH 11/21] block: Call transaction callbacks with lock held Kevin Wolf
2023-08-22 19:19   ` Stefan Hajnoczi
2023-08-17 12:50 ` [PATCH 12/21] block: Mark bdrv_attach_child() GRAPH_WRLOCK Kevin Wolf
2023-08-21  7:35   ` Emanuele Giuseppe Esposito
2023-08-22 19:21   ` Stefan Hajnoczi
2023-08-17 12:50 ` [PATCH 13/21] block: Mark bdrv_parent_perms_conflict() and callers GRAPH_RDLOCK Kevin Wolf
2023-08-21  7:35   ` Emanuele Giuseppe Esposito
2023-08-22 19:23   ` Stefan Hajnoczi
2023-08-17 12:50 ` [PATCH 14/21] block: Mark bdrv_get_cumulative_perm() " Kevin Wolf
2023-08-21  7:35   ` Emanuele Giuseppe Esposito
2023-08-22 19:28   ` Stefan Hajnoczi
2023-08-17 12:50 ` [PATCH 15/21] block: Mark bdrv_child_perm() GRAPH_RDLOCK Kevin Wolf
2023-08-21  7:35   ` Emanuele Giuseppe Esposito
2023-08-22 19:29   ` Stefan Hajnoczi
2023-08-17 12:50 ` [PATCH 16/21] block: Mark bdrv_parent_cb_change_media() GRAPH_RDLOCK Kevin Wolf
2023-08-21  7:35   ` Emanuele Giuseppe Esposito
2023-08-22 19:29   ` Stefan Hajnoczi
2023-08-17 12:50 ` [PATCH 17/21] block: Take graph rdlock in bdrv_drop_intermediate() Kevin Wolf
2023-08-21  7:35   ` Emanuele Giuseppe Esposito
2023-08-22 19:35   ` Stefan Hajnoczi
2023-09-05 15:26     ` Kevin Wolf
2023-08-17 12:50 ` [PATCH 18/21] block: Take graph rdlock in bdrv_change_aio_context() Kevin Wolf
2023-08-21  7:35   ` Emanuele Giuseppe Esposito
2023-08-17 12:50 ` [PATCH 19/21] block: Mark bdrv_root_unref_child() GRAPH_WRLOCK Kevin Wolf
2023-08-21  7:35   ` Emanuele Giuseppe Esposito
2023-08-22 19:38   ` Stefan Hajnoczi
2023-08-17 12:50 ` [PATCH 20/21] block: Mark bdrv_unref_child() GRAPH_WRLOCK Kevin Wolf
2023-08-21  7:35   ` Emanuele Giuseppe Esposito
2023-08-22 19:39   ` Stefan Hajnoczi
2023-08-17 12:50 ` [PATCH 21/21] block: Mark bdrv_add/del_child() and caller GRAPH_WRLOCK Kevin Wolf
2023-08-21  7:35   ` Emanuele Giuseppe Esposito
2023-08-22 19:40   ` Stefan Hajnoczi
2023-08-22 18:20 ` [PATCH 00/21] Graph locking part 4 (node management) Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230817125020.208339-9-kwolf@redhat.com \
    --to=kwolf@redhat.com \
    --cc=eblake@redhat.com \
    --cc=eesposit@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=vsementsov@yandex-team.ru \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).