On Wed, Nov 9, 2022 at 8:47 PM Jeremy Allison <jra@xxxxxxxxx> wrote: > > On Wed, Nov 09, 2022 at 10:38:02AM -0800, Jeremy Allison via samba-technical wrote: > >On Wed, Nov 09, 2022 at 11:32:30AM +0200, Amir Goldstein wrote: > >>On Tue, Nov 8, 2022 at 7:53 PM Jeremy Allison via samba-technical > >><samba-technical@xxxxxxxxxxxxxxx> wrote: > >>> > >>>On Mon, Nov 07, 2022 at 10:47:48PM -0800, Christoph Hellwig wrote: > >>>>On Mon, Nov 07, 2022 at 04:53:42PM -0800, Jeremy Allison via samba-technical wrote: > >>>>> ret = ioctl(fsp_get_io_fd(dest_fsp), BTRFS_IOC_CLONE_RANGE, &cr_args); > >>>>> > >>>>> what ioctls are used for this in XFS ? > >>>>> > >>>>> We'd need a VFS module that implements them for XFS. > >>>> > >>>>That ioctl is now implemented in the Linux VFS and supported by btrfs, > >>>>ocfs2, xfs, nfs (v4.2), cifs and overlayfs. > >>> > >>>I'm assuming it's this: > >>> > >>>https://man7.org/linux/man-pages/man2/ioctl_ficlonerange.2.html > >>> > >>>Yeah ? I'll write some test code and see if I can get it > >>>into the vfs_default code. > >>> > >> > >>Looks like this was already discussed during the work on generic > >>implementation of FSCTL_SRV_COPYCHUNK: > >>https://bugzilla.samba.org/show_bug.cgi?id=12033#c3 > >> > >>Forgotten? > > > >Yep :-). > > > >>Left for later? > > > >So looks like we do copy_file_range(), but not CLONE_RANGE, > >or rather CLONE_RANGE only in btrfs. > > > >So the code change needed is to move the logic in vfs_btrfs.c > >into vfs_default.c, and change the call in vfs_btrfs.c:btrfs_offload_write_send() > >to SMB_VFS_NEXT_OFFLOAD_WRITE_SEND() to call the old fallback code > >inside vfs_default.c (vfswrap_offload_write_send()). > > Although looking at the current Linux kernel I find inside: > > ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in, > struct file *file_out, loff_t pos_out, > size_t len, unsigned int flags) > { > > https://github.com/torvalds/linux/blob/0adc313c4f20639f7e235b8d6719d96a2024cf91/fs/read_write.c#L1506 > > /* > * Try cloning first, this is supported by more file systems, and > * more efficient if both clone and copy are supported (e.g. NFS). > */ > if (file_in->f_op->remap_file_range && > file_inode(file_in)->i_sb == file_inode(file_out)->i_sb) { > loff_t cloned; > > cloned = file_in->f_op->remap_file_range(file_in, pos_in, > file_out, pos_out, > min_t(loff_t, MAX_RW_COUNT, len), > REMAP_FILE_CAN_SHORTEN); > if (cloned > 0) { > ret = cloned; > > and looking at the code supporting int ioctl(int dest_fd, FICLONERANGE, struct file_clone_range *arg); > we have: > > loff_t do_clone_file_range(struct file *file_in, loff_t pos_in, > struct file *file_out, loff_t pos_out, > loff_t len, unsigned int remap_flags) > ... > ret = file_in->f_op->remap_file_range(file_in, pos_in, > file_out, pos_out, len, remap_flags); > > So it *looks* like the copy_file_range() syscall will internally > call the equivalent of FICLONERANGE if the underlying file > system supports it. > It's true. Unless you have some SMB command that requires the clone to be a clone (what is VFS_COPY_CHUNK_FL_MUST_CLONE in the referred comment?) because currently there is no flag that can be passed to copy_file_range() to request only clone. Thanks, Amir. > So maybe the right fix is to remove the FICLONERANGE specific > code from our vfs_btrfs.c and just always use copy_file_range(). > > Any comments from other Samba Team members ? > > Jeremy.
