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

[Stefan Hajnoczi]

On Wed, Oct 12, 2011 at 2:19 PM, Aneesh Kumar K.V
<address@hidden> wrote:
> On Wed, 12 Oct 2011 10:55:21 +0100, Stefan Hajnoczi <address@hidden> wrote:
>> On Mon, Oct 10, 2011 at 10:06 AM, Aneesh Kumar K.V
>> <address@hidden> wrote:
>> > diff --git a/hw/9pfs/virtio-9p-handle.c b/hw/9pfs/virtio-9p-handle.c
>> > index 5c8b5ed..441a37f 100644
>> > --- a/hw/9pfs/virtio-9p-handle.c
>> > +++ b/hw/9pfs/virtio-9p-handle.c
>> > @@ -202,6 +202,15 @@ static ssize_t handle_pwritev(FsContext *ctx, int fd, 
>> > const struct iovec *iov,
>> >         return writev(fd, iov, iovcnt);
>>
>> The sync_file_range(2) call below is dead-code since we'll return
>> immediately after writev(2) completes.  The writev(2) return value
>> needs to be saved temporarily and then returned after
>> sync_file_range(2).
>
> Missed that. Will fix in the next update
>
>>
>> >     }
>> >  #endif
>> > +    if (ctx->cache_flags & V9FS_WRITETHROUGH_CACHE) {
>>
>> -drive cache=writethrough means something different from 9pfs
>> "writethrough".  This is confusing so I wonder if there is a better
>> name like immediate write-out.
>>
>
> cache=immediate-write-out ?
>
>> > +        /*
>> > +         * Initiate a writeback. This is not a data integrity sync.
>> > +         * We want to ensure that we don't leave dirty pages in the cache
>> > +         * after write when cache=writethrough is sepcified.
>> > +         */
>> > +        sync_file_range(fd, offset, 0,
>> > +                        SYNC_FILE_RANGE_WAIT_BEFORE | 
>> > SYNC_FILE_RANGE_WRITE);
>> > +    }
>>
>> I'm not sure whether SYNC_FILE_RANGE_WAIT_BEFORE is necessary.  As a
>> best-effort mechanism just SYNC_FILE_RANGE_WRITE does the job although
>> a client that rapidly rewrites may be able to leave dirty pages in the
>> host page cache
>
> Shouldn't we prevent this ? That is the reason for me to use that
> WAIT_BEFORE ?

The flag will cause sync_file_range(2) to wait on in-flight I/O.  The
guest will notice slow I/O.  You should at least specify a range
instead of nbytes=0 in the arguments.

Stefan