qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Fam Zheng <famz@redhat.com>
Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, jcody@redhat.com,
	Max Reitz <mreitz@redhat.com>,
	pbonzini@redhat.com, Stefan Hajnoczi <stefanha@redhat.com>
Subject: Re: [Qemu-devel] [PATCH RFC 0/9] block: Rewrite block drain begin/end
Date: Wed, 29 Nov 2017 18:25:46 +0100	[thread overview]
Message-ID: <20171129172546.GG3753@localhost.localdomain> (raw)
In-Reply-To: <20171129144956.11409-1-famz@redhat.com>

Am 29.11.2017 um 15:49 hat Fam Zheng geschrieben:
> While we look at the fixes for 2.11, I briefly prototyped this series
> to see if it makes sense as a simplification of the drain API for
> 2.12.
> 
> The idea is to let AioContext manage quiesce callbacks, then the block
> layer only needs to do the in-flight request waiting. This lets us get
> rid of the callback recursion (both up and down).

So essentially you don't drain individual nodes any more, but whole
AioContexts. I have a feeeling that this would be a step in the wrong
direction.

Not only would it completely bypass the path I/O requests take and
potentially drain a lot more than is actually necessary, but it also
requires that all nodes that are connected in a tree are in the same
AioContext.

Paolo can say more on this, but my understanding was that the long-term
plan is to make the block layer fully thread safe so that any thread
could call things on any node. I remember Paolo saying that AioContext
was even fully going away in the future. I don't see how this is
compatible with your approach.

And finally, I don't really think that the recursion is even a problem.
The problem is with graph changes made by callbacks that drain allows to
run. With your changes, it might be a bit easier to avoid that
bdrv_drain() itself gets into trouble due to graph changes, but this
doesn't solve the problem for any (possibly indirect) callers of
bdrv_drain().

Kevin

  parent reply	other threads:[~2017-11-29 17:25 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-29 14:49 [Qemu-devel] [PATCH RFC 0/9] block: Rewrite block drain begin/end Fam Zheng
2017-11-29 14:49 ` [Qemu-devel] [PATCH RFC 1/9] block: Remove unused bdrv_requests_pending Fam Zheng
2017-12-01 12:35   ` [Qemu-devel] [Qemu-block] " Alberto Garcia
2017-11-29 14:49 ` [Qemu-devel] [PATCH RFC 2/9] aio: Add drain begin/end API to AioContext Fam Zheng
2017-11-30 16:07   ` Stefan Hajnoczi
2017-11-29 14:49 ` [Qemu-devel] [PATCH RFC 3/9] blockjob: Implement AioContext drain ops Fam Zheng
2017-11-30 16:09   ` Stefan Hajnoczi
2017-11-29 14:49 ` [Qemu-devel] [PATCH RFC 4/9] throttle: " Fam Zheng
2017-11-29 14:49 ` [Qemu-devel] [PATCH RFC 5/9] qed: " Fam Zheng
2017-11-29 14:49 ` [Qemu-devel] [PATCH RFC 6/9] block: Use aio_context_drained_begin in bdrv_set_aio_context Fam Zheng
2017-11-29 14:49 ` [Qemu-devel] [PATCH RFC 7/9] block: Switch to use AIO drained begin/end API Fam Zheng
2017-11-29 14:49 ` [Qemu-devel] [PATCH RFC 8/9] block: Drop old drained_{begin, end} callbacks Fam Zheng
2017-11-29 14:49 ` [Qemu-devel] [PATCH RFC 9/9] blockjob: Drop unused functions Fam Zheng
2017-11-29 17:25 ` Kevin Wolf [this message]
2017-11-30  2:03   ` [Qemu-devel] [PATCH RFC 0/9] block: Rewrite block drain begin/end Fam Zheng
2017-11-30 10:31     ` Kevin Wolf
2017-11-30 14:34       ` Fam Zheng
2017-11-30 15:10         ` Kevin Wolf
2017-11-30 16:04           ` Paolo Bonzini
2017-12-01  9:51             ` Fam Zheng
2017-12-01 12:24               ` 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=20171129172546.GG3753@localhost.localdomain \
    --to=kwolf@redhat.com \
    --cc=famz@redhat.com \
    --cc=jcody@redhat.com \
    --cc=mreitz@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 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).