Return-Path: Received: from mail-qk0-f194.google.com ([209.85.220.194]:35622 "EHLO mail-qk0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751873AbdGFU6i (ORCPT ); Thu, 6 Jul 2017 16:58:38 -0400 MIME-Version: 1.0 In-Reply-To: <20170630205803.95725-1-kolga@netapp.com> References: <20170630205803.95725-1-kolga@netapp.com> From: Olga Kornievskaia Date: Thu, 6 Jul 2017 16:58:37 -0400 Message-ID: Subject: Re: [PATCH v2 1/1] VFS permit cross device vfs_copy_file_range To: Olga Kornievskaia Cc: "linux-fsdevel@vger.kernel.org" , linux-nfs Content-Type: text/plain; charset="UTF-8" Sender: linux-nfs-owner@vger.kernel.org List-ID: Hi folks, Are there any more comments? Will this be accepted? On Fri, Jun 30, 2017 at 4:58 PM, Olga Kornievskaia wrote: > Allow copy_file_range to copy between different superblocks but only > of the same file system types. > > This feature is needed by NFSv4.2 to perform file copy operation on > the same server or file copy between different NFSv4.2 servers. > > If a file system's fileoperations copy_file_range operation prohibits > cross-device copies, fall back to do_splice_direct. This would be > needed for the NFS (destination) server side implementation of the > file copy and currently for CIFS. > > Besides NFS, there is only 1 implementor of the copy_file_range FS > operation -- CIFS. CIFS assumes incoming file descriptors are both > CIFS but it will check if they are coming from different servers and > return error code to fall back to do_splice_direct. > > NFS will allow for copies between different NFS servers. > > Adding to the vfs.txt documentation to explicitly warn about allowing > for different superblocks of the same file type to be passed into the > copy_file_range for the future users of copy_file_range method. > > Signed-off-by: Olga Kornievskaia > --- > Documentation/filesystems/vfs.txt | 7 +++++++ > fs/read_write.c | 13 ++++++------- > 2 files changed, 13 insertions(+), 7 deletions(-) > > diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt > index f42b906..0c243f6 100644 > --- a/Documentation/filesystems/vfs.txt > +++ b/Documentation/filesystems/vfs.txt > @@ -845,6 +845,8 @@ struct file_operations { > #ifndef CONFIG_MMU > unsigned (*mmap_capabilities)(struct file *); > #endif > + ssize_t (*copy_file_range)(struct file *, loff_t, struct file *, > + loff_t, size_t, unsigned int); > }; > > Again, all methods are called without any locks being held, unless > @@ -913,6 +915,11 @@ otherwise noted. > > fallocate: called by the VFS to preallocate blocks or punch a hole. > > + copy_file_range: called by copy_file_range(2) system call. This method > + works on two file descriptors that might reside on > + different superblocks of the same type of file system. > + > + > Note that the file operations are implemented by the specific > filesystem in which the inode resides. When opening a device node > (character or block special) most filesystems will call special > diff --git a/fs/read_write.c b/fs/read_write.c > index 47c1d44..f981d49 100644 > --- a/fs/read_write.c > +++ b/fs/read_write.c > @@ -1589,10 +1589,6 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in, > (file_out->f_flags & O_APPEND)) > return -EBADF; > > - /* this could be relaxed once a method supports cross-fs copies */ > - if (inode_in->i_sb != inode_out->i_sb) > - return -EXDEV; > - > if (len == 0) > return 0; > > @@ -1602,7 +1598,8 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in, > * 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->clone_file_range) { > + if (inode_in->i_sb == inode_out->i_sb && > + file_in->f_op->clone_file_range) { > ret = file_in->f_op->clone_file_range(file_in, pos_in, > file_out, pos_out, len); > if (ret == 0) { > @@ -1611,10 +1608,12 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in, > } > } > > - if (file_out->f_op->copy_file_range) { > + if (file_out->f_op->copy_file_range && > + (file_in->f_op->copy_file_range == > + file_out->f_op->copy_file_range)) { > ret = file_out->f_op->copy_file_range(file_in, pos_in, file_out, > pos_out, len, flags); > - if (ret != -EOPNOTSUPP) > + if (ret != -EOPNOTSUPP && ret != -EXDEV) > goto done; > } > > -- > 1.8.3.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html