Re: [PATCH, RFC] virtio_blk: add cache flush command

Rusty Russell
2009-05-13T01:52:30Z

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 <hch@infradead.org>
Cc: Jens Axboe <jens.axboe@oracle.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
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);