From mboxrd@z Thu Jan 1 00:00:00 1970 From: Rusty Russell Subject: Re: [PATCH, RFC] virtio_blk: add cache flush command Date: Wed, 13 May 2009 11:22:17 +0930 Message-ID: <200905131122.18221.rusty@rustcorp.com.au> References: <20090511083908.GB20082@lst.de> <200905122324.14970.rusty@rustcorp.com.au> <200905121618.36104.borntraeger@de.ibm.com> Mime-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Cc: Christoph Hellwig , kvm@vger.kernel.org To: Christian Borntraeger Return-path: Received: from ozlabs.org ([203.10.76.45]:41319 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752256AbZEMBwa (ORCPT ); Tue, 12 May 2009 21:52:30 -0400 In-Reply-To: <200905121618.36104.borntraeger@de.ibm.com> Content-Disposition: inline Sender: kvm-owner@vger.kernel.org List-ID: On Tue, 12 May 2009 11:48:36 pm Christian Borntraeger wrote: > Am Tuesday 12 May 2009 15:54:14 schrieb Rusty Russell: > > On Mon, 11 May 2009 06:09:08 pm Christoph Hellwig wrote: > > > Do we need a new feature flag for this command or can we expect that > > > all previous barrier support was buggy enough anyway? > > > > You mean reuse the VIRTIO_BLK_F_BARRIER for this as well? Seems fine. > > It is also used by kuli > (http://www.ibm.com/developerworks/linux/linux390/kuli.html) and kuli used > fdatasync. OK, that's sufficient for me. It's not like block is going to catch up with net for feature flags any time soon. Christoph, please make a new feature flag. Oh and FYI Christian, you might not have seen this patch: lguest: barrier me harder Impact: barrier correctness in example launcher I doubt either lguest user will complain about performance. Reported-by: Christoph Hellwig Cc: Jens Axboe Signed-off-by: Rusty Russell --- Documentation/lguest/lguest.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Documentation/lguest/lguest.c b/Documentation/lguest/lguest.c --- a/Documentation/lguest/lguest.c +++ b/Documentation/lguest/lguest.c @@ -1630,6 +1630,13 @@ static bool service_io(struct device *de } } + /* OK, so we noted that it was pretty poor to use an fdatasync as a + * barrier. But Christoph Hellwig points out that we need a sync + * *afterwards* as well: "Barriers specify no reordering to the front + * or the back." And Jens Axboe confirmed it, so here we are: */ + if (out->type & VIRTIO_BLK_T_BARRIER) + fdatasync(vblk->fd); + /* We can't trigger an IRQ, because we're not the Launcher. It does * that when we tell it we're done. */ add_used(dev->vq, head, wlen);