qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Fiona Ebner <f.ebner@proxmox.com>
To: qemu-block@nongnu.org
Cc: qemu-devel@nongnu.org, kwolf@redhat.com, den@virtuozzo.com,
	andrey.drobyshev@virtuozzo.com, hreitz@redhat.com,
	stefanha@redhat.com, eblake@redhat.com, jsnow@redhat.com,
	vsementsov@yandex-team.ru, xiechanglong.d@gmail.com,
	wencongyang2@huawei.com, berto@igalia.com, fam@euphon.net,
	ari@tuxera.com
Subject: [PATCH v3 23/24] block: never use atomics to access bs->quiesce_counter
Date: Mon, 26 May 2025 15:21:39 +0200	[thread overview]
Message-ID: <20250526132140.1641377-24-f.ebner@proxmox.com> (raw)
In-Reply-To: <20250526132140.1641377-1-f.ebner@proxmox.com>

All accesses of bs->quiesce_counter are in the main thread, either
after a GLOBAL_STATE_CODE() macro or in a function with GRAPH_WRLOCK
annotation.

This is essentially a revert of 414c2ec358 ("block: access
quiesce_counter with atomic ops"). At that time, neither the
GLOBAL_STATE_CODE() macro nor the GRAPH_WRLOCK annotation existed.
Even if the field was only accessed in the main thread back then (did
not check if that is actually the case), it wouldn't have been easy to
verify.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

No changes in v3.

 block/io.c                       | 7 ++-----
 include/block/block_int-common.h | 2 +-
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/block/io.c b/block/io.c
index ac5c7174f6..9bd8ba8431 100644
--- a/block/io.c
+++ b/block/io.c
@@ -361,7 +361,7 @@ static void bdrv_do_drained_begin(BlockDriverState *bs, BdrvChild *parent,
     GLOBAL_STATE_CODE();
 
     /* Stop things in parent-to-child order */
-    if (qatomic_fetch_inc(&bs->quiesce_counter) == 0) {
+    if (bs->quiesce_counter++ == 0) {
         GRAPH_RDLOCK_GUARD_MAINLOOP();
         bdrv_parent_drained_begin(bs, parent);
         if (bs->drv && bs->drv->bdrv_drain_begin) {
@@ -401,8 +401,6 @@ bdrv_drained_begin(BlockDriverState *bs)
  */
 static void bdrv_do_drained_end(BlockDriverState *bs, BdrvChild *parent)
 {
-    int old_quiesce_counter;
-
     IO_OR_GS_CODE();
 
     if (qemu_in_coroutine()) {
@@ -415,8 +413,7 @@ static void bdrv_do_drained_end(BlockDriverState *bs, BdrvChild *parent)
     assert(bs->quiesce_counter > 0);
 
     /* Re-enable things in child-to-parent order */
-    old_quiesce_counter = qatomic_fetch_dec(&bs->quiesce_counter);
-    if (old_quiesce_counter == 1) {
+    if (--bs->quiesce_counter == 0) {
         GRAPH_RDLOCK_GUARD_MAINLOOP();
         if (bs->drv && bs->drv->bdrv_drain_end) {
             bs->drv->bdrv_drain_end(bs);
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 168f703fa1..ea846aff03 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -1239,7 +1239,7 @@ struct BlockDriverState {
     /* do we need to tell the quest if we have a volatile write cache? */
     int enable_write_cache;
 
-    /* Accessed with atomic ops.  */
+    /* Accessed only in the main thread. */
     int quiesce_counter;
 
     unsigned int write_gen;               /* Current data generation */
-- 
2.39.5




  parent reply	other threads:[~2025-05-26 13:27 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-26 13:21 [PATCH v3 00/24] block: do not drain while holding the graph lock Fiona Ebner
2025-05-26 13:21 ` [PATCH v3 01/24] block: remove outdated comments about AioContext locking Fiona Ebner
2025-05-26 13:21 ` [PATCH v3 02/24] block: move drain outside of read-locked bdrv_reopen_queue_child() Fiona Ebner
2025-05-26 13:21 ` [PATCH v3 03/24] block/snapshot: move drain outside of read-locked bdrv_snapshot_delete() Fiona Ebner
2025-05-26 13:21 ` [PATCH v3 04/24] block: move drain outside of read-locked bdrv_inactivate_recurse() Fiona Ebner
2025-05-26 13:21 ` [PATCH v3 05/24] block: mark bdrv_parent_change_aio_context() GRAPH_RDLOCK Fiona Ebner
2025-05-26 13:21 ` [PATCH v3 06/24] block: mark change_aio_ctx() callback and instances as GRAPH_RDLOCK(_PTR) Fiona Ebner
2025-05-26 13:21 ` [PATCH v3 07/24] block: mark bdrv_child_change_aio_context() GRAPH_RDLOCK Fiona Ebner
2025-05-26 13:21 ` [PATCH v3 08/24] block: move drain outside of bdrv_change_aio_context() and mark GRAPH_RDLOCK Fiona Ebner
2025-05-27 15:11   ` Kevin Wolf
2025-05-26 13:21 ` [PATCH v3 09/24] block: move drain outside of bdrv_try_change_aio_context() Fiona Ebner
2025-05-27 15:12   ` Kevin Wolf
2025-05-26 13:21 ` [PATCH v3 10/24] block: move drain outside of bdrv_attach_child_common(_abort)() Fiona Ebner
2025-05-27 15:23   ` Kevin Wolf
2025-05-28 11:41     ` Fiona Ebner
2025-05-26 13:21 ` [PATCH v3 11/24] block: move drain outside of bdrv_set_backing_hd_drained() Fiona Ebner
2025-05-27 15:29   ` Kevin Wolf
2025-05-28 13:09     ` Fiona Ebner
2025-05-26 13:21 ` [PATCH v3 12/24] block: move drain outside of bdrv_root_attach_child() Fiona Ebner
2025-05-27 17:16   ` Kevin Wolf
2025-05-26 13:21 ` [PATCH v3 13/24] block: move drain outside of bdrv_attach_child() Fiona Ebner
2025-05-27 17:20   ` Kevin Wolf
2025-05-26 13:21 ` [PATCH v3 14/24] block: move drain outside of quorum_add_child() Fiona Ebner
2025-05-27 17:23   ` Kevin Wolf
2025-05-26 13:21 ` [PATCH v3 15/24] block: move drain outside of bdrv_root_unref_child() Fiona Ebner
2025-05-27 17:50   ` Kevin Wolf
2025-05-26 13:21 ` [PATCH v3 16/24] block: move drain outside of quorum_del_child() Fiona Ebner
2025-05-27 17:52   ` Kevin Wolf
2025-05-26 13:21 ` [PATCH v3 17/24] blockdev: drain while unlocked in internal_snapshot_action() Fiona Ebner
2025-05-26 13:21 ` [PATCH v3 18/24] blockdev: drain while unlocked in external_snapshot_action() Fiona Ebner
2025-05-26 13:21 ` [PATCH v3 19/24] block: mark bdrv_drained_begin() and friends as GRAPH_UNLOCKED Fiona Ebner
2025-05-26 13:21 ` [PATCH v3 20/24] iotests/graph-changes-while-io: remove image file after test Fiona Ebner
2025-05-26 13:21 ` [PATCH v3 21/24] iotests/graph-changes-while-io: add test case with removal of lower snapshot Fiona Ebner
2025-05-26 13:21 ` [PATCH v3 22/24] block/io: remove duplicate GLOBAL_STATE_CODE() in bdrv_do_drained_end() Fiona Ebner
2025-05-26 13:21 ` Fiona Ebner [this message]
2025-05-26 13:21 ` [PATCH v3 24/24] block: add bdrv_graph_wrlock_drained() convenience wrapper Fiona Ebner

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=20250526132140.1641377-24-f.ebner@proxmox.com \
    --to=f.ebner@proxmox.com \
    --cc=andrey.drobyshev@virtuozzo.com \
    --cc=ari@tuxera.com \
    --cc=berto@igalia.com \
    --cc=den@virtuozzo.com \
    --cc=eblake@redhat.com \
    --cc=fam@euphon.net \
    --cc=hreitz@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=vsementsov@yandex-team.ru \
    --cc=wencongyang2@huawei.com \
    --cc=xiechanglong.d@gmail.com \
    /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).