Re: [Qemu-devel] [PATCH] 9pfs: don't ignore O_DIRECT flag in the 9pfs server

[Greg Kurz]

On Tue, 21 Nov 2017 11:28:29 +0800
jiangyiwen <address@hidden> wrote:

> On 2017/11/20 18:13, Greg Kurz wrote:
> > On Mon, 20 Nov 2017 13:48:59 +0800
> > jiangyiwen <address@hidden> wrote:
> >   
> >> Now v9fs in linux has already supported O_DIRECT(v9fs_direct_IO),
> >> when guest user open file with O_DIRECT flag and return success,
> >> so user hopes data doesn't pass through page cache, but 9pfs in
> >> qemu ignore direct disk access and use host page cache, it is not
> >> match to DIRECT_IO semantic, so we should not ignore O_DIRECT in
> >> 9pfs unless v9fs in linux don't support DIRECT_IO.
> >>
> >> And if server fs don't support O_DIRECT, user will receive -EINVAL
> >> and know the filesystem don't support O_DIRECT.
> >>
> >> So in order to ensure semantic consistency, don't ignore direct
> >> disk access in 9pfs.
> >>  
> > 
> > There are good reasons for us to ignore O_DIRECT. AFAIK, O_DIRECT requires
> > the application to take care of the alignment of the buffers with either the
> > logical block size of the filesystem (linux <= 2.4) or the logical block 
> > size
> > of the underlying storage... I don't really see how you can achieve that 
> > since
> > the linux 9p client only cares to break up requests into msize-chunks, and
> > ignores alignment. ie, you're likely to not ensure semantic consistency on 
> > the
> > host side anyway and linux will fallback to cached I/O.
> > 
> > Also, this change would silently break existing users of O_DIRECT, which 
> > isn't
> > acceptable... it would require some fsdev property to enable this new 
> > behavior.
> >   
> 
> Thank you very much, I understand what your mean, v9fs in linux doesn't take 
> care
> of the alignment, only split as the msize. Then there is another problem,
> should v9fs support DIRECT_IO? or delete DIRECT_IO ops in v9fs?
> 

The linux open(2) manual page says:

"O_DIRECT (since Linux 2.4.10)
        Try to minimize cache effects of the I/O to and from this  file."

ie, it doesn't ensure you necessarily skip any cache that may exist between
the application and the ultimate storage. The manual page also elaborates
how direct I/O behaves with NFS:

"The  behavior  of O_DIRECT with NFS will differ from local filesystems.  Older
 kernels, or kernels configured in certain ways, may not support this 
combination.
 The NFS protocol does not support passing the flag to the server, so O_DIRECT 
I/O
 will bypass the page cache only on the client; the server may still cache the 
I/O.
 The client asks the server to make the I/O synchronous to preserve the 
synchronous
 semantics of O_DIRECT.  Some servers will perform poorly under these 
circumstances,
 especially if the I/O size is small.  Some servers may also be configured to 
lie to
 clients about the I/O having reached stable storage; this will avoid the 
performance
 penalty at some risk to data integrity in the event of server power failure. 
The
 Linux NFS client places no alignment restrictions on O_DIRECT I/O."

So, Direct I/O in v9fs basically means the page cache is skipped in the client,
which looks acceptable IMHO... And, again, removing this may break existing
setups, so I'm not sure we should do that.

Cheers,

--
Greg

> Thanks,
> Yiwen.
> >> Signed-off-by: Yiwen Jiang <address@hidden>
> >> ---
> >>  hw/9pfs/9p.c | 4 ----
> >>  1 file changed, 4 deletions(-)
> >>
> >> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> >> index 52d4663..5ea01c4 100644
> >> --- a/hw/9pfs/9p.c
> >> +++ b/hw/9pfs/9p.c
> >> @@ -155,10 +155,6 @@ static int get_dotl_openflags(V9fsState *s, int 
> >> oflags)
> >>       */
> >>      flags = dotl_to_open_flags(oflags);
> >>      flags &= ~(O_NOCTTY | O_ASYNC | O_CREAT);
> >> -    /*
> >> -     * Ignore direct disk access hint until the server supports it.
> >> -     */
> >> -    flags &= ~O_DIRECT;
> >>      return flags;
> >>  }
> >>  
> > 
> > 
> > .
> >   
> 
>