On Wed, Aug 06, 2025 at 08:48:41PM +0100, Luis Henriques wrote:
> On Wed, Aug 06 2025, Darrick J. Wong wrote:
>
> > 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.
>
> Oh, OK. So looks like I misunderstood that code. In vfs_copy_file_range(),
> I assumed that the fallback to splice ('splice = true;') would cap the IO
> with the following:
>
> ret = do_splice_direct(file_in, &pos_in, file_out, &pos_out,
> min_t(size_t, len, MAX_RW_COUNT), 0);
>
> And that's why I suggested to do the same here instead of UINT_MAX - 4096.
(/me stumbles back in after FOSSY)
Yeah -- splice actually /does/ dirty pages, because it's actually doing
a buffer copy in a (hopefully) more efficiently than havng the userspace
process do malloc/read/write and endure syscall overhead. That's why
it's ok to limit a splice to MAX_RW_COUNT, because it's basically a
write().
--D
> Cheers,
> --
> Luís
>
>
> > --D
> >
> >> Cheers,
> >> --
> >> Luís
> >>
> >>
> >> > Reported-by: Florian Weimer <fweimer@xxxxxxxxxx>
> >> > Closes: https://lore.kernel.org/all/lhuh5ynl8z5.fsf@xxxxxxxxxxxxxxxxxxxxxxxx/
> >> > Signed-off-by: Miklos Szeredi <mszeredi@xxxxxxxxxx>
> >> > ---
> >> > 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
> >> >
> >>
> >>
>
>
