qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Cc: qemu-block@nongnu.org, eesposit@redhat.com, stefanha@redhat.com,
	hreitz@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org
Subject: Re: [PATCH v2 11/15] block: Call drain callbacks only once
Date: Mon, 28 Nov 2022 13:37:15 +0100	[thread overview]
Message-ID: <Y4Sre2NtpGtuA9AF@redhat.com> (raw)
In-Reply-To: <2f4a4c25-c215-bec9-5148-5b5892232345@yandex-team.ru>

Am 25.11.2022 um 15:59 hat Vladimir Sementsov-Ogievskiy geschrieben:
> On 11/18/22 20:41, Kevin Wolf wrote:
> > We only need to call both the BlockDriver's callback and the parent
> > callbacks when going from undrained to drained or vice versa. A second
> > drain section doesn't make a difference for the driver or the parent,
> > they weren't supposed to send new requests before and after the second
> > drain.
> > 
> > One thing that gets in the way is the 'ignore_bds_parents' parameter in
> > bdrv_do_drained_begin_quiesce() and bdrv_do_drained_end(): It means that
> > bdrv_drain_all_begin() increases bs->quiesce_counter, but does not
> > quiesce the parent through BdrvChildClass callbacks. If an additional
> > drain section is started now, bs->quiesce_counter will be non-zero, but
> > we would still need to quiesce the parent through BdrvChildClass in
> > order to keep things consistent (and unquiesce it on the matching
> > bdrv_drained_end(), even though the counter would reach 0 yet as long as
> 
> would reach -> would NOT reach
> 
> if I understand correctly)

Yes, you're right. Thanks for catching it. Fixing this while applying
the series.

Kevin

> > the bdrv_drain_all() section is still active).
> > 
> > Instead of keeping track of this, let's just get rid of the parameter.
> > It was introduced in commit 6cd5c9d7b2d as an optimisation so that
> > during bdrv_drain_all(), we wouldn't recursively drain all parents up to
> > the root for each node, resulting in quadratic complexity. As it happens,
> > calling the callbacks only once solves the same problem, so as of this
> > patch, we'll still have O(n) complexity and ignore_bds_parents is not
> > needed any more.
> > 
> > This patch only ignores the 'ignore_bds_parents' parameter. It will be
> > removed in a separate patch.
> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > Reviewed-by: Hanna Reitz <hreitz@redhat.com>
> 
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> 
> 
> 
> 
> -- 
> Best regards,
> Vladimir
> 



  reply	other threads:[~2022-11-28 12:38 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-18 17:40 [PATCH v2 00/15] block: Simplify drain Kevin Wolf
2022-11-18 17:40 ` [PATCH v2 01/15] qed: Don't yield in bdrv_qed_co_drain_begin() Kevin Wolf
2022-11-18 17:40 ` [PATCH v2 02/15] test-bdrv-drain: Don't yield in .bdrv_co_drained_begin/end() Kevin Wolf
2022-11-18 17:40 ` [PATCH v2 03/15] block: Revert .bdrv_drained_begin/end to non-coroutine_fn Kevin Wolf
2022-11-18 17:40 ` [PATCH v2 04/15] block: Remove drained_end_counter Kevin Wolf
2022-11-18 17:41 ` [PATCH v2 05/15] block: Inline bdrv_drain_invoke() Kevin Wolf
2022-11-18 17:41 ` [PATCH v2 06/15] block: Fix locking for bdrv_reopen_queue_child() Kevin Wolf
2022-11-25 12:56   ` Vladimir Sementsov-Ogievskiy
2022-11-18 17:41 ` [PATCH v2 07/15] block: Drain invidual nodes during reopen Kevin Wolf
2022-11-25 13:10   ` Vladimir Sementsov-Ogievskiy
2022-11-18 17:41 ` [PATCH v2 08/15] block: Don't use subtree drains in bdrv_drop_intermediate() Kevin Wolf
2022-11-18 17:41 ` [PATCH v2 09/15] stream: Replace subtree drain with a single node drain Kevin Wolf
2022-11-18 17:41 ` [PATCH v2 10/15] block: Remove subtree drains Kevin Wolf
2022-11-18 17:41 ` [PATCH v2 11/15] block: Call drain callbacks only once Kevin Wolf
2022-11-25 14:59   ` Vladimir Sementsov-Ogievskiy
2022-11-28 12:37     ` Kevin Wolf [this message]
2022-11-18 17:41 ` [PATCH v2 12/15] block: Remove ignore_bds_parents parameter from drain_begin/end Kevin Wolf
2022-11-25 15:05   ` Vladimir Sementsov-Ogievskiy
2022-11-18 17:41 ` [PATCH v2 13/15] block: Drop out of coroutine in bdrv_do_drained_begin_quiesce() Kevin Wolf
2022-11-25 15:13   ` Vladimir Sementsov-Ogievskiy
2022-11-18 17:41 ` [PATCH v2 14/15] block: Don't poll in bdrv_replace_child_noperm() Kevin Wolf
2022-11-25 16:07   ` Vladimir Sementsov-Ogievskiy
2022-11-28 12:59     ` Kevin Wolf
2022-12-09 16:53   ` Paolo Bonzini
2022-12-12  9:09     ` Paolo Bonzini
2022-12-12 15:57     ` Kevin Wolf
2022-12-12 17:31       ` Paolo Bonzini
2022-11-18 17:41 ` [PATCH v2 15/15] block: Remove poll parameter from bdrv_parent_drained_begin_single() Kevin Wolf
2022-11-25 16:08   ` Vladimir Sementsov-Ogievskiy
2022-11-24 18:26 ` [PATCH v2 00/15] block: Simplify drain Hanna Reitz
2022-11-28 13:00 ` Kevin Wolf

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=Y4Sre2NtpGtuA9AF@redhat.com \
    --to=kwolf@redhat.com \
    --cc=eesposit@redhat.com \
    --cc=hreitz@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).