All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-block@nongnu.org, famz@redhat.com, qemu-devel@nongnu.org,
	stefanha@redhat.com
Subject: Re: [Qemu-devel] [PATCH 1/3] block: add BDS field to count in-flight requests
Date: Wed, 12 Oct 2016 11:50:27 +0200	[thread overview]
Message-ID: <20161012095027.GE5544@noname.redhat.com> (raw)
In-Reply-To: <a8c9f31a-f2c5-adf8-8e29-47871c9b6e67@redhat.com>

Am 11.10.2016 um 18:45 hat Paolo Bonzini geschrieben:
> > I think my point was that you don't have to count requests at the BB
> > level if you know that there are no requests pending on the BB level
> > that haven't reached the BDS level yet.
> 
> I need to count requests at the BB level because the blk_aio_*
> operations have a separate bottom half that is invoked if either 1) they
> never reach BDS (because of an error); or 2) the bdrv_co_* call
> completes without yielding.  The count must be >0 when blk_aio_*
> returns, or bdrv_drain (and thus blk_drain) won't loop.  Because
> bdrv_drain and blk_drain are conflated, the counter must be the BDS one.

Okay, makes sense.

> In turn, the BDS counter is needed because of the lack of isolated
> sections.  The right design would be for blk_isolate_begin to call
> blk_drain on *other* BlockBackends reachable in a child-to-parent visit.

Not really the blk_drain() that completes all pending requests of the
BB, but the BdrvChild callbacks that quiesce the BB. But I think we
really agree here and are just having trouble with the terminology.

>  Instead, until that is implemented, we have bdrv_drained_begin that
> emulates that through the same-named callback, followed by a
> parent-to-child bdrv_drain that is almost always unnecessary.

Yes.

> (By the way, I need to repost this series anyway, but let's finish the
> discussion first to understand what you'd like to have in 2.8).

I'm still not completely sold on the order in which we should do things,
but you've been insisting enough that I'll just trust you on this.

Kevin

  reply	other threads:[~2016-10-12  9:50 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-07 16:19 [Qemu-devel] [PATCH 0/3] block: new bdrv_drain implementation Paolo Bonzini
2016-10-07 16:19 ` [Qemu-devel] [PATCH 1/3] block: add BDS field to count in-flight requests Paolo Bonzini
2016-10-10 10:36   ` Kevin Wolf
2016-10-10 16:37     ` Paolo Bonzini
2016-10-11 11:00       ` Kevin Wolf
2016-10-11 14:09         ` Paolo Bonzini
2016-10-11 15:25           ` Kevin Wolf
2016-10-11 16:45             ` Paolo Bonzini
2016-10-12  9:50               ` Kevin Wolf [this message]
2016-10-12 10:13                 ` Paolo Bonzini
2016-10-07 16:19 ` [Qemu-devel] [PATCH 2/3] block: change drain to look only at one child at a time Paolo Bonzini
2016-10-10 11:17   ` Kevin Wolf
2016-10-07 16:19 ` [Qemu-devel] [PATCH 3/3] qed: Implement .bdrv_drain Paolo Bonzini
2016-10-11 16:46 ` [Qemu-devel] [PATCH 0/3] block: new bdrv_drain implementation no-reply

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=20161012095027.GE5544@noname.redhat.com \
    --to=kwolf@redhat.com \
    --cc=famz@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.