[Virtio-fs] [PATCH 6/9] virtio-fs: let dax style override directIO style when dax+cache=none

[Liu Bo]

On Wed, Apr 17, 2019 at 10:25:53AM +0200, Miklos Szeredi wrote:
> On Tue, Apr 16, 2019 at 9:38 PM Vivek Goyal <vgoyal at redhat.com> wrote:
> >
> > On Wed, Apr 17, 2019 at 02:03:19AM +0800, Liu Bo wrote:
> > > In case of dax+cache=none, mmap uses dax style prior to directIO style,
> > > while read/write don't, but it seems that there is no reason not to do so.
> > >
> > > Signed-off-by: Liu Bo <bo.liu at linux.alibaba.com>
> > > Reviewed-by: Joseph Qi <joseph.qi at linux.alibaba.com>
> >
> > This is interesting. I was thinking about it today itself. I noticed
> > that ext4 and xfs also check for DAX inode first and use dax path
> > if dax is enabled.
> >
> > cache=never sets FOPEN_DIRECT_IO (even if application never asked for
> > direct IO). If dax is enabled, for data its equivalent to doing direct
> > IO. And for mmap() we are already checking for DAX first. So it makes
> > sense to do same thing for read/write path as well.
> >
> > CCing Miklos as well. He might have some thougts on this. I am curios
> > that initially whey did he make this change only for mmap() and not
> > for read/write paths.
> 
> AFAIR the main reason was that we had performance issues with size
> extending writes with dax.
> 
> There is also the question of mtime updates and atime updates, which
> are handled properly with FOPEN_DIRECT_IO, but not with DAX I/O.

Looks like fuse_iget() has set inode with NOATIME and NOCMTIME under
the case "!fc->writeback_cache", which is true in dax mode, do we
still case about time update?

AFAICS, these metadata are maintained by host side.

thanks,
-liubo

> 
> If we solve those issues then I think it makes sense to switch
> read/write to using DAX.
> 
> Does that answer your question?
> 
> Thanks,
> Miklos
> 
> >
> > Thanks
> > Vivek
> >
> > > ---
> > >  fs/fuse/file.c | 11 ++++++-----
> > >  1 file changed, 6 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > > index c6090f5..620326e 100644
> > > --- a/fs/fuse/file.c
> > > +++ b/fs/fuse/file.c
> > > @@ -1158,12 +1158,12 @@ static ssize_t fuse_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
> > >       struct inode *inode = file->f_mapping->host;
> > >       struct fuse_conn *fc = get_fuse_conn(inode);
> > >
> > > -     if (ff->open_flags & FOPEN_DIRECT_IO)
> > > -             return fuse_direct_read_iter(iocb, to);
> > > -
> > >       if (IS_DAX(inode))
> > >               return fuse_dax_read_iter(iocb, to);
> > >
> > > +     if (ff->open_flags & FOPEN_DIRECT_IO)
> > > +             return fuse_direct_read_iter(iocb, to);
> > > +
> > >       /*
> > >        * In auto invalidate mode, always update attributes on read.
> > >        * Otherwise, only update if we attempt to read past EOF (to ensure
> > > @@ -1426,11 +1426,12 @@ static ssize_t fuse_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
> > >       ssize_t err;
> > >       loff_t endbyte = 0;
> > >
> > > -     if (ff->open_flags & FOPEN_DIRECT_IO)
> > > -             return fuse_direct_write_iter(iocb, from);
> > >       if (IS_DAX(inode))
> > >               return fuse_dax_write_iter(iocb, from);
> > >
> > > +     if (ff->open_flags & FOPEN_DIRECT_IO)
> > > +             return fuse_direct_write_iter(iocb, from);
> > > +
> > >       if (get_fuse_conn(inode)->writeback_cache) {
> > >               /* Update size (EOF optimization) and mode (SUID clearing) */
> > >               err = fuse_update_attributes(mapping->host, file);
> > > --
> > > 1.8.3.1
> > >
> > > _______________________________________________
> > > Virtio-fs mailing list
> > > Virtio-fs at redhat.com
> > > https://www.redhat.com/mailman/listinfo/virtio-fs