Re: [PATCH v6 0/7] fuse,virtiofs: support per-file DAX

[Vivek Goyal <vgoyal@redhat.com>]

On Wed, Oct 20, 2021 at 01:22:32PM +0800, JeffleXu wrote:
> 
> 
> On 10/18/21 11:21 PM, Vivek Goyal wrote:
> > On Mon, Oct 11, 2021 at 11:00:45AM +0800, Jeffle Xu wrote:
> >> changes since v5:
> >> Overall Design Changes:
> >> 1. virtiofsd now supports ioctl (only FS_IOC_SETFLAGS and
> >>   FS_IOC_FSSETXATTR), so that users inside guest could set/clear
> >>   persistent inode flags now. (FUSE kernel module has already supported
> >>   .ioctl(), virtiofsd need to suuport it.)
> > 
> > So no changes needed in fuse side (kernel) to support FS_IOC_FSSETXATTR?
> > Only virtiofsd needs to be changed. That sounds good.
> > 
> 
> Yes, the fuse kernel modules has already supported FUSE_IOCTL.
> 
> Per inode DAX on ext4/xfs will also call d_mark_dontcache() and try to
> evict this inode as soon as possible when the persistent (DAX) inode
> attribute has changed, just like [1].
> 
> But because of following reason:
> > 
> >> 2. The
> >>   algorithm used by virtiofsd to determine whether DAX shall be enabled
> >>   or not is totally implementation specific, and thus the following
> >>   scenario may exist: users inside guest has already set related persistent
> >>   inode flag (i.e. FS_XFLAG_DAX) on corresponding file but FUSE server finnaly
> >>   decides not to enable DAX for this file.
> 
> If we always call d_mark_dontcache() and try to evict this inode when
> the persistent (DAX) inode attribute has changed, the DAX state returned
> by virtiofsd may sustain the same, and thus the previous eviction is
> totally wasted and unnecessary.
> 
> So, as the following said,
> 
> >> Also because of this, d_mark_dontcache() is
> >>   not called when FS_IOC_SETFLAGS/FS_IOC_FSSETXATTR ioctl is done inside
> >>   guest. It's delayed to be done if the FUSE_ATTR_DAX flag **indeed**
> >>   changes (as showed in patch 6).
> 
> the call for d_mark_dontcache() and inode eviction is delayed when the
> DAX state returned by virtiofsd **indeed** changed (when dentry is timed
> out and a new FUSE_LOOKUP is requested). But the defect is that, if '-o
> cache=always' is set for virtiofsd, then the DAX state won't be updated
> for a long time, after users have changed the persistent (DAX) inode
> attribute inside guest via FS_IOC_FSSETXATTR ioctl.

Good point. I guess this probably is not too bad. If it becomes a concern,
we can always mark inode don't cache whenever client changes persistent
DAX flag.

Vivek
> 
> 
> 
> [1] https://www.spinics.net/lists/linux-fsdevel/msg200851.html
> 
> -- 
> Thanks,
> Jeffle
>