Re: [PATCH v2 4/4] fuse: support changing per-file DAX flag inside guest

[Vivek Goyal <vgoyal@redhat.com>]

On Fri, Jul 16, 2021 at 06:47:53PM +0800, Jeffle Xu wrote:
> Fuse client can enable or disable per-file DAX inside guest by
> chattr(1). Similarly the new state won't be updated until the file is
> closed and reopened later.
> 
> It is worth nothing that it is a best-effort style, since whether
> per-file DAX is enabled or not is controlled by fuse_attr.flags retrieved
> by FUSE LOOKUP routine, while the algorithm constructing fuse_attr.flags
> is totally fuse server specific, not to mention ioctl may not be
> supported by fuse server at all.
> 
> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
> ---
>  fs/fuse/ioctl.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/fuse/ioctl.c b/fs/fuse/ioctl.c
> index 546ea3d58fb4..172e05c3f038 100644
> --- a/fs/fuse/ioctl.c
> +++ b/fs/fuse/ioctl.c
> @@ -460,6 +460,7 @@ int fuse_fileattr_set(struct user_namespace *mnt_userns,
>  	struct fuse_file *ff;
>  	unsigned int flags = fa->flags;
>  	struct fsxattr xfa;
> +	bool newdax;
>  	int err;
>  
>  	ff = fuse_priv_ioctl_prepare(inode);
> @@ -467,10 +468,9 @@ int fuse_fileattr_set(struct user_namespace *mnt_userns,
>  		return PTR_ERR(ff);
>  
>  	if (fa->flags_valid) {
> +		newdax = flags & FS_DAX_FL;
>  		err = fuse_priv_ioctl(inode, ff, FS_IOC_SETFLAGS,
>  				      &flags, sizeof(flags));
> -		if (err)
> -			goto cleanup;
>  	} else {
>  		memset(&xfa, 0, sizeof(xfa));
>  		xfa.fsx_xflags = fa->fsx_xflags;
> @@ -479,11 +479,14 @@ int fuse_fileattr_set(struct user_namespace *mnt_userns,
>  		xfa.fsx_projid = fa->fsx_projid;
>  		xfa.fsx_cowextsize = fa->fsx_cowextsize;
>  
> +		newdax = fa->fsx_xflags & FS_XFLAG_DAX;
>  		err = fuse_priv_ioctl(inode, ff, FS_IOC_FSSETXATTR,
>  				      &xfa, sizeof(xfa));
>  	}
>  
> -cleanup:
> +	if (!err && IS_ENABLED(CONFIG_FUSE_DAX))
> +		fuse_dax_dontcache(inode, newdax);

This assumes that server will set ATTR_DAX flag for inode based on
whether inode has FS_DAX_FL set or not.

So that means server first will have to know that client has DAX enabled
so that it can query FS_DAX_FL. And in current framework we don't have
a way for server to know if client is using DAX or not.

I think there is little disconnect here. So either client should be
checking FS_DAX_FL flag on inode. But we probably don't want to pay
extra round trip cost for this. 

That means somehow server should return this information as part of
inode attrs only if client wants this extra file attr informaiton. So
may be GETATTR should be enhanced instead to return file attr information
too if client asked for it?

I have not looked what it takes to implement this. If this is too 
complicated, then alternate approach will be that it is up to the
server to decide what inodes should use DAX and there is no guarantee
that server will make sue of FS_DAX_FL flag. fuse will still support
setting FS_DAX_FL but server could choose to not use it at all. In
that case fuse client will not have to query file attrs in GETATTR
and just rely on ATTR_DAX flag set by server. I think that's what
you are implementing.  If that's the case then dontcache does not make
much sense because you don't even know if server is looking at
FS_DAX_FL to decide whether DAX should be used or not.

Thanks
Vivek

> +
>  	fuse_priv_ioctl_cleanup(inode, ff);
>  
>  	return err;
> -- 
> 2.27.0
> 

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization