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 v4 23/48] block: never use atomics to access bs->quiesce_counter
Date: Fri, 30 May 2025 17:11:00 +0200	[thread overview]
Message-ID: <20250530151125.955508-24-f.ebner@proxmox.com> (raw)
In-Reply-To: <20250530151125.955508-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>
---
 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 925a3e7353..e96c6a6a03 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -1253,7 +1253,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-30 15:15 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-30 15:10 [PATCH v4 00/48] block: do not drain while holding the graph lock Fiona Ebner
2025-05-30 15:10 ` [PATCH v4 01/48] block: remove outdated comments about AioContext locking Fiona Ebner
2025-05-30 15:10 ` [PATCH v4 02/48] block: move drain outside of read-locked bdrv_reopen_queue_child() Fiona Ebner
2025-05-30 15:10 ` [PATCH v4 03/48] block/snapshot: move drain outside of read-locked bdrv_snapshot_delete() Fiona Ebner
2025-05-30 15:10 ` [PATCH v4 04/48] block: move drain outside of read-locked bdrv_inactivate_recurse() Fiona Ebner
2025-05-30 15:10 ` [PATCH v4 05/48] block: mark bdrv_parent_change_aio_context() GRAPH_RDLOCK Fiona Ebner
2025-05-30 15:10 ` [PATCH v4 06/48] block: mark change_aio_ctx() callback and instances as GRAPH_RDLOCK(_PTR) Fiona Ebner
2025-05-30 15:10 ` [PATCH v4 07/48] block: mark bdrv_child_change_aio_context() GRAPH_RDLOCK Fiona Ebner
2025-05-30 15:10 ` [PATCH v4 08/48] block: move drain outside of bdrv_change_aio_context() and mark GRAPH_RDLOCK Fiona Ebner
2025-05-30 15:10 ` [PATCH v4 09/48] block: move drain outside of bdrv_try_change_aio_context() Fiona Ebner
2025-05-30 15:10 ` [PATCH v4 10/48] block: move drain outside of bdrv_attach_child_common(_abort)() Fiona Ebner
2025-05-30 15:10 ` [PATCH v4 11/48] block: move drain outside of bdrv_set_backing_hd_drained() Fiona Ebner
2025-05-30 15:10 ` [PATCH v4 12/48] block: move drain outside of bdrv_root_attach_child() Fiona Ebner
2025-05-30 15:10 ` [PATCH v4 13/48] block: move drain outside of bdrv_attach_child() Fiona Ebner
2025-05-30 15:10 ` [PATCH v4 14/48] block: move drain outside of quorum_add_child() Fiona Ebner
2025-05-30 15:10 ` [PATCH v4 15/48] block: move drain outside of bdrv_root_unref_child() Fiona Ebner
2025-05-30 15:10 ` [PATCH v4 16/48] block: move drain outside of quorum_del_child() Fiona Ebner
2025-05-30 15:10 ` [PATCH v4 17/48] blockdev: drain while unlocked in internal_snapshot_action() Fiona Ebner
2025-05-30 15:10 ` [PATCH v4 18/48] blockdev: drain while unlocked in external_snapshot_action() Fiona Ebner
2025-05-30 15:10 ` [PATCH v4 19/48] block: mark bdrv_drained_begin() and friends as GRAPH_UNLOCKED Fiona Ebner
2025-05-30 15:10 ` [PATCH v4 20/48] iotests/graph-changes-while-io: remove image file after test Fiona Ebner
2025-05-30 15:10 ` [PATCH v4 21/48] iotests/graph-changes-while-io: add test case with removal of lower snapshot Fiona Ebner
2025-05-30 15:10 ` [PATCH v4 22/48] block/io: remove duplicate GLOBAL_STATE_CODE() in bdrv_do_drained_end() Fiona Ebner
2025-05-30 15:11 ` Fiona Ebner [this message]
2025-06-02 14:45   ` [PATCH v4 23/48] block: never use atomics to access bs->quiesce_counter Fiona Ebner
2025-07-01 11:24     ` Kevin Wolf
2025-05-30 15:11 ` [PATCH v4 24/48] block: add bdrv_graph_wrlock_drained() convenience wrapper Fiona Ebner
2025-07-01 11:37   ` Kevin Wolf
2025-05-30 15:11 ` [PATCH v4 25/48] block/mirror: switch to bdrv_set_backing_hd_drained() variant Fiona Ebner
2025-05-30 15:11 ` [PATCH v4 26/48] block/commit: " Fiona Ebner
2025-05-30 15:11 ` [PATCH v4 27/48] block: call bdrv_set_backing_hd() while unlocked in bdrv_open_backing_file() Fiona Ebner
2025-07-01 13:13   ` Kevin Wolf
2025-05-30 15:11 ` [PATCH v4 28/48] block: mark bdrv_set_backing_hd() as GRAPH_UNLOCKED Fiona Ebner
2025-05-30 15:11 ` [PATCH v4 29/48] blockdev: avoid locking and draining multiple times in external_snapshot_abort() Fiona Ebner
2025-06-02  8:56   ` Fiona Ebner
2025-05-30 15:11 ` [PATCH v4 30/48] block: drop wrapper for bdrv_set_backing_hd_drained() Fiona Ebner
2025-05-30 15:11 ` [PATCH v4 31/48] block-backend: mark blk_drain_all() as GRAPH_UNLOCKED Fiona Ebner
2025-05-30 15:11 ` [PATCH v4 32/48] block/snapshot: mark bdrv_all_delete_snapshot() " Fiona Ebner
2025-05-30 15:11 ` [PATCH v4 33/48] block/stream: mark stream_prepare() " Fiona Ebner
2025-05-30 15:11 ` [PATCH v4 34/48] block: mark bdrv_reopen_queue() and bdrv_reopen_multiple() " Fiona Ebner
2025-05-30 15:11 ` [PATCH v4 35/48] block: mark bdrv_inactivate() as GRAPH_RDLOCK and move drain to callers Fiona Ebner
2025-05-30 15:11 ` [PATCH v4 36/48] block: mark bdrv_inactivate_all() as GRAPH_UNLOCKED Fiona Ebner
2025-05-30 15:11 ` [PATCH v4 37/48] block: mark blk_remove_bs() " Fiona Ebner
2025-05-30 15:11 ` [PATCH v4 38/48] block: mark blk_drain() " Fiona Ebner
2025-05-30 15:11 ` [PATCH v4 39/48] block-backend: mark blk_io_limits_disable() " Fiona Ebner
2025-05-30 15:11 ` [PATCH v4 40/48] block/commit: mark commit_abort() " Fiona Ebner
2025-05-30 15:11 ` [PATCH v4 41/48] block: mark bdrv_new() " Fiona Ebner
2025-07-01 16:55   ` Kevin Wolf
2025-05-30 15:11 ` [PATCH v4 42/48] block: mark bdrv_replace_child_bs() " Fiona Ebner
2025-05-30 15:11 ` [PATCH v4 43/48] block: mark bdrv_insert_node() " Fiona Ebner
2025-05-30 15:11 ` [PATCH v4 44/48] block: mark bdrv_drop_intermediate() " Fiona Ebner
2025-05-30 15:11 ` [PATCH v4 45/48] block: mark bdrv_close_all() " Fiona Ebner
2025-05-30 15:11 ` [PATCH v4 46/48] block: mark bdrv_close() " Fiona Ebner
2025-05-30 15:11 ` [PATCH v4 47/48] block: mark bdrv_open_child_common() and its callers GRAPH_UNLOCKED Fiona Ebner
2025-05-30 15:11 ` [PATCH v4 48/48] blockjob: mark block_job_remove_all_bdrv() as GRAPH_UNLOCKED Fiona Ebner
2025-06-03 14:54 ` [PATCH v4 00/48] block: do not drain while holding the graph lock Kevin Wolf
2025-06-04  7:38   ` Fiona Ebner
2025-07-01 17:16 ` Kevin Wolf
2025-07-14 13:43   ` Kevin Wolf
2025-07-15 13:24     ` 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=20250530151125.955508-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).