From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:56421)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1bFHUl-0001tp-VO
	for qemu-devel@nongnu.org; Tue, 21 Jun 2016 04:57:25 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1bFHUi-0002g2-2w
	for qemu-devel@nongnu.org; Tue, 21 Jun 2016 04:57:19 -0400
Date: Tue, 21 Jun 2016 10:57:04 +0200
From: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20160621085704.GB4520@noname.redhat.com>
References: <1466435958-308-1-git-send-email-den@openvz.org>
	<45cce6cb-af87-9e2f-eeb2-4d4f258a3dc9@redhat.com>
	<20160621074507.GA4520@noname.redhat.com>
	<5768F811.5080105@openvz.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <5768F811.5080105@openvz.org>
Subject: Re: [Qemu-devel] [PATCH 1/1] block: ignore flush requests when
 storage is clean
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: "Denis V. Lunev" <den@openvz.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>, qemu-block@nongnu.org, qemu-devel@nongnu.org, Fam Zheng <famz@redhat.com>, Evgeny Yakovlev <eyakovlev@virtuozzo.com>, Max Reitz <mreitz@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>

Am 21.06.2016 um 10:17 hat Denis V. Lunev geschrieben:
> On 06/21/2016 10:45 AM, Kevin Wolf wrote:
> >Am 21.06.2016 um 09:32 hat Paolo Bonzini geschrieben:
> >>
> >>On 20/06/2016 17:19, Denis V. Lunev wrote:
> >>>+    /* Check if storage is actually dirty before flushing to disk */
> >>>+    if (!bs->dirty) {
> >>>+        goto flush_parent;
> >>>+    }
> >>>+    bs->dirty = false;
> >>>+
> >>This should be cleared after the flush is complete.  If you have
> >>
> >>     write begin
> >>     write end
> >>     flush #1 begin
> >>     flush #2 begin
> >>
> >>Then the second flush must only return after the first has finished.
> >I think clearing bs->dirty after the flush completion wouldn't
> >necessarily be right either if there are concurrent writes in flight, as
> >only completed writes are guaranteed to be flushed by it.
> >
> >Kevin
> this is not a problem if flush 2 will return after flush 1.
> This will mean that all writes prior to both flushes
> will land to the disk.

We need to be careful in cases like this:

start + complete write #1
start flush #1
start + complete write #2
start flush #2
complete flush #1
complete flush #2

Here letting flush #2 wait for flush #1 isn't enough because that one
only guarantees to flush write #1, but not write #2. However, flush #2
is required to flush write #2, too.

> Keeping this in mind dirty should be cleared before
> flush operation start.

Yes, if you do it like this, so that flush #2 ends up being an actual
flush instead of just waiting for flush #1, we're good.

Kevin