Re: [PATCH v15 9/9] fuse: auto-invalidate inode attributes in passthrough mode

[Sweet Tea Dorminy <sweettea-kernel@dorminy.me>]

On 2/6/24 09:24, Amir Goldstein wrote:
> After passthrough read/write, we invalidate a/c/mtime/size attributes
> if the backing inode attributes differ from FUSE inode attributes.
> 
> Do the same in fuse_getattr() and after detach of backing inode, so that
> passthrough mmap read/write changes to a/c/mtime/size attribute of the
> backing inode will be propagated to the FUSE inode.
> 
> The rules of invalidating a/c/mtime/size attributes with writeback cache
> are more complicated, so for now, writeback cache and passthrough cannot
> be enabled on the same filesystem.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>   fs/fuse/dir.c         |  4 ++++
>   fs/fuse/fuse_i.h      |  2 ++
>   fs/fuse/inode.c       |  4 ++++
>   fs/fuse/iomode.c      |  5 +++-
>   fs/fuse/passthrough.c | 55 ++++++++++++++++++++++++++++++++++++-------
>   5 files changed, 61 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index 95330c2ca3d8..7f9d002b8f23 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -2118,6 +2118,10 @@ static int fuse_getattr(struct mnt_idmap *idmap,
>   		return -EACCES;
>   	}
>   
> +	/* Maybe update/invalidate attributes from backing inode */
> +	if (fuse_inode_backing(get_fuse_inode(inode)))
> +		fuse_backing_update_attr_mask(inode, request_mask);
> +
>   	return fuse_update_get_attr(inode, NULL, stat, request_mask, flags);
>   }
>   
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 98f878a52af1..4b011d31012f 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -1456,6 +1456,8 @@ void fuse_backing_files_init(struct fuse_conn *fc);
>   void fuse_backing_files_free(struct fuse_conn *fc);
>   int fuse_backing_open(struct fuse_conn *fc, struct fuse_backing_map *map);
>   int fuse_backing_close(struct fuse_conn *fc, int backing_id);
> +void fuse_backing_update_attr(struct inode *inode, struct fuse_backing *fb);
> +void fuse_backing_update_attr_mask(struct inode *inode, u32 request_mask);
>   
>   struct fuse_backing *fuse_passthrough_open(struct file *file,
>   					   struct inode *inode,
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index c26a84439934..c68f005b6e86 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -1302,9 +1302,13 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
>   			 * on a stacked fs (e.g. overlayfs) themselves and with
>   			 * max_stack_depth == 1, FUSE fs can be stacked as the
>   			 * underlying fs of a stacked fs (e.g. overlayfs).
> +			 *
> +			 * For now, writeback cache and passthrough cannot be
> +			 * enabled on the same filesystem.
>   			 */
>   			if (IS_ENABLED(CONFIG_FUSE_PASSTHROUGH) &&
>   			    (flags & FUSE_PASSTHROUGH) &&
> +			    !fc->writeback_cache &&
>   			    arg->max_stack_depth > 0 &&
>   			    arg->max_stack_depth <= FILESYSTEM_MAX_STACK_DEPTH) {
>   				fc->passthrough = 1;
> diff --git a/fs/fuse/iomode.c b/fs/fuse/iomode.c
> index c545058a01e1..96eb311fe7bd 100644
> --- a/fs/fuse/iomode.c
> +++ b/fs/fuse/iomode.c
> @@ -157,8 +157,11 @@ void fuse_file_uncached_io_end(struct inode *inode)
>   	spin_unlock(&fi->lock);
>   	if (!uncached_io)
>   		wake_up(&fi->direct_io_waitq);
> -	if (oldfb)
> +	if (oldfb) {
> +		/* Maybe update attributes after detaching backing inode */
> +		fuse_backing_update_attr(inode, oldfb);
>   		fuse_backing_put(oldfb);
> +	}
>   }
>   
>   /*
> diff --git a/fs/fuse/passthrough.c b/fs/fuse/passthrough.c
> index 260e76fc72d5..c1bb80a6e536 100644
> --- a/fs/fuse/passthrough.c
> +++ b/fs/fuse/passthrough.c
> @@ -11,11 +11,8 @@
>   #include <linux/backing-file.h>
>   #include <linux/splice.h>
>   
> -static void fuse_file_accessed(struct file *file)
> +static void fuse_backing_accessed(struct inode *inode, struct fuse_backing *fb)
>   {
> -	struct inode *inode = file_inode(file);
> -	struct fuse_inode *fi = get_fuse_inode(inode);
> -	struct fuse_backing *fb = fuse_inode_backing(fi);
>   	struct inode *backing_inode = file_inode(fb->file);
>   	struct timespec64 atime = inode_get_atime(inode);
>   	struct timespec64 batime = inode_get_atime(backing_inode);
> @@ -25,11 +22,8 @@ static void fuse_file_accessed(struct file *file)
>   		fuse_invalidate_atime(inode);
>   }
>   
> -static void fuse_file_modified(struct file *file)
> +static void fuse_backing_modified(struct inode *inode, struct fuse_backing *fb)
>   {
> -	struct inode *inode = file_inode(file);
> -	struct fuse_inode *fi = get_fuse_inode(inode);
> -	struct fuse_backing *fb = fuse_inode_backing(fi);
>   	struct inode *backing_inode = file_inode(fb->file);
>   	struct timespec64 ctime = inode_get_ctime(inode);
>   	struct timespec64 mtime = inode_get_mtime(inode);
> @@ -42,6 +36,51 @@ static void fuse_file_modified(struct file *file)
>   		fuse_invalidate_attr_mask(inode, FUSE_STATX_MODSIZE);
>   }
>   
> +/* Called from fuse_file_uncached_io_end() after detach of backing inode */
> +void fuse_backing_update_attr(struct inode *inode, struct fuse_backing *fb)
> +{
> +	fuse_backing_modified(inode, fb);
> +	fuse_backing_accessed(inode, fb);
> +}
> +
> +/* Called from fuse_getattr() - may race with detach of backing inode */
> +void fuse_backing_update_attr_mask(struct inode *inode, u32 request_mask)
> +{
> +	struct fuse_inode *fi = get_fuse_inode(inode);
> +	struct fuse_backing *fb;
> +
> +	rcu_read_lock();
> +	fb = fuse_backing_get(fuse_inode_backing(fi));
> +	rcu_read_unlock();
> +	if (!fb)
> +		return;
> +
> +	if (request_mask & FUSE_STATX_MODSIZE)
> +		fuse_backing_modified(inode, fb);
> +	if (request_mask & STATX_ATIME)
> +		fuse_backing_accessed(inode, fb);
> +
> +	fuse_backing_put(fb);
> +}
> +
> +static void fuse_file_accessed(struct file *file)
> +{
> +	struct inode *inode = file_inode(file);
> +	struct fuse_inode *fi = get_fuse_inode(inode);
> +	struct fuse_backing *fb = fuse_inode_backing(fi);
> +
> +	fuse_backing_accessed(inode, fb);
> +}
> +
> +static void fuse_file_modified(struct file *file)
> +{
> +	struct inode *inode = file_inode(file);
> +	struct fuse_inode *fi = get_fuse_inode(inode);
> +	struct fuse_backing *fb = fuse_inode_backing(fi);
> +
> +	fuse_backing_modified(inode, fb);
> +}
> +
>   ssize_t fuse_passthrough_read_iter(struct kiocb *iocb, struct iov_iter *iter)
>   {
>   	struct file *file = iocb->ki_filp;

I noticed this patch doesn't seem to have made it into 6.9 like the rest 
of these passthrough patches -- may I ask why it didn't make it? I think 
it still makes sense but I might be missing some change between what's 
in 6.9 and this version of the patches.

Thanks!

Sweet Tea