Re: [PATCH 1/2] fuse: fix COPY_FILE_RANGE interface

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Aug 06, 2025 at 10:17:06AM +0100, Luis Henriques wrote:
> On Tue, Aug 05 2025, Miklos Szeredi wrote:
> 
> > The FUSE protocol uses struct fuse_write_out to convey the return value of
> > copy_file_range, which is restricted to uint32_t.  But the COPY_FILE_RANGE
> > interface supports a 64-bit size copies.
> >
> > Currently the number of bytes copied is silently truncated to 32-bit, which
> > is unfortunate at best.
> >
> > Introduce a new op COPY_FILE_RANGE_64, which is identical, except the
> > number of bytes copied is returned in a 64-bit value.
> >
> > If the fuse server does not support COPY_FILE_RANGE_64, fall back to
> > COPY_FILE_RANGE and truncate the size to UINT_MAX - 4096.
> 
> I was wondering if it wouldn't make more sense to truncate the size to
> MAX_RW_COUNT instead.  My reasoning is that, if I understand the code
> correctly (which is probably a big 'if'!), the VFS will fallback to
> splice() if the file system does not implement copy_file_range.  And in
> this case splice() seems to limit the operation to MAX_RW_COUNT.

It doesn't, because copy_file_range implementations can do other things
(like remapping/reflinking file blocks) that produce a very small amount
of disk IO for what is effectively a very large change to file contents.
That's why the VFS doesn't cap len at MAX_RW_COUNT bytes.

--D

> Cheers,
> -- 
> Luís
> 
> 
> > Reported-by: Florian Weimer <fweimer@redhat.com>
> > Closes: https://lore.kernel.org/all/lhuh5ynl8z5.fsf@oldenburg.str.redhat.com/
> > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> > ---
> >  fs/fuse/file.c            | 34 ++++++++++++++++++++++++++--------
> >  fs/fuse/fuse_i.h          |  3 +++
> >  include/uapi/linux/fuse.h | 12 +++++++++++-
> >  3 files changed, 40 insertions(+), 9 deletions(-)
> >
> > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > index adc4aa6810f5..bd6624885855 100644
> > --- a/fs/fuse/file.c
> > +++ b/fs/fuse/file.c
> > @@ -3017,6 +3017,8 @@ static ssize_t __fuse_copy_file_range(struct file *file_in, loff_t pos_in,
> >  		.flags = flags
> >  	};
> >  	struct fuse_write_out outarg;
> > +	struct fuse_copy_file_range_out outarg_64;
> > +	u64 bytes_copied;
> >  	ssize_t err;
> >  	/* mark unstable when write-back is not used, and file_out gets
> >  	 * extended */
> > @@ -3066,30 +3068,46 @@ static ssize_t __fuse_copy_file_range(struct file *file_in, loff_t pos_in,
> >  	if (is_unstable)
> >  		set_bit(FUSE_I_SIZE_UNSTABLE, &fi_out->state);
> >  
> > -	args.opcode = FUSE_COPY_FILE_RANGE;
> > +	args.opcode = FUSE_COPY_FILE_RANGE_64;
> >  	args.nodeid = ff_in->nodeid;
> >  	args.in_numargs = 1;
> >  	args.in_args[0].size = sizeof(inarg);
> >  	args.in_args[0].value = &inarg;
> >  	args.out_numargs = 1;
> > -	args.out_args[0].size = sizeof(outarg);
> > -	args.out_args[0].value = &outarg;
> > +	args.out_args[0].size = sizeof(outarg_64);
> > +	args.out_args[0].value = &outarg_64;
> > +	if (fc->no_copy_file_range_64) {
> > +fallback:
> > +		/* Fall back to old op that can't handle large copy length */
> > +		args.opcode = FUSE_COPY_FILE_RANGE;
> > +		args.out_args[0].size = sizeof(outarg);
> > +		args.out_args[0].value = &outarg;
> > +		inarg.len = min_t(size_t, len, 0xfffff000);
> > +	}
> >  	err = fuse_simple_request(fm, &args);
> >  	if (err == -ENOSYS) {
> > -		fc->no_copy_file_range = 1;
> > -		err = -EOPNOTSUPP;
> > +		if (fc->no_copy_file_range_64) {
> > +			fc->no_copy_file_range = 1;
> > +			err = -EOPNOTSUPP;
> > +		} else {
> > +			fc->no_copy_file_range_64 = 1;
> > +			goto fallback;
> > +		}
> >  	}
> >  	if (err)
> >  		goto out;
> >  
> > +	bytes_copied = fc->no_copy_file_range_64 ?
> > +		outarg.size : outarg_64.bytes_copied;
> > +
> >  	truncate_inode_pages_range(inode_out->i_mapping,
> >  				   ALIGN_DOWN(pos_out, PAGE_SIZE),
> > -				   ALIGN(pos_out + outarg.size, PAGE_SIZE) - 1);
> > +				   ALIGN(pos_out + bytes_copied, PAGE_SIZE) - 1);
> >  
> >  	file_update_time(file_out);
> > -	fuse_write_update_attr(inode_out, pos_out + outarg.size, outarg.size);
> > +	fuse_write_update_attr(inode_out, pos_out + bytes_copied, bytes_copied);
> >  
> > -	err = outarg.size;
> > +	err = bytes_copied;
> >  out:
> >  	if (is_unstable)
> >  		clear_bit(FUSE_I_SIZE_UNSTABLE, &fi_out->state);
> > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> > index b54f4f57789f..a8be19f686b1 100644
> > --- a/fs/fuse/fuse_i.h
> > +++ b/fs/fuse/fuse_i.h
> > @@ -850,6 +850,9 @@ struct fuse_conn {
> >  	/** Does the filesystem support copy_file_range? */
> >  	unsigned no_copy_file_range:1;
> >  
> > +	/** Does the filesystem support copy_file_range_64? */
> > +	unsigned no_copy_file_range_64:1;
> > +
> >  	/* Send DESTROY request */
> >  	unsigned int destroy:1;
> >  
> > diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> > index 122d6586e8d4..94621f68a5cc 100644
> > --- a/include/uapi/linux/fuse.h
> > +++ b/include/uapi/linux/fuse.h
> > @@ -235,6 +235,10 @@
> >   *
> >   *  7.44
> >   *  - add FUSE_NOTIFY_INC_EPOCH
> > + *
> > + *  7.45
> > + *  - add FUSE_COPY_FILE_RANGE_64
> > + *  - add struct fuse_copy_file_range_out
> >   */
> >  
> >  #ifndef _LINUX_FUSE_H
> > @@ -270,7 +274,7 @@
> >  #define FUSE_KERNEL_VERSION 7
> >  
> >  /** Minor version number of this interface */
> > -#define FUSE_KERNEL_MINOR_VERSION 44
> > +#define FUSE_KERNEL_MINOR_VERSION 45
> >  
> >  /** The node ID of the root inode */
> >  #define FUSE_ROOT_ID 1
> > @@ -657,6 +661,7 @@ enum fuse_opcode {
> >  	FUSE_SYNCFS		= 50,
> >  	FUSE_TMPFILE		= 51,
> >  	FUSE_STATX		= 52,
> > +	FUSE_COPY_FILE_RANGE_64	= 53,
> >  
> >  	/* CUSE specific operations */
> >  	CUSE_INIT		= 4096,
> > @@ -1148,6 +1153,11 @@ struct fuse_copy_file_range_in {
> >  	uint64_t	flags;
> >  };
> >  
> > +/* For FUSE_COPY_FILE_RANGE_64 */
> > +struct fuse_copy_file_range_out {
> > +	uint64_t	bytes_copied;
> > +};
> > +
> >  #define FUSE_SETUPMAPPING_FLAG_WRITE (1ull << 0)
> >  #define FUSE_SETUPMAPPING_FLAG_READ (1ull << 1)
> >  struct fuse_setupmapping_in {
> > -- 
> > 2.49.0
> >
> 
>