Re: [Qemu-devel] [PATCH 1/2] hw/9pfs: Add new virtfs option cache=writethrough to skip host page cache

[Stefan Hajnoczi]

On Sun, Oct 9, 2011 at 7:35 PM, Aneesh Kumar K.V
<address@hidden> wrote:
> On Sun, 9 Oct 2011 17:16:50 +0100, Stefan Hajnoczi <address@hidden> wrote:
>> On Sun, Oct 9, 2011 at 4:34 PM, Aneesh Kumar K.V
>> <address@hidden> wrote:
>> > On Sat, 8 Oct 2011 12:24:37 +0100, Stefan Hajnoczi <address@hidden> wrote:
>> >> On Fri, Oct 7, 2011 at 7:46 AM, Aneesh Kumar K.V
>> >> <address@hidden> wrote:
>> >> > cache=writethrough implies the file are opened in the host with O_SYNC 
>> >> > open flag
>> >> >
>> >> > Signed-off-by: Aneesh Kumar K.V <address@hidden>
>> >> > ---
>> >> >  fsdev/file-op-9p.h         |    1 +
>> >> >  fsdev/qemu-fsdev.c         |   10 ++++++++--
>> >> >  fsdev/qemu-fsdev.h         |    2 ++
>> >> >  hw/9pfs/virtio-9p-device.c |    5 +++++
>> >> >  hw/9pfs/virtio-9p.c        |   24 ++++++++++++++++++------
>> >> >  qemu-config.c              |    6 ++++++
>> >> >  qemu-options.hx            |   17 ++++++++++++-----
>> >> >  vl.c                       |    6 ++++++
>> >> >  8 files changed, 58 insertions(+), 13 deletions(-)
>> >>
>> >> When would this be used?  For serving up vanilla 9P?
>> >>
>> >> I think 9P.u and 9P.l have support for fsync(2) while vanilla 9P does not.
>> >>
>> >
>> > TFSYNC is added by 9p.L. So we would need this for 9p.u.
>>
>> I think 9p.u is covered by this wstat hack in
>> http://ericvh.github.com/9p-rfc/rfc9p2000.html#anchor32:
>>
>> "if all the elements of the directory entry in a Twstat message are
>> ``don't touch'' val- ues, the server may interpret it as a request to
>> guarantee that the contents of the associated file are committed to
>> stable storage before the Rwstat message is returned."
>>
>> A real TFSYNC operation is nicer though and could be mandatory (the
>> 9P2000 RFC only says "the server *may*").
>>
>> > Another use
>> > case is to ensure that we don't leave pages on host as dirty. That would
>> > ensure that large writeback from a guest don't result in large number of
>> > dirty pages on the host, thereby resulting in writeback in the host. It
>> > would be needed for predictable I/O behavior in a setup where we have
>> > multiple guest.
>>
>> I see.  I'm mostly curious about this change because the caching modes
>> are a nightmare with block devices - a lot of time is spent discussing
>> and benchmarking them, and they cause confusion when configuring KVM.
>>
>> It sounds like O_SYNC is being used in order to keep page cache clean.
>>  But keeping the page cache clean is a side-effect of O_SYNC's
>> behavior: writing out each page and synchronizing the disk write
>> cache.  If you are attempting to bypass the page cache, just use
>> O_DIRECT without O_SYNC.
>
> But how about reads. I want to make sure i get to use the page cache for
> reads and also want to keep the page cache clean.
>
>>  O_SYNC is doing the additional disk write
>> cache synchronization which will slow down I/O and prevent the server
>> from using disk write cache.  O_SYNC is not the right flag to use.
>
> O_DIRECT have additional requirement on buffer alignment, and we don't
> want to send every read to disk.  VirtFS also support zero copy
> read/write, so that buffer alignment will always not be possible.
> We want to make sure writes don't leave the page cache dirty so
> that host doesn't spent much time in write back of data dirtied by the guest.

sync_file_range(2) kicks off the write-out but does not flush file
system metadata or synchronize the disk write cache.

Stefan