* [PATCH v1 01/11] fs: Don't copy beyond the end of the file @ 2018-10-19 15:30 Olga Kornievskaia 2018-10-19 15:30 ` [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range Olga Kornievskaia 0 siblings, 1 reply; 39+ messages in thread From: Olga Kornievskaia @ 2018-10-19 15:30 UTC (permalink / raw) To: linux-fsdevel; +Cc: linux-nfs, fweimer, smfrench From: Anna Schumaker <Anna.Schumaker@Netapp.com> Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com> --- fs/read_write.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/read_write.c b/fs/read_write.c index 39b4a21..c60790f 100644 --- a/fs/read_write.c +++ b/fs/read_write.c @@ -1570,6 +1570,9 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in, if (unlikely(ret)) return ret; + if (pos_in >= i_size_read(inode_in)) + return -EINVAL; + if (!(file_in->f_mode & FMODE_READ) || !(file_out->f_mode & FMODE_WRITE) || (file_out->f_flags & O_APPEND)) -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-19 15:30 [PATCH v1 01/11] fs: Don't copy beyond the end of the file Olga Kornievskaia @ 2018-10-19 15:30 ` Olga Kornievskaia 2018-10-19 15:54 ` Amir Goldstein ` (2 more replies) 0 siblings, 3 replies; 39+ messages in thread From: Olga Kornievskaia @ 2018-10-19 15:30 UTC (permalink / raw) To: linux-fsdevel; +Cc: linux-nfs, fweimer, smfrench From: Olga Kornievskaia <kolga@netapp.com> Allow copy_file_range to copy between different superblocks but only of the same file system types. This feature was of interest to CIFS as well as NFS. 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 <kolga@netapp.com> --- Documentation/filesystems/vfs.txt | 4 +++- fs/read_write.c | 13 ++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt index a6c6a8a..5e520de 100644 --- a/Documentation/filesystems/vfs.txt +++ b/Documentation/filesystems/vfs.txt @@ -958,7 +958,9 @@ otherwise noted. fallocate: called by the VFS to preallocate blocks or punch a hole. - copy_file_range: called by the copy_file_range(2) system call. + 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. clone_file_range: called by the ioctl(2) system call for FICLONERANGE and FICLONE commands. diff --git a/fs/read_write.c b/fs/read_write.c index c60790f..474e740 100644 --- a/fs/read_write.c +++ b/fs/read_write.c @@ -1578,10 +1578,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; @@ -1591,7 +1587,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) { @@ -1600,10 +1597,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 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-19 15:30 ` [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range Olga Kornievskaia @ 2018-10-19 15:54 ` Amir Goldstein 2018-10-19 16:14 ` Amir Goldstein 2018-10-19 16:24 ` Olga Kornievskaia 2018-10-19 17:58 ` Matthew Wilcox 2018-10-20 4:05 ` Al Viro 2 siblings, 2 replies; 39+ messages in thread From: Amir Goldstein @ 2018-10-19 15:54 UTC (permalink / raw) To: olga.kornievskaia Cc: linux-fsdevel, Linux NFS Mailing List, fweimer, Steve French On Fri, Oct 19, 2018 at 6:30 PM Olga Kornievskaia <olga.kornievskaia@gmail.com> wrote: > > From: Olga Kornievskaia <kolga@netapp.com> > > Allow copy_file_range to copy between different superblocks but only > of the same file system types. This feature was of interest to CIFS > as well as NFS. > > 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 <kolga@netapp.com> > --- > Documentation/filesystems/vfs.txt | 4 +++- > fs/read_write.c | 13 ++++++------- > 2 files changed, 9 insertions(+), 8 deletions(-) > > diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt > index a6c6a8a..5e520de 100644 > --- a/Documentation/filesystems/vfs.txt > +++ b/Documentation/filesystems/vfs.txt > @@ -958,7 +958,9 @@ otherwise noted. > > fallocate: called by the VFS to preallocate blocks or punch a hole. > > - copy_file_range: called by the copy_file_range(2) system call. > + 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. > > clone_file_range: called by the ioctl(2) system call for FICLONERANGE and > FICLONE commands. > diff --git a/fs/read_write.c b/fs/read_write.c > index c60790f..474e740 100644 > --- a/fs/read_write.c > +++ b/fs/read_write.c > @@ -1578,10 +1578,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; > - You need to hoist this limitation to clone_file_range() syscall, because you are not allowed to change user facing behavior. Maybe you can later add a uapi flag for copy_file_range() to explicitly allow for cross-sb copy? Maybe you can add the flag now for internal use - only nfsv4 will pass that flag to the vfs helper and system call will verify that flags == 0. Thanks, Amir. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-19 15:54 ` Amir Goldstein @ 2018-10-19 16:14 ` Amir Goldstein 2018-10-19 17:44 ` Matthew Wilcox 2018-10-19 16:24 ` Olga Kornievskaia 1 sibling, 1 reply; 39+ messages in thread From: Amir Goldstein @ 2018-10-19 16:14 UTC (permalink / raw) To: olga.kornievskaia Cc: linux-fsdevel, Linux NFS Mailing List, fweimer, Steve French, linux-api On Fri, Oct 19, 2018 at 6:54 PM Amir Goldstein <amir73il@gmail.com> wrote: > > On Fri, Oct 19, 2018 at 6:30 PM Olga Kornievskaia > <olga.kornievskaia@gmail.com> wrote: > > > > From: Olga Kornievskaia <kolga@netapp.com> > > > > Allow copy_file_range to copy between different superblocks but only > > of the same file system types. This feature was of interest to CIFS > > as well as NFS. > > > > 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 <kolga@netapp.com> > > --- > > Documentation/filesystems/vfs.txt | 4 +++- > > fs/read_write.c | 13 ++++++------- > > 2 files changed, 9 insertions(+), 8 deletions(-) > > > > diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt > > index a6c6a8a..5e520de 100644 > > --- a/Documentation/filesystems/vfs.txt > > +++ b/Documentation/filesystems/vfs.txt > > @@ -958,7 +958,9 @@ otherwise noted. > > > > fallocate: called by the VFS to preallocate blocks or punch a hole. > > > > - copy_file_range: called by the copy_file_range(2) system call. > > + 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. > > > > clone_file_range: called by the ioctl(2) system call for FICLONERANGE and > > FICLONE commands. > > diff --git a/fs/read_write.c b/fs/read_write.c > > index c60790f..474e740 100644 > > --- a/fs/read_write.c > > +++ b/fs/read_write.c > > @@ -1578,10 +1578,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; > > - > > You need to hoist this limitation to clone_file_range() syscall, because > you are not allowed to change user facing behavior. > Maybe you can later add a uapi flag for copy_file_range() to explicitly allow > for cross-sb copy? > Maybe you can add the flag now for internal use - only nfsv4 will pass that > flag to the vfs helper and system call will verify that flags == 0. > Sorry, you meant to allow cross sb copy from client, so you do want to change user facing behavior. For that you need to declare a flag to opt in for the new behavior and document it. Please keep linux-api CC-ed when posting the flag. Referencing a draft for man page is recommended. Thanks, Amir. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-19 16:14 ` Amir Goldstein @ 2018-10-19 17:44 ` Matthew Wilcox 2018-10-19 17:58 ` Amir Goldstein 0 siblings, 1 reply; 39+ messages in thread From: Matthew Wilcox @ 2018-10-19 17:44 UTC (permalink / raw) To: Amir Goldstein Cc: olga.kornievskaia, linux-fsdevel, Linux NFS Mailing List, fweimer, Steve French, linux-api On Fri, Oct 19, 2018 at 07:14:23PM +0300, Amir Goldstein wrote: > > On Fri, Oct 19, 2018 at 6:30 PM Olga Kornievskaia > > <olga.kornievskaia@gmail.com> wrote: > > > Allow copy_file_range to copy between different superblocks but only > > > of the same file system types. This feature was of interest to CIFS > > > as well as NFS. > > Sorry, you meant to allow cross sb copy from client, so you do want > to change user facing behavior. > For that you need to declare a flag to opt in for the new behavior > and document it. > Please keep linux-api CC-ed when posting the flag. > Referencing a draft for man page is recommended. Huh? We're allowed to make things work that used to fail. There's no need to add a flag that the user must set. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-19 17:44 ` Matthew Wilcox @ 2018-10-19 17:58 ` Amir Goldstein 0 siblings, 0 replies; 39+ messages in thread From: Amir Goldstein @ 2018-10-19 17:58 UTC (permalink / raw) To: Matthew Wilcox Cc: olga.kornievskaia, linux-fsdevel, Linux NFS Mailing List, fweimer, Steve French, linux-api, Christoph Hellwig On Fri, Oct 19, 2018 at 8:44 PM Matthew Wilcox <willy@infradead.org> wrote: > > On Fri, Oct 19, 2018 at 07:14:23PM +0300, Amir Goldstein wrote: > > > On Fri, Oct 19, 2018 at 6:30 PM Olga Kornievskaia > > > <olga.kornievskaia@gmail.com> wrote: > > > > Allow copy_file_range to copy between different superblocks but only > > > > of the same file system types. This feature was of interest to CIFS > > > > as well as NFS. > > > > Sorry, you meant to allow cross sb copy from client, so you do want > > to change user facing behavior. > > For that you need to declare a flag to opt in for the new behavior > > and document it. > > Please keep linux-api CC-ed when posting the flag. > > Referencing a draft for man page is recommended. > > Huh? We're allowed to make things work that used to fail. There's no > need to add a flag that the user must set. OK, you are right. Maybe this case doesn't call for an opt-in flag. I'll let other people weight in. I should note that the behavior of copy_file_range() has already changed silently to try to clone_file_range() first and that went quietly... Thanks, Amir. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-19 15:54 ` Amir Goldstein 2018-10-19 16:14 ` Amir Goldstein @ 2018-10-19 16:24 ` Olga Kornievskaia 2018-10-19 17:04 ` Olga Kornievskaia 2018-10-20 1:37 ` Steve French 1 sibling, 2 replies; 39+ messages in thread From: Olga Kornievskaia @ 2018-10-19 16:24 UTC (permalink / raw) To: Amir Goldstein; +Cc: linux-fsdevel, linux-nfs, fweimer, Steve French On Fri, Oct 19, 2018 at 11:55 AM Amir Goldstein <amir73il@gmail.com> wrote: > > On Fri, Oct 19, 2018 at 6:30 PM Olga Kornievskaia > <olga.kornievskaia@gmail.com> wrote: > > > > From: Olga Kornievskaia <kolga@netapp.com> > > > > Allow copy_file_range to copy between different superblocks but only > > of the same file system types. This feature was of interest to CIFS > > as well as NFS. > > > > 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 <kolga@netapp.com> > > --- > > Documentation/filesystems/vfs.txt | 4 +++- > > fs/read_write.c | 13 ++++++------- > > 2 files changed, 9 insertions(+), 8 deletions(-) > > > > diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt > > index a6c6a8a..5e520de 100644 > > --- a/Documentation/filesystems/vfs.txt > > +++ b/Documentation/filesystems/vfs.txt > > @@ -958,7 +958,9 @@ otherwise noted. > > > > fallocate: called by the VFS to preallocate blocks or punch a hole. > > > > - copy_file_range: called by the copy_file_range(2) system call. > > + 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. > > > > clone_file_range: called by the ioctl(2) system call for FICLONERANGE and > > FICLONE commands. > > diff --git a/fs/read_write.c b/fs/read_write.c > > index c60790f..474e740 100644 > > --- a/fs/read_write.c > > +++ b/fs/read_write.c > > @@ -1578,10 +1578,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; > > - > > You need to hoist this limitation to clone_file_range() syscall, because > you are not allowed to change user facing behavior. > Maybe you can later add a uapi flag for copy_file_range() to explicitly allow > for cross-sb copy? Sure I can do that. > Maybe you can add the flag now for internal use - only nfsv4 will pass that > flag to the vfs helper and system call will verify that flags == 0. Not only NFS wants it CIFS want it too. > > Thanks, > Amir. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-19 16:24 ` Olga Kornievskaia @ 2018-10-19 17:04 ` Olga Kornievskaia 2018-10-20 1:37 ` Steve French 1 sibling, 0 replies; 39+ messages in thread From: Olga Kornievskaia @ 2018-10-19 17:04 UTC (permalink / raw) To: Amir Goldstein; +Cc: linux-fsdevel, linux-nfs, fweimer, Steve French On Fri, Oct 19, 2018 at 12:24 PM Olga Kornievskaia <olga.kornievskaia@gmail.com> wrote: > > On Fri, Oct 19, 2018 at 11:55 AM Amir Goldstein <amir73il@gmail.com> wrote: > > > > On Fri, Oct 19, 2018 at 6:30 PM Olga Kornievskaia > > <olga.kornievskaia@gmail.com> wrote: > > > > > > From: Olga Kornievskaia <kolga@netapp.com> > > > > > > Allow copy_file_range to copy between different superblocks but only > > > of the same file system types. This feature was of interest to CIFS > > > as well as NFS. > > > > > > 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 <kolga@netapp.com> > > > --- > > > Documentation/filesystems/vfs.txt | 4 +++- > > > fs/read_write.c | 13 ++++++------- > > > 2 files changed, 9 insertions(+), 8 deletions(-) > > > > > > diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt > > > index a6c6a8a..5e520de 100644 > > > --- a/Documentation/filesystems/vfs.txt > > > +++ b/Documentation/filesystems/vfs.txt > > > @@ -958,7 +958,9 @@ otherwise noted. > > > > > > fallocate: called by the VFS to preallocate blocks or punch a hole. > > > > > > - copy_file_range: called by the copy_file_range(2) system call. > > > + 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. > > > > > > clone_file_range: called by the ioctl(2) system call for FICLONERANGE and > > > FICLONE commands. > > > diff --git a/fs/read_write.c b/fs/read_write.c > > > index c60790f..474e740 100644 > > > --- a/fs/read_write.c > > > +++ b/fs/read_write.c > > > @@ -1578,10 +1578,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; > > > - > > > > You need to hoist this limitation to clone_file_range() syscall, because > > you are not allowed to change user facing behavior. > > Maybe you can later add a uapi flag for copy_file_range() to explicitly allow > > for cross-sb copy? > > Sure I can do that. > > > Maybe you can add the flag now for internal use - only nfsv4 will pass that > > flag to the vfs helper and system call will verify that flags == 0. > > Not only NFS wants it CIFS want it too. Can you please elaborate on what do you mean "internal use only"? nfsv4 doesn't call copy_file_range(), a user land application (such as glibc cp, or whatever does a system call for the copy_file_range()) is the one calling and doing a copy between two different server (ie superblocks). Thus the system call can't keep the check "flags == 0". I can (maybe?) add user level flags that copy_file_range() system call will pass, I will have to remove the "flags == 0" check and then I can add a check if such a flag is passed, then skip the check for the cross device check in copy_file_range(). Is this what's desired? > > > > > Thanks, > > Amir. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-19 16:24 ` Olga Kornievskaia 2018-10-19 17:04 ` Olga Kornievskaia @ 2018-10-20 1:37 ` Steve French 1 sibling, 0 replies; 39+ messages in thread From: Steve French @ 2018-10-20 1:37 UTC (permalink / raw) To: olga.kornievskaia; +Cc: Amir Goldstein, linux-fsdevel, linux-nfs, fweimer On Fri, Oct 19, 2018 at 11:24 AM Olga Kornievskaia <olga.kornievskaia@gmail.com> wrote: > > On Fri, Oct 19, 2018 at 11:55 AM Amir Goldstein <amir73il@gmail.com> wrote: > > > > On Fri, Oct 19, 2018 at 6:30 PM Olga Kornievskaia > > <olga.kornievskaia@gmail.com> wrote: > > > > > > From: Olga Kornievskaia <kolga@netapp.com> > > > > > > Allow copy_file_range to copy between different superblocks but only > > > of the same file system types. This feature was of interest to CIFS > > > as well as NFS. > > > > > > 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. > > > <snip> > > Maybe you can add the flag now for internal use - only nfsv4 will pass that > > flag to the vfs helper and system call will verify that flags == 0. > > Not only NFS wants it CIFS want it too. Allowing copy offload across distinct smb3 mounts would be very helpful for cifs.ko Yes - note that many servers that cifs.ko (SMB3) has to deal with have already broadly implemented copy offload (multiple flavors, not just the 'copy chunk' style) and 100s of millions of these systems (not just Windows and Samba) support them as long as the source and target of the copy are to the same server. It is the client side and lack of tools and to some extent cross-mount copies that is holding it back from being more widely used. One reason I have only implemented two of the copy offload mechanisms for SMB3 ('duplicate extents' and 'copy chunk') and not the third, the popular 'odx' (T10 style) copy offload is in part that the value of the 'odx' style is more when the source and target are different servers. Note that this is broadly implemented on servers even on other Unix (see e.g. Nexenta's description of server impmlementation of 'odx' style https://www.slideshare.net/gordonross/smb3-offload-data-transfer-odx) so the client API enhancements would be quickly useful for cifs.ko (SMB3 mounts) use case. -- Thanks, Steve ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-19 15:30 ` [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range Olga Kornievskaia 2018-10-19 15:54 ` Amir Goldstein @ 2018-10-19 17:58 ` Matthew Wilcox 2018-10-19 18:47 ` Olga Kornievskaia 2018-10-21 14:10 ` Jeff Layton 2018-10-20 4:05 ` Al Viro 2 siblings, 2 replies; 39+ messages in thread From: Matthew Wilcox @ 2018-10-19 17:58 UTC (permalink / raw) To: Olga Kornievskaia; +Cc: linux-fsdevel, linux-nfs, fweimer, smfrench On Fri, Oct 19, 2018 at 11:30:18AM -0400, Olga Kornievskaia wrote: > +++ b/Documentation/filesystems/vfs.txt > @@ -958,7 +958,9 @@ otherwise noted. > > fallocate: called by the VFS to preallocate blocks or punch a hole. > > - copy_file_range: called by the copy_file_range(2) system call. > + 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. I don't think this text is explicit enough about what has changed, and I think this is the wrong place for it. I think there should be a paragraph in Documentation/filesystems/porting and it should follow the current style in there. > @@ -1591,7 +1587,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) { This reads weirdly to me. I know it's the same order the tests were done in before, but it would feel more natural to me to test: if (file_in->f_op->clone_file_range && inode_in->i_sb == inode_out->i_sb) Am I just suffering from "I would have done this differently"itis, or is it unnatural? > @@ -1600,10 +1597,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)) { Can we avoid this extra test here? I know the various stamdards groups including T10 and the IETF have been trying to define a universal identifier for the same blob of storage, no matter how it's accessed; potentially allowing access to the same storage across iSCSI, CIFS and NFS. If we ever get to a point where we support that (and I am dubious), we'd want to remove this test again, and have to revalidate all the filesystems. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-19 17:58 ` Matthew Wilcox @ 2018-10-19 18:47 ` Olga Kornievskaia 2018-10-19 19:06 ` Matthew Wilcox 2018-10-21 14:10 ` Jeff Layton 1 sibling, 1 reply; 39+ messages in thread From: Olga Kornievskaia @ 2018-10-19 18:47 UTC (permalink / raw) To: willy; +Cc: linux-fsdevel, linux-nfs, fweimer, Steve French On Fri, Oct 19, 2018 at 1:58 PM Matthew Wilcox <willy@infradead.org> wrote: > > On Fri, Oct 19, 2018 at 11:30:18AM -0400, Olga Kornievskaia wrote: > > +++ b/Documentation/filesystems/vfs.txt > > @@ -958,7 +958,9 @@ otherwise noted. > > > > fallocate: called by the VFS to preallocate blocks or punch a hole. > > > > - copy_file_range: called by the copy_file_range(2) system call. > > + 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. > > I don't think this text is explicit enough about what has changed, Can you suggest a different wording that would be stronger? I can't say any more clear that copy is allowed between different superblock which wasn't the case before. > and I > think this is the wrong place for it. I think there should be a paragraph > in Documentation/filesystems/porting and it should follow the current style > in there. I have looked at the "porting" file and it's cryptic. I don't understand what [mandatory] [recommended] stanzas are. There is currently no copy_file_range. Is this just a "changelog" and you are looking for something like [mandatory] copy_file_range() now allows copying between different superblocks. I can add this wording to the "porting" file. > > @@ -1591,7 +1587,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) { > > This reads weirdly to me. I know it's the same order the tests were done > in before, but it would feel more natural to me to test: > > if (file_in->f_op->clone_file_range && > inode_in->i_sb == inode_out->i_sb) > > Am I just suffering from "I would have done this differently"itis, or > is it unnatural? I really don't care one way or another as long as the functionality goes in. I can change it. > > @@ -1600,10 +1597,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)) { > > Can we avoid this extra test here? I know the various stamdards groups > including T10 and the IETF have been trying to define a universal > identifier for the same blob of storage, no matter how it's accessed; > potentially allowing access to the same storage across iSCSI, CIFS > and NFS. If we ever get to a point where we support that (and I am > dubious), we'd want to remove this test again, and have to revalidate > all the filesystems. Well from previous submissions I was explicitly asked to add this check. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-19 18:47 ` Olga Kornievskaia @ 2018-10-19 19:06 ` Matthew Wilcox 2018-10-21 13:01 ` Jeff Layton 2018-10-22 18:39 ` Olga Kornievskaia 0 siblings, 2 replies; 39+ messages in thread From: Matthew Wilcox @ 2018-10-19 19:06 UTC (permalink / raw) To: Olga Kornievskaia; +Cc: linux-fsdevel, linux-nfs, fweimer, Steve French On Fri, Oct 19, 2018 at 02:47:42PM -0400, Olga Kornievskaia wrote: > On Fri, Oct 19, 2018 at 1:58 PM Matthew Wilcox <willy@infradead.org> wrote: > > > > On Fri, Oct 19, 2018 at 11:30:18AM -0400, Olga Kornievskaia wrote: > > > +++ b/Documentation/filesystems/vfs.txt > > > @@ -958,7 +958,9 @@ otherwise noted. > > > > > > fallocate: called by the VFS to preallocate blocks or punch a hole. > > > > > > - copy_file_range: called by the copy_file_range(2) system call. > > > + 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. > > > > I don't think this text is explicit enough about what has changed, > > Can you suggest a different wording that would be stronger? I can't > say any more clear that copy is allowed between different superblock > which wasn't the case before. I would leave this file alone. > > and I > > think this is the wrong place for it. I think there should be a paragraph > > in Documentation/filesystems/porting and it should follow the current style > > in there. > > I have looked at the "porting" file and it's cryptic. I don't > understand what [mandatory] [recommended] stanzas are. There is > currently no copy_file_range. Is this just a "changelog" and you are > looking for something like > [mandatory] > copy_file_range() now allows copying between different superblocks. > > I can add this wording to the "porting" file. The porting file is written from the point of view of someone who's trying to port an old filesystem to current Linux. Maybe something like [mandatory] ->copy_file_range() may now be passed files which belong to two different filesystems. The destination's copy_file_range() is the function which is called. If it cannot copy ranges from the source, it should return -EXDEV. > > > - 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)) { > > > > Can we avoid this extra test here? I know the various stamdards groups > > including T10 and the IETF have been trying to define a universal > > identifier for the same blob of storage, no matter how it's accessed; > > potentially allowing access to the same storage across iSCSI, CIFS > > and NFS. If we ever get to a point where we support that (and I am > > dubious), we'd want to remove this test again, and have to revalidate > > all the filesystems. > > Well from previous submissions I was explicitly asked to add this check. I'm not sure that's exactly what Jeff was asking for. Or maybe it was and my argument above will change his mind. Jeff, what do you think? If we do do this, cifs will need a modification as part of this patch to reject non-CIFS files as it currently assumes that src_file->private_data is a pointer to a struct cifsFileInfo. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-19 19:06 ` Matthew Wilcox @ 2018-10-21 13:01 ` Jeff Layton 2018-10-22 18:39 ` Olga Kornievskaia 1 sibling, 0 replies; 39+ messages in thread From: Jeff Layton @ 2018-10-21 13:01 UTC (permalink / raw) To: Matthew Wilcox, Olga Kornievskaia Cc: linux-fsdevel, linux-nfs, fweimer, Steve French, Al Viro On Fri, 2018-10-19 at 12:06 -0700, Matthew Wilcox wrote: > On Fri, Oct 19, 2018 at 02:47:42PM -0400, Olga Kornievskaia wrote: > > On Fri, Oct 19, 2018 at 1:58 PM Matthew Wilcox <willy@infradead.org> wrote: > > > > > > On Fri, Oct 19, 2018 at 11:30:18AM -0400, Olga Kornievskaia wrote: > > > > +++ b/Documentation/filesystems/vfs.txt > > > > @@ -958,7 +958,9 @@ otherwise noted. > > > > > > > > fallocate: called by the VFS to preallocate blocks or punch a hole. > > > > > > > > - copy_file_range: called by the copy_file_range(2) system call. > > > > + 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. > > > > > > I don't think this text is explicit enough about what has changed, > > > > Can you suggest a different wording that would be stronger? I can't > > say any more clear that copy is allowed between different superblock > > which wasn't the case before. > > I would leave this file alone. > > > > and I > > > think this is the wrong place for it. I think there should be a paragraph > > > in Documentation/filesystems/porting and it should follow the current style > > > in there. > > > > I have looked at the "porting" file and it's cryptic. I don't > > understand what [mandatory] [recommended] stanzas are. There is > > currently no copy_file_range. Is this just a "changelog" and you are > > looking for something like > > [mandatory] > > copy_file_range() now allows copying between different superblocks. > > > > I can add this wording to the "porting" file. > > The porting file is written from the point of view of someone who's trying > to port an old filesystem to current Linux. > > Maybe something like > > [mandatory] > ->copy_file_range() may now be passed files which belong to two > different filesystems. The destination's copy_file_range() is the > function which is called. If it cannot copy ranges from the source, > it should return -EXDEV. > > > > > - 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)) { > > > > > > Can we avoid this extra test here? I know the various stamdards groups > > > including T10 and the IETF have been trying to define a universal > > > identifier for the same blob of storage, no matter how it's accessed; > > > potentially allowing access to the same storage across iSCSI, CIFS > > > and NFS. If we ever get to a point where we support that (and I am > > > dubious), we'd want to remove this test again, and have to revalidate > > > all the filesystems. > > > > Well from previous submissions I was explicitly asked to add this check. > > I'm not sure that's exactly what Jeff was asking for. Or maybe it was > and my argument above will change his mind. Jeff, what do you think? > > If we do do this, cifs will need a modification as part of this patch to > reject non-CIFS files as it currently assumes that src_file->private_data > is a pointer to a struct cifsFileInfo. My suggestion to Olga was more of a "if you feel you have to have a check like this at the vfs layer...", but I think you and Al have convinced me that comparing operations like this is not a good idea. I still think that this check belongs down inside the fs copy_file_range operation itself. That allows the the freedom to implement xdev copies on a per-fs basis later without needing to alter vfs-level code. -- Jeff Layton <jlayton@kernel.org> ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-19 19:06 ` Matthew Wilcox 2018-10-21 13:01 ` Jeff Layton @ 2018-10-22 18:39 ` Olga Kornievskaia 1 sibling, 0 replies; 39+ messages in thread From: Olga Kornievskaia @ 2018-10-22 18:39 UTC (permalink / raw) To: willy; +Cc: linux-fsdevel, linux-nfs, fweimer, Steve French On Fri, Oct 19, 2018 at 3:06 PM Matthew Wilcox <willy@infradead.org> wrote: > > On Fri, Oct 19, 2018 at 02:47:42PM -0400, Olga Kornievskaia wrote: > > On Fri, Oct 19, 2018 at 1:58 PM Matthew Wilcox <willy@infradead.org> wrote: > > > > > > On Fri, Oct 19, 2018 at 11:30:18AM -0400, Olga Kornievskaia wrote: > > > > +++ b/Documentation/filesystems/vfs.txt > > > > @@ -958,7 +958,9 @@ otherwise noted. > > > > > > > > fallocate: called by the VFS to preallocate blocks or punch a hole. > > > > > > > > - copy_file_range: called by the copy_file_range(2) system call. > > > > + 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. > > > > > > I don't think this text is explicit enough about what has changed, > > > > Can you suggest a different wording that would be stronger? I can't > > say any more clear that copy is allowed between different superblock > > which wasn't the case before. > > I would leave this file alone. > > > > and I > > > think this is the wrong place for it. I think there should be a paragraph > > > in Documentation/filesystems/porting and it should follow the current style > > > in there. > > > > I have looked at the "porting" file and it's cryptic. I don't > > understand what [mandatory] [recommended] stanzas are. There is > > currently no copy_file_range. Is this just a "changelog" and you are > > looking for something like > > [mandatory] > > copy_file_range() now allows copying between different superblocks. > > > > I can add this wording to the "porting" file. > > The porting file is written from the point of view of someone who's trying > to port an old filesystem to current Linux. > > Maybe something like > > [mandatory] > ->copy_file_range() may now be passed files which belong to two > different filesystems. The destination's copy_file_range() is the > function which is called. If it cannot copy ranges from the source, > it should return -EXDEV. Thank you I can add this to the porting file. > > > > > - 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)) { > > > > > > Can we avoid this extra test here? I know the various stamdards groups > > > including T10 and the IETF have been trying to define a universal > > > identifier for the same blob of storage, no matter how it's accessed; > > > potentially allowing access to the same storage across iSCSI, CIFS > > > and NFS. If we ever get to a point where we support that (and I am > > > dubious), we'd want to remove this test again, and have to revalidate > > > all the filesystems. > > > > Well from previous submissions I was explicitly asked to add this check. > > I'm not sure that's exactly what Jeff was asking for. Or maybe it was > and my argument above will change his mind. Jeff, what do you think? > > If we do do this, cifs will need a modification as part of this patch to > reject non-CIFS files as it currently assumes that src_file->private_data > is a pointer to a struct cifsFileInfo. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-19 17:58 ` Matthew Wilcox 2018-10-19 18:47 ` Olga Kornievskaia @ 2018-10-21 14:10 ` Jeff Layton 1 sibling, 0 replies; 39+ messages in thread From: Jeff Layton @ 2018-10-21 14:10 UTC (permalink / raw) To: Matthew Wilcox, Olga Kornievskaia Cc: linux-fsdevel, linux-nfs, fweimer, smfrench On Fri, 2018-10-19 at 10:58 -0700, Matthew Wilcox wrote: > On Fri, Oct 19, 2018 at 11:30:18AM -0400, Olga Kornievskaia wrote: > > +++ b/Documentation/filesystems/vfs.txt > > @@ -958,7 +958,9 @@ otherwise noted. > > > > fallocate: called by the VFS to preallocate blocks or punch a hole. > > > > - copy_file_range: called by the copy_file_range(2) system call. > > + 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. > > I don't think this text is explicit enough about what has changed, and I > think this is the wrong place for it. I think there should be a paragraph > in Documentation/filesystems/porting and it should follow the current style > in there. > > > @@ -1591,7 +1587,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) { > > This reads weirdly to me. I know it's the same order the tests were done > in before, but it would feel more natural to me to test: > > if (file_in->f_op->clone_file_range && > inode_in->i_sb == inode_out->i_sb) > > Am I just suffering from "I would have done this differently"itis, or > is it unnatural? > > > @@ -1600,10 +1597,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)) { > > Can we avoid this extra test here? I know the various stamdards groups > including T10 and the IETF have been trying to define a universal > identifier for the same blob of storage, no matter how it's accessed; > potentially allowing access to the same storage across iSCSI, CIFS > and NFS. If we ever get to a point where we support that (and I am > dubious), we'd want to remove this test again, and have to revalidate > all the filesystems. > Agreed. I think this really ought to be left up to the lower fs. Pass the op both struct file pointers and let it sort it out. We just need to document the expectation that file copy_file_range ops vet file_in carefully as it could be from anything, and have them return an appropriate error code if it's not something they can deal with. -- Jeff Layton <jlayton@redhat.com> ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-19 15:30 ` [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range Olga Kornievskaia 2018-10-19 15:54 ` Amir Goldstein 2018-10-19 17:58 ` Matthew Wilcox @ 2018-10-20 4:05 ` Al Viro 2018-10-20 8:54 ` Amir Goldstein 2 siblings, 1 reply; 39+ messages in thread From: Al Viro @ 2018-10-20 4:05 UTC (permalink / raw) To: Olga Kornievskaia; +Cc: linux-fsdevel, linux-nfs, fweimer, smfrench On Fri, Oct 19, 2018 at 11:30:18AM -0400, Olga Kornievskaia wrote: > Allow copy_file_range to copy between different superblocks but only > of the same file system types. This feature was of interest to CIFS > as well as NFS. > + if (file_out->f_op->copy_file_range && > + (file_in->f_op->copy_file_range == > + file_out->f_op->copy_file_range)) { That is seriously asking for trouble. If at some point we add a library function usable as ->copy_file_range() instance, this will turn into a hard-to-spot problem. Comparing methods like that is best avoided. If you want to compare fs types, do just that - it's not hard. Another potential issue here is the interplay with local filesystems using vfs_clone_file_prep_inodes() (or Darrick's series lifting that into generic code). There we assume the same block size on both sides; that is automatically true if they live on the same superblock, but with your changes it's no longer true. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-20 4:05 ` Al Viro @ 2018-10-20 8:54 ` Amir Goldstein 2018-10-22 18:45 ` Olga Kornievskaia 0 siblings, 1 reply; 39+ messages in thread From: Amir Goldstein @ 2018-10-20 8:54 UTC (permalink / raw) To: Al Viro Cc: Olga Kornievskaia, linux-fsdevel, Linux NFS Mailing List, fweimer, Steve French, Darrick J. Wong, Christoph Hellwig, linux-api On Sat, Oct 20, 2018 at 7:06 AM Al Viro <viro@zeniv.linux.org.uk> wrote: > > On Fri, Oct 19, 2018 at 11:30:18AM -0400, Olga Kornievskaia wrote: > > Allow copy_file_range to copy between different superblocks but only > > of the same file system types. This feature was of interest to CIFS > > as well as NFS. > > > + if (file_out->f_op->copy_file_range && > > + (file_in->f_op->copy_file_range == > > + file_out->f_op->copy_file_range)) { > > That is seriously asking for trouble. If at some point we add > a library function usable as ->copy_file_range() instance, this > will turn into a hard-to-spot problem. > > Comparing methods like that is best avoided. If you want to compare > fs types, do just that - it's not hard. Another thing is the commit message claims to: "Allow copy_file_range to copy between different superblocks but only of the same file system types" But what the patch actually does is: "Allow copy_file_range() syscall to copy between different filesystems AND allow calling the filesystems' copy_file_range() method between different superblocks but only of the same file system types" It's probably OK and quite useful to do the former, but maybe man page should be fixed to explicitly mention that the copy is expected to work across filesystems since kernel version XXX (?) If you don't wish to change cross filesystem type behavior and only relax cross super block limitation, then you should replace the same inode->i_sb check above with same inode->i_sb->s_type check instead of doing the check only for calling the filesystem copy_file_range() method. > > Another potential issue here is the interplay with local filesystems > using vfs_clone_file_prep_inodes() (or Darrick's series lifting that > into generic code). There we assume the same block size on both > sides; that is automatically true if they live on the same superblock, > but with your changes it's no longer true. Heh? I don't see that Darrick's work has anything to do with copy_file_range(). It only touched vfs_copy_file_range() for the ->clone_file_range() call, which Olga's patch protects with same sb check. Thanks, Amir. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-20 8:54 ` Amir Goldstein @ 2018-10-22 18:45 ` Olga Kornievskaia 2018-10-22 19:06 ` Matthew Wilcox 0 siblings, 1 reply; 39+ messages in thread From: Olga Kornievskaia @ 2018-10-22 18:45 UTC (permalink / raw) To: Amir Goldstein Cc: viro, linux-fsdevel, linux-nfs, fweimer, Steve French, Darrick J. Wong, Christoph Hellwig, Linux API On Sat, Oct 20, 2018 at 4:54 AM Amir Goldstein <amir73il@gmail.com> wrote: > > On Sat, Oct 20, 2018 at 7:06 AM Al Viro <viro@zeniv.linux.org.uk> wrote: > > > > On Fri, Oct 19, 2018 at 11:30:18AM -0400, Olga Kornievskaia wrote: > > > Allow copy_file_range to copy between different superblocks but only > > > of the same file system types. This feature was of interest to CIFS > > > as well as NFS. > > > > > + if (file_out->f_op->copy_file_range && > > > + (file_in->f_op->copy_file_range == > > > + file_out->f_op->copy_file_range)) { > > > > That is seriously asking for trouble. If at some point we add > > a library function usable as ->copy_file_range() instance, this > > will turn into a hard-to-spot problem. > > > > Comparing methods like that is best avoided. If you want to compare > > fs types, do just that - it's not hard. > > Another thing is the commit message claims to: > "Allow copy_file_range to copy between different superblocks but only > of the same file system types" > > But what the patch actually does is: > "Allow copy_file_range() syscall to copy between different filesystems > AND allow calling the filesystems' copy_file_range() method > between different superblocks but only of the same file system types" > > It's probably OK and quite useful to do the former, but maybe man page > should be fixed to explicitly mention that the copy is expected to work > across filesystems since kernel version XXX (?) > > If you don't wish to change cross filesystem type behavior and only > relax cross super block limitation, then you should replace the > same inode->i_sb check above with same inode->i_sb->s_type > check instead of doing the check only for calling the filesystem > copy_file_range() method. Thank you for the feedback. In the next version, I will remove the check for the functions and instead check for the same file system types. > > > > > Another potential issue here is the interplay with local filesystems > > using vfs_clone_file_prep_inodes() (or Darrick's series lifting that > > into generic code). There we assume the same block size on both > > sides; that is automatically true if they live on the same superblock, > > but with your changes it's no longer true. > > Heh? I don't see that Darrick's work has anything to do with > copy_file_range(). It only touched vfs_copy_file_range() for > the ->clone_file_range() call, which Olga's patch protects with same sb > check. > > Thanks, > Amir. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-22 18:45 ` Olga Kornievskaia @ 2018-10-22 19:06 ` Matthew Wilcox 2018-10-22 19:34 ` Olga Kornievskaia 0 siblings, 1 reply; 39+ messages in thread From: Matthew Wilcox @ 2018-10-22 19:06 UTC (permalink / raw) To: Olga Kornievskaia Cc: Amir Goldstein, viro, linux-fsdevel, linux-nfs, fweimer, Steve French, Darrick J. Wong, Christoph Hellwig, Linux API On Mon, Oct 22, 2018 at 02:45:04PM -0400, Olga Kornievskaia wrote: > On Sat, Oct 20, 2018 at 4:54 AM Amir Goldstein <amir73il@gmail.com> wrote: > > Another thing is the commit message claims to: > > "Allow copy_file_range to copy between different superblocks but only > > of the same file system types" > > > > But what the patch actually does is: > > "Allow copy_file_range() syscall to copy between different filesystems > > AND allow calling the filesystems' copy_file_range() method > > between different superblocks but only of the same file system types" > > > > It's probably OK and quite useful to do the former, but maybe man page > > should be fixed to explicitly mention that the copy is expected to work > > across filesystems since kernel version XXX (?) > > > > If you don't wish to change cross filesystem type behavior and only > > relax cross super block limitation, then you should replace the > > same inode->i_sb check above with same inode->i_sb->s_type > > check instead of doing the check only for calling the filesystem > > copy_file_range() method. > > Thank you for the feedback. In the next version, I will remove the > check for the functions and instead check for the same file system > types. Jeff and I agree that this is the wrong way to go. Instead, the cross-device check should be in the individual instances, not the top level code. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-22 19:06 ` Matthew Wilcox @ 2018-10-22 19:34 ` Olga Kornievskaia 2018-10-22 19:48 ` Amir Goldstein 2018-10-22 23:39 ` Jeff Layton 0 siblings, 2 replies; 39+ messages in thread From: Olga Kornievskaia @ 2018-10-22 19:34 UTC (permalink / raw) To: willy Cc: Amir Goldstein, viro, linux-fsdevel, linux-nfs, fweimer, Steve French, Darrick J. Wong, Christoph Hellwig, Linux API On Mon, Oct 22, 2018 at 3:06 PM Matthew Wilcox <willy@infradead.org> wrote: > > On Mon, Oct 22, 2018 at 02:45:04PM -0400, Olga Kornievskaia wrote: > > On Sat, Oct 20, 2018 at 4:54 AM Amir Goldstein <amir73il@gmail.com> wrote: > > > Another thing is the commit message claims to: > > > "Allow copy_file_range to copy between different superblocks but only > > > of the same file system types" > > > > > > But what the patch actually does is: > > > "Allow copy_file_range() syscall to copy between different filesystems > > > AND allow calling the filesystems' copy_file_range() method > > > between different superblocks but only of the same file system types" > > > > > > It's probably OK and quite useful to do the former, but maybe man page > > > should be fixed to explicitly mention that the copy is expected to work > > > across filesystems since kernel version XXX (?) > > > > > > If you don't wish to change cross filesystem type behavior and only > > > relax cross super block limitation, then you should replace the > > > same inode->i_sb check above with same inode->i_sb->s_type > > > check instead of doing the check only for calling the filesystem > > > copy_file_range() method. > > > > Thank you for the feedback. In the next version, I will remove the > > check for the functions and instead check for the same file system > > types. > > Jeff and I agree that this is the wrong way to go. Instead, the > cross-device check should be in the individual instances, not the top > level code. So remove the check all together for the VFS (that was my original patch to begin with (like #1 not this one). So am I missing the point again, I keep getting different corrections every time. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-22 19:34 ` Olga Kornievskaia @ 2018-10-22 19:48 ` Amir Goldstein 2018-10-22 20:29 ` Matthew Wilcox 2018-10-22 23:39 ` Jeff Layton 1 sibling, 1 reply; 39+ messages in thread From: Amir Goldstein @ 2018-10-22 19:48 UTC (permalink / raw) To: Olga Kornievskaia Cc: Matthew Wilcox, Al Viro, linux-fsdevel, Linux NFS Mailing List, fweimer, Steve French, Darrick J. Wong, Christoph Hellwig, linux-api On Mon, Oct 22, 2018 at 10:35 PM Olga Kornievskaia <olga.kornievskaia@gmail.com> wrote: > > On Mon, Oct 22, 2018 at 3:06 PM Matthew Wilcox <willy@infradead.org> wrote: > > > > On Mon, Oct 22, 2018 at 02:45:04PM -0400, Olga Kornievskaia wrote: > > > On Sat, Oct 20, 2018 at 4:54 AM Amir Goldstein <amir73il@gmail.com> wrote: > > > > Another thing is the commit message claims to: > > > > "Allow copy_file_range to copy between different superblocks but only > > > > of the same file system types" > > > > > > > > But what the patch actually does is: > > > > "Allow copy_file_range() syscall to copy between different filesystems > > > > AND allow calling the filesystems' copy_file_range() method > > > > between different superblocks but only of the same file system types" > > > > > > > > It's probably OK and quite useful to do the former, but maybe man page > > > > should be fixed to explicitly mention that the copy is expected to work > > > > across filesystems since kernel version XXX (?) > > > > > > > > If you don't wish to change cross filesystem type behavior and only > > > > relax cross super block limitation, then you should replace the > > > > same inode->i_sb check above with same inode->i_sb->s_type > > > > check instead of doing the check only for calling the filesystem > > > > copy_file_range() method. > > > > > > Thank you for the feedback. In the next version, I will remove the > > > check for the functions and instead check for the same file system > > > types. > > > > Jeff and I agree that this is the wrong way to go. Instead, the > > cross-device check should be in the individual instances, not the top > > level code. > > So remove the check all together for the VFS (that was my original > patch to begin with (like #1 not this one). So am I missing the point > again, I keep getting different corrections every time. Because there are different opinions... although you did get the opinion of the VFS maintainer, which was: compare i_sb->s_type. Jeff, Matthew, really, what's the use of "allowing" cross fs type copy inside filesystem code? and which method is going to be called? file_out->f_op->copy_file_range()? file_in->f_op->copy_file_range()? Do we need to check if both are implemented? either? This is just confusing Olga and gives no real value to anyone. If we ever have a filesystem copy_file_range() method that can deal with cross fs type copy, we can change it then when we know the required semantics of that future call. That is not to say that we cannot relax same fs type from copy_file_range() syscall. That has already been done with the current patch, just not officially declared in commit message. Thanks, Amir. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-22 19:48 ` Amir Goldstein @ 2018-10-22 20:29 ` Matthew Wilcox 0 siblings, 0 replies; 39+ messages in thread From: Matthew Wilcox @ 2018-10-22 20:29 UTC (permalink / raw) To: Amir Goldstein Cc: Olga Kornievskaia, Al Viro, linux-fsdevel, Linux NFS Mailing List, fweimer, Steve French, Darrick J. Wong, Christoph Hellwig, linux-api On Mon, Oct 22, 2018 at 10:48:10PM +0300, Amir Goldstein wrote: > On Mon, Oct 22, 2018 at 10:35 PM Olga Kornievskaia > <olga.kornievskaia@gmail.com> wrote: > > So remove the check all together for the VFS (that was my original > > patch to begin with (like #1 not this one). So am I missing the point > > again, I keep getting different corrections every time. > > Because there are different opinions... although you did get the opinion > of the VFS maintainer, which was: compare i_sb->s_type. > > Jeff, Matthew, really, what's the use of "allowing" cross fs type copy inside > filesystem code? and which method is going to be called? > file_out->f_op->copy_file_range()? > file_in->f_op->copy_file_range()? The destination's method, as Olga originally had. > Do we need to check if both are implemented? either? > This is just confusing Olga and gives no real value to anyone. > If we ever have a filesystem copy_file_range() method that can deal > with cross fs type copy, we can change it then when we know the > required semantics of that future call. Wrong. Go back and read my reasoning earlier this thread. > That is not to say that we cannot relax same fs type from copy_file_range() > syscall. That has already been done with the current patch, just not officially > declared in commit message. > > Thanks, > Amir. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-22 19:34 ` Olga Kornievskaia 2018-10-22 19:48 ` Amir Goldstein @ 2018-10-22 23:39 ` Jeff Layton 2018-10-23 6:05 ` Amir Goldstein 1 sibling, 1 reply; 39+ messages in thread From: Jeff Layton @ 2018-10-22 23:39 UTC (permalink / raw) To: Olga Kornievskaia, willy Cc: Amir Goldstein, viro, linux-fsdevel, linux-nfs, fweimer, Steve French, Darrick J. Wong, Christoph Hellwig, Linux API On Mon, 2018-10-22 at 15:34 -0400, Olga Kornievskaia wrote: > On Mon, Oct 22, 2018 at 3:06 PM Matthew Wilcox <willy@infradead.org> wrote: > > > > On Mon, Oct 22, 2018 at 02:45:04PM -0400, Olga Kornievskaia wrote: > > > On Sat, Oct 20, 2018 at 4:54 AM Amir Goldstein <amir73il@gmail.com> wrote: > > > > Another thing is the commit message claims to: > > > > "Allow copy_file_range to copy between different superblocks but only > > > > of the same file system types" > > > > > > > > But what the patch actually does is: > > > > "Allow copy_file_range() syscall to copy between different filesystems > > > > AND allow calling the filesystems' copy_file_range() method > > > > between different superblocks but only of the same file system types" > > > > > > > > It's probably OK and quite useful to do the former, but maybe man page > > > > should be fixed to explicitly mention that the copy is expected to work > > > > across filesystems since kernel version XXX (?) > > > > > > > > If you don't wish to change cross filesystem type behavior and only > > > > relax cross super block limitation, then you should replace the > > > > same inode->i_sb check above with same inode->i_sb->s_type > > > > check instead of doing the check only for calling the filesystem > > > > copy_file_range() method. > > > > > > Thank you for the feedback. In the next version, I will remove the > > > check for the functions and instead check for the same file system > > > types. > > > > Jeff and I agree that this is the wrong way to go. Instead, the > > cross-device check should be in the individual instances, not the top > > level code. > > So remove the check all together for the VFS (that was my original > patch to begin with (like #1 not this one). So am I missing the point > again, I keep getting different corrections every time. Sorry if I wasn't clear before: Basically, I think Willy and I are both envisioning that some copy_file_range implementations may eventually not be subject to the limitations of the checks you're adding. All of the current and currently proposed ones are however, so we need these checks in place. We have two options. We can either do them globally at the vfs layer or in the individual filesystem "drivers". I argue that it's better to do it at the driver level, because if we ever want to implement one that is not subject to these limitations, you'll effectively have to push these checks down into the drivers later. That's where things become ugly for backports and such. You'll basically be changing a subtle expectation in the driver interface, which could be a source of bugs later. If we set the expectation now that the drivers need to do this checking then we can (hopefully) ensure that they all do before they're ever merged. -- Jeff Layton <jlayton@redhat.com> ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-22 23:39 ` Jeff Layton @ 2018-10-23 6:05 ` Amir Goldstein 2018-10-23 15:03 ` Olga Kornievskaia 0 siblings, 1 reply; 39+ messages in thread From: Amir Goldstein @ 2018-10-23 6:05 UTC (permalink / raw) To: Jeff Layton Cc: Olga Kornievskaia, Matthew Wilcox, Al Viro, linux-fsdevel, Linux NFS Mailing List, fweimer, Steve French, Darrick J. Wong, Christoph Hellwig, linux-api On Tue, Oct 23, 2018 at 2:39 AM Jeff Layton <jlayton@redhat.com> wrote: > > On Mon, 2018-10-22 at 15:34 -0400, Olga Kornievskaia wrote: > > On Mon, Oct 22, 2018 at 3:06 PM Matthew Wilcox <willy@infradead.org> wrote: > > > > > > On Mon, Oct 22, 2018 at 02:45:04PM -0400, Olga Kornievskaia wrote: > > > > On Sat, Oct 20, 2018 at 4:54 AM Amir Goldstein <amir73il@gmail.com> wrote: > > > > > Another thing is the commit message claims to: > > > > > "Allow copy_file_range to copy between different superblocks but only > > > > > of the same file system types" > > > > > > > > > > But what the patch actually does is: > > > > > "Allow copy_file_range() syscall to copy between different filesystems > > > > > AND allow calling the filesystems' copy_file_range() method > > > > > between different superblocks but only of the same file system types" > > > > > > > > > > It's probably OK and quite useful to do the former, but maybe man page > > > > > should be fixed to explicitly mention that the copy is expected to work > > > > > across filesystems since kernel version XXX (?) > > > > > > > > > > If you don't wish to change cross filesystem type behavior and only > > > > > relax cross super block limitation, then you should replace the > > > > > same inode->i_sb check above with same inode->i_sb->s_type > > > > > check instead of doing the check only for calling the filesystem > > > > > copy_file_range() method. > > > > > > > > Thank you for the feedback. In the next version, I will remove the > > > > check for the functions and instead check for the same file system > > > > types. > > > > > > Jeff and I agree that this is the wrong way to go. Instead, the > > > cross-device check should be in the individual instances, not the top > > > level code. > > > > So remove the check all together for the VFS (that was my original > > patch to begin with (like #1 not this one). So am I missing the point > > again, I keep getting different corrections every time. > > Sorry if I wasn't clear before: > > Basically, I think Willy and I are both envisioning that some > copy_file_range implementations may eventually not be subject to the > limitations of the checks you're adding. > Yes. Eventually. And even Matthew is (quote) "dubious" about that ever happening. Changing the interface as Matthew proposed has a price and we need to compare this price to the alleged backporting price that nobody may ever need to pay. As far as I can tell, passing a struct file * on a file_operations method that does not belong to that filesystem in unprecedented (*) and is a far more lethal landmine than the alleged backporting landmine. (*) prior to v4.19-rc1, filesystems could get an overlayfs file, but file_inode(file) has always belonged to the filesystem. Olga, I do not strongly object to Matthew's proposal, so don't feel obligated to choose my side of the argument. I am just trying to offer a different perspective. In any case, my outstanding concerns with the patch are: 1. If you change syscall to support cross fs type copy (which is good IMO) need to document that in commit message and possibly follow up later with a note in man page 2. Commit message says: "This feature was of interest of ... NFS" "This feature is needed by NFSv4.2..." "NFS will allow for copies between different NFS servers." It is not clear to me if we are talking about present of future NFSv4.2 code. If NFSv4.2 currently does not support cross sb copy (??) than your patch need to enforce same sb in nfs4_copy_file_range(). If it does support cross sb copy than please edit the commit message to make that clear. Thanks, Amir. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-23 6:05 ` Amir Goldstein @ 2018-10-23 15:03 ` Olga Kornievskaia 2018-10-23 15:30 ` Olga Kornievskaia 2018-10-23 15:39 ` Matthew Wilcox 0 siblings, 2 replies; 39+ messages in thread From: Olga Kornievskaia @ 2018-10-23 15:03 UTC (permalink / raw) To: Amir Goldstein Cc: Jeff Layton, willy, viro, linux-fsdevel, linux-nfs, fweimer, Steve French, Darrick J. Wong, Christoph Hellwig, Linux API On Tue, Oct 23, 2018 at 2:05 AM Amir Goldstein <amir73il@gmail.com> wrote: > > On Tue, Oct 23, 2018 at 2:39 AM Jeff Layton <jlayton@redhat.com> wrote: > > > > On Mon, 2018-10-22 at 15:34 -0400, Olga Kornievskaia wrote: > > > On Mon, Oct 22, 2018 at 3:06 PM Matthew Wilcox <willy@infradead.org> wrote: > > > > > > > > On Mon, Oct 22, 2018 at 02:45:04PM -0400, Olga Kornievskaia wrote: > > > > > On Sat, Oct 20, 2018 at 4:54 AM Amir Goldstein <amir73il@gmail.com> wrote: > > > > > > Another thing is the commit message claims to: > > > > > > "Allow copy_file_range to copy between different superblocks but only > > > > > > of the same file system types" > > > > > > > > > > > > But what the patch actually does is: > > > > > > "Allow copy_file_range() syscall to copy between different filesystems > > > > > > AND allow calling the filesystems' copy_file_range() method > > > > > > between different superblocks but only of the same file system types" > > > > > > > > > > > > It's probably OK and quite useful to do the former, but maybe man page > > > > > > should be fixed to explicitly mention that the copy is expected to work > > > > > > across filesystems since kernel version XXX (?) > > > > > > > > > > > > If you don't wish to change cross filesystem type behavior and only > > > > > > relax cross super block limitation, then you should replace the > > > > > > same inode->i_sb check above with same inode->i_sb->s_type > > > > > > check instead of doing the check only for calling the filesystem > > > > > > copy_file_range() method. > > > > > > > > > > Thank you for the feedback. In the next version, I will remove the > > > > > check for the functions and instead check for the same file system > > > > > types. > > > > > > > > Jeff and I agree that this is the wrong way to go. Instead, the > > > > cross-device check should be in the individual instances, not the top > > > > level code. > > > > > > So remove the check all together for the VFS (that was my original > > > patch to begin with (like #1 not this one). So am I missing the point > > > again, I keep getting different corrections every time. > > > > Sorry if I wasn't clear before: > > > > Basically, I think Willy and I are both envisioning that some > > copy_file_range implementations may eventually not be subject to the > > limitations of the checks you're adding. > > > > Yes. Eventually. And even Matthew is (quote) "dubious" about that ever > happening. Changing the interface as Matthew proposed has a price > and we need to compare this price to the alleged backporting price > that nobody may ever need to pay. > > As far as I can tell, passing a struct file * on a file_operations method > that does not belong to that filesystem in unprecedented (*) and is a far > more lethal landmine than the alleged backporting landmine. > > (*) prior to v4.19-rc1, filesystems could get an overlayfs file, but > file_inode(file) has always belonged to the filesystem. > > Olga, > > I do not strongly object to Matthew's proposal, so don't feel > obligated to choose my side of the argument. I am just trying > to offer a different perspective. > > In any case, my outstanding concerns with the patch are: > > 1. If you change syscall to support cross fs type copy (which is > good IMO) need to document that in commit message > and possibly follow up later with a note in man page > > 2. Commit message says: > "This feature was of interest of ... NFS" > "This feature is needed by NFSv4.2..." > "NFS will allow for copies between different NFS servers." > It is not clear to me if we are talking about present of future > NFSv4.2 code. If NFSv4.2 currently does not support cross > sb copy (??) than your patch need to enforce same sb > in nfs4_copy_file_range(). If it does support cross sb copy > than please edit the commit message to make that clear. I personally agree with Amir. I think it's far fetched that a file system would know how to handle something that's not of its type. When the copy_file_range() was checked in, there was a comment above the superblock check saying "we might want to relax this in the future". It deemed appropriate then to enforce the check since none of the file systems used it. Now, the future is here, and we are removing the check but proposing a different once because again the future isn't here and having a single check simplifies the code. But I don't feel strongly about the check (or rather the location of it VFS vs each FS) and what I ultimately need is to removed same sb check. It sounds like if Amir isn't objecting, then the check for same file system type should be removed from VFS. And, for each of the file systems that currently support copy_file_range() -- CIFS, NFS, and overlayfs -- I need to modify them and add a check for the same fs_type. Amir to answer your question, only NFSv4.2 has copy_offload functionality (not earlier NFS versions). Furthermore, existing upstream only supports same sb copy offload. What this patch series adds is support for copy offload across different superblocks but NFS will not support (and would need a check) for copy offload across different file system types. Also I kinda stand behind the ideas stated: 1) this is of interest to NFS (where NFS here is to represent a community, and CIFS is used to represent the other community). 2) NFSv4.2 copy offload a specific feature that needs this functionality. 3rd statement is bad. Only NFSv4.2 will allow copies between different NFS servers (ie., after this patch +series), the emphasis was on "will allow" meaning what this patch will allow to be done (ie, patch's purpose). Or also, if the NFS server exports different exports, then a copy between them (assuming exports of the same file system type). In the next version of the patch, I'll do my best to specified what changed as the consequence of removing the cross sb check (ie, file system types of the passed in file can be from different file systems). I will add wording to the man page and add the suggested wording to the "porting" file. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-23 15:03 ` Olga Kornievskaia @ 2018-10-23 15:30 ` Olga Kornievskaia 2018-10-23 17:16 ` Olga Kornievskaia 2018-10-23 15:39 ` Matthew Wilcox 1 sibling, 1 reply; 39+ messages in thread From: Olga Kornievskaia @ 2018-10-23 15:30 UTC (permalink / raw) To: Amir Goldstein Cc: Jeff Layton, willy, viro, linux-fsdevel, linux-nfs, fweimer, Steve French, Darrick J. Wong, Christoph Hellwig, Linux API On Tue, Oct 23, 2018 at 11:03 AM Olga Kornievskaia <olga.kornievskaia@gmail.com> wrote: > > On Tue, Oct 23, 2018 at 2:05 AM Amir Goldstein <amir73il@gmail.com> wrote: > > > > On Tue, Oct 23, 2018 at 2:39 AM Jeff Layton <jlayton@redhat.com> wrote: > > > > > > On Mon, 2018-10-22 at 15:34 -0400, Olga Kornievskaia wrote: > > > > On Mon, Oct 22, 2018 at 3:06 PM Matthew Wilcox <willy@infradead.org> wrote: > > > > > > > > > > On Mon, Oct 22, 2018 at 02:45:04PM -0400, Olga Kornievskaia wrote: > > > > > > On Sat, Oct 20, 2018 at 4:54 AM Amir Goldstein <amir73il@gmail.com> wrote: > > > > > > > Another thing is the commit message claims to: > > > > > > > "Allow copy_file_range to copy between different superblocks but only > > > > > > > of the same file system types" > > > > > > > > > > > > > > But what the patch actually does is: > > > > > > > "Allow copy_file_range() syscall to copy between different filesystems > > > > > > > AND allow calling the filesystems' copy_file_range() method > > > > > > > between different superblocks but only of the same file system types" > > > > > > > > > > > > > > It's probably OK and quite useful to do the former, but maybe man page > > > > > > > should be fixed to explicitly mention that the copy is expected to work > > > > > > > across filesystems since kernel version XXX (?) > > > > > > > > > > > > > > If you don't wish to change cross filesystem type behavior and only > > > > > > > relax cross super block limitation, then you should replace the > > > > > > > same inode->i_sb check above with same inode->i_sb->s_type > > > > > > > check instead of doing the check only for calling the filesystem > > > > > > > copy_file_range() method. > > > > > > > > > > > > Thank you for the feedback. In the next version, I will remove the > > > > > > check for the functions and instead check for the same file system > > > > > > types. > > > > > > > > > > Jeff and I agree that this is the wrong way to go. Instead, the > > > > > cross-device check should be in the individual instances, not the top > > > > > level code. > > > > > > > > So remove the check all together for the VFS (that was my original > > > > patch to begin with (like #1 not this one). So am I missing the point > > > > again, I keep getting different corrections every time. > > > > > > Sorry if I wasn't clear before: > > > > > > Basically, I think Willy and I are both envisioning that some > > > copy_file_range implementations may eventually not be subject to the > > > limitations of the checks you're adding. > > > > > > > Yes. Eventually. And even Matthew is (quote) "dubious" about that ever > > happening. Changing the interface as Matthew proposed has a price > > and we need to compare this price to the alleged backporting price > > that nobody may ever need to pay. > > > > As far as I can tell, passing a struct file * on a file_operations method > > that does not belong to that filesystem in unprecedented (*) and is a far > > more lethal landmine than the alleged backporting landmine. > > > > (*) prior to v4.19-rc1, filesystems could get an overlayfs file, but > > file_inode(file) has always belonged to the filesystem. > > > > Olga, > > > > I do not strongly object to Matthew's proposal, so don't feel > > obligated to choose my side of the argument. I am just trying > > to offer a different perspective. > > > > In any case, my outstanding concerns with the patch are: > > > > 1. If you change syscall to support cross fs type copy (which is > > good IMO) need to document that in commit message > > and possibly follow up later with a note in man page > > > > 2. Commit message says: > > "This feature was of interest of ... NFS" > > "This feature is needed by NFSv4.2..." > > "NFS will allow for copies between different NFS servers." > > It is not clear to me if we are talking about present of future > > NFSv4.2 code. If NFSv4.2 currently does not support cross > > sb copy (??) than your patch need to enforce same sb > > in nfs4_copy_file_range(). If it does support cross sb copy > > than please edit the commit message to make that clear. > > I personally agree with Amir. I think it's far fetched that a file > system would know how to handle something that's not of its type. When > the copy_file_range() was checked in, there was a comment above the > superblock check saying "we might want to relax this in the future". > It deemed appropriate then to enforce the check since none of the file > systems used it. Now, the future is here, and we are removing the > check but proposing a different once because again the future isn't > here and having a single check simplifies the code. Sorry Ok I wrote this too fast. I think I'm changing my mind and siding with the check by the file system. > But I don't feel strongly about the check (or rather the location of > it VFS vs each FS) and what I ultimately need is to removed same sb > check. It sounds like if Amir isn't objecting, then the check for same > file system type should be removed from VFS. And, for each of the file > systems that currently support copy_file_range() -- CIFS, NFS, and > overlayfs -- I need to modify them and add a check for the same > fs_type. > > Amir to answer your question, only NFSv4.2 has copy_offload > functionality (not earlier NFS versions). Furthermore, existing > upstream only supports same sb copy offload. What this patch series > adds is support for copy offload across different superblocks but NFS > will not support (and would need a check) for copy offload across > different file system types. Also I kinda stand behind the ideas > stated: 1) this is of interest to NFS (where NFS here is to represent > a community, and CIFS is used to represent the other community). 2) > NFSv4.2 copy offload a specific feature that needs this functionality. > 3rd statement is bad. Only NFSv4.2 will allow copies between different > NFS servers (ie., after this patch +series), the emphasis was on "will > allow" meaning what this patch will allow to be done (ie, patch's > purpose). Or also, if the NFS server exports different exports, then a > copy between them (assuming exports of the same file system type). > > In the next version of the patch, I'll do my best to specified what > changed as the consequence of removing the cross sb check (ie, file > system types of the passed in file can be from different file > systems). I will add wording to the man page and add the suggested > wording to the "porting" file. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-23 15:30 ` Olga Kornievskaia @ 2018-10-23 17:16 ` Olga Kornievskaia 2018-10-24 11:17 ` Jeff Layton 0 siblings, 1 reply; 39+ messages in thread From: Olga Kornievskaia @ 2018-10-23 17:16 UTC (permalink / raw) To: Amir Goldstein Cc: Jeff Layton, willy, viro, linux-fsdevel, linux-nfs, fweimer, Steve French, Darrick J. Wong, Christoph Hellwig, Linux API On Tue, Oct 23, 2018 at 11:30 AM Olga Kornievskaia <olga.kornievskaia@gmail.com> wrote: > > On Tue, Oct 23, 2018 at 11:03 AM Olga Kornievskaia > <olga.kornievskaia@gmail.com> wrote: > > > > On Tue, Oct 23, 2018 at 2:05 AM Amir Goldstein <amir73il@gmail.com> wrote: > > > > > > On Tue, Oct 23, 2018 at 2:39 AM Jeff Layton <jlayton@redhat.com> wrote: > > > > > > > > On Mon, 2018-10-22 at 15:34 -0400, Olga Kornievskaia wrote: > > > > > On Mon, Oct 22, 2018 at 3:06 PM Matthew Wilcox <willy@infradead.org> wrote: > > > > > > > > > > > > On Mon, Oct 22, 2018 at 02:45:04PM -0400, Olga Kornievskaia wrote: > > > > > > > On Sat, Oct 20, 2018 at 4:54 AM Amir Goldstein <amir73il@gmail.com> wrote: > > > > > > > > Another thing is the commit message claims to: > > > > > > > > "Allow copy_file_range to copy between different superblocks but only > > > > > > > > of the same file system types" > > > > > > > > > > > > > > > > But what the patch actually does is: > > > > > > > > "Allow copy_file_range() syscall to copy between different filesystems > > > > > > > > AND allow calling the filesystems' copy_file_range() method > > > > > > > > between different superblocks but only of the same file system types" > > > > > > > > > > > > > > > > It's probably OK and quite useful to do the former, but maybe man page > > > > > > > > should be fixed to explicitly mention that the copy is expected to work > > > > > > > > across filesystems since kernel version XXX (?) > > > > > > > > > > > > > > > > If you don't wish to change cross filesystem type behavior and only > > > > > > > > relax cross super block limitation, then you should replace the > > > > > > > > same inode->i_sb check above with same inode->i_sb->s_type > > > > > > > > check instead of doing the check only for calling the filesystem > > > > > > > > copy_file_range() method. > > > > > > > > > > > > > > Thank you for the feedback. In the next version, I will remove the > > > > > > > check for the functions and instead check for the same file system > > > > > > > types. > > > > > > > > > > > > Jeff and I agree that this is the wrong way to go. Instead, the > > > > > > cross-device check should be in the individual instances, not the top > > > > > > level code. > > > > > > > > > > So remove the check all together for the VFS (that was my original > > > > > patch to begin with (like #1 not this one). So am I missing the point > > > > > again, I keep getting different corrections every time. > > > > > > > > Sorry if I wasn't clear before: > > > > > > > > Basically, I think Willy and I are both envisioning that some > > > > copy_file_range implementations may eventually not be subject to the > > > > limitations of the checks you're adding. > > > > > > > > > > Yes. Eventually. And even Matthew is (quote) "dubious" about that ever > > > happening. Changing the interface as Matthew proposed has a price > > > and we need to compare this price to the alleged backporting price > > > that nobody may ever need to pay. > > > > > > As far as I can tell, passing a struct file * on a file_operations method > > > that does not belong to that filesystem in unprecedented (*) and is a far > > > more lethal landmine than the alleged backporting landmine. > > > > > > (*) prior to v4.19-rc1, filesystems could get an overlayfs file, but > > > file_inode(file) has always belonged to the filesystem. > > > > > > Olga, > > > > > > I do not strongly object to Matthew's proposal, so don't feel > > > obligated to choose my side of the argument. I am just trying > > > to offer a different perspective. > > > > > > In any case, my outstanding concerns with the patch are: > > > > > > 1. If you change syscall to support cross fs type copy (which is > > > good IMO) need to document that in commit message > > > and possibly follow up later with a note in man page > > > > > > 2. Commit message says: > > > "This feature was of interest of ... NFS" > > > "This feature is needed by NFSv4.2..." > > > "NFS will allow for copies between different NFS servers." > > > It is not clear to me if we are talking about present of future > > > NFSv4.2 code. If NFSv4.2 currently does not support cross > > > sb copy (??) than your patch need to enforce same sb > > > in nfs4_copy_file_range(). If it does support cross sb copy > > > than please edit the commit message to make that clear. > > > > I personally agree with Amir. I think it's far fetched that a file > > system would know how to handle something that's not of its type. When > > the copy_file_range() was checked in, there was a comment above the > > superblock check saying "we might want to relax this in the future". > > It deemed appropriate then to enforce the check since none of the file > > systems used it. Now, the future is here, and we are removing the > > check but proposing a different once because again the future isn't > > here and having a single check simplifies the code. > > Sorry Ok I wrote this too fast. I think I'm changing my mind and > siding with the check by the file system. The reason I think we should remove the check all together is we'd allow the callers of copy_file_range() to utilize the do_splice_direct() between file system types even when copy_file_range() doesn't support cross fs copy. Isn't that beneficial? > > But I don't feel strongly about the check (or rather the location of > > it VFS vs each FS) and what I ultimately need is to removed same sb > > check. It sounds like if Amir isn't objecting, then the check for same > > file system type should be removed from VFS. And, for each of the file > > systems that currently support copy_file_range() -- CIFS, NFS, and > > overlayfs -- I need to modify them and add a check for the same > > fs_type. > > > > Amir to answer your question, only NFSv4.2 has copy_offload > > functionality (not earlier NFS versions). Furthermore, existing > > upstream only supports same sb copy offload. What this patch series > > adds is support for copy offload across different superblocks but NFS > > will not support (and would need a check) for copy offload across > > different file system types. Also I kinda stand behind the ideas > > stated: 1) this is of interest to NFS (where NFS here is to represent > > a community, and CIFS is used to represent the other community). 2) > > NFSv4.2 copy offload a specific feature that needs this functionality. > > 3rd statement is bad. Only NFSv4.2 will allow copies between different > > NFS servers (ie., after this patch +series), the emphasis was on "will > > allow" meaning what this patch will allow to be done (ie, patch's > > purpose). Or also, if the NFS server exports different exports, then a > > copy between them (assuming exports of the same file system type). > > > > In the next version of the patch, I'll do my best to specified what > > changed as the consequence of removing the cross sb check (ie, file > > system types of the passed in file can be from different file > > systems). I will add wording to the man page and add the suggested > > wording to the "porting" file. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-23 17:16 ` Olga Kornievskaia @ 2018-10-24 11:17 ` Jeff Layton 2018-10-24 19:59 ` Olga Kornievskaia 0 siblings, 1 reply; 39+ messages in thread From: Jeff Layton @ 2018-10-24 11:17 UTC (permalink / raw) To: Olga Kornievskaia, Amir Goldstein Cc: willy, viro, linux-fsdevel, linux-nfs, fweimer, Steve French, Darrick J. Wong, Christoph Hellwig, Linux API On Tue, 2018-10-23 at 13:16 -0400, Olga Kornievskaia wrote: > On Tue, Oct 23, 2018 at 11:30 AM Olga Kornievskaia > <olga.kornievskaia@gmail.com> wrote: > > > > On Tue, Oct 23, 2018 at 11:03 AM Olga Kornievskaia > > <olga.kornievskaia@gmail.com> wrote: > > > > > > On Tue, Oct 23, 2018 at 2:05 AM Amir Goldstein <amir73il@gmail.com> wrote: > > > > > > > > On Tue, Oct 23, 2018 at 2:39 AM Jeff Layton <jlayton@redhat.com> wrote: > > > > > > > > > > On Mon, 2018-10-22 at 15:34 -0400, Olga Kornievskaia wrote: > > > > > > On Mon, Oct 22, 2018 at 3:06 PM Matthew Wilcox <willy@infradead.org> wrote: > > > > > > > > > > > > > > On Mon, Oct 22, 2018 at 02:45:04PM -0400, Olga Kornievskaia wrote: > > > > > > > > On Sat, Oct 20, 2018 at 4:54 AM Amir Goldstein <amir73il@gmail.com> wrote: > > > > > > > > > Another thing is the commit message claims to: > > > > > > > > > "Allow copy_file_range to copy between different superblocks but only > > > > > > > > > of the same file system types" > > > > > > > > > > > > > > > > > > But what the patch actually does is: > > > > > > > > > "Allow copy_file_range() syscall to copy between different filesystems > > > > > > > > > AND allow calling the filesystems' copy_file_range() method > > > > > > > > > between different superblocks but only of the same file system types" > > > > > > > > > > > > > > > > > > It's probably OK and quite useful to do the former, but maybe man page > > > > > > > > > should be fixed to explicitly mention that the copy is expected to work > > > > > > > > > across filesystems since kernel version XXX (?) > > > > > > > > > > > > > > > > > > If you don't wish to change cross filesystem type behavior and only > > > > > > > > > relax cross super block limitation, then you should replace the > > > > > > > > > same inode->i_sb check above with same inode->i_sb->s_type > > > > > > > > > check instead of doing the check only for calling the filesystem > > > > > > > > > copy_file_range() method. > > > > > > > > > > > > > > > > Thank you for the feedback. In the next version, I will remove the > > > > > > > > check for the functions and instead check for the same file system > > > > > > > > types. > > > > > > > > > > > > > > Jeff and I agree that this is the wrong way to go. Instead, the > > > > > > > cross-device check should be in the individual instances, not the top > > > > > > > level code. > > > > > > > > > > > > So remove the check all together for the VFS (that was my original > > > > > > patch to begin with (like #1 not this one). So am I missing the point > > > > > > again, I keep getting different corrections every time. > > > > > > > > > > Sorry if I wasn't clear before: > > > > > > > > > > Basically, I think Willy and I are both envisioning that some > > > > > copy_file_range implementations may eventually not be subject to the > > > > > limitations of the checks you're adding. > > > > > > > > > > > > > Yes. Eventually. And even Matthew is (quote) "dubious" about that ever > > > > happening. Changing the interface as Matthew proposed has a price > > > > and we need to compare this price to the alleged backporting price > > > > that nobody may ever need to pay. > > > > > > > > As far as I can tell, passing a struct file * on a file_operations method > > > > that does not belong to that filesystem in unprecedented (*) and is a far > > > > more lethal landmine than the alleged backporting landmine. > > > > > > > > (*) prior to v4.19-rc1, filesystems could get an overlayfs file, but > > > > file_inode(file) has always belonged to the filesystem. > > > > > > > > Olga, > > > > > > > > I do not strongly object to Matthew's proposal, so don't feel > > > > obligated to choose my side of the argument. I am just trying > > > > to offer a different perspective. > > > > > > > > In any case, my outstanding concerns with the patch are: > > > > > > > > 1. If you change syscall to support cross fs type copy (which is > > > > good IMO) need to document that in commit message > > > > and possibly follow up later with a note in man page > > > > > > > > 2. Commit message says: > > > > "This feature was of interest of ... NFS" > > > > "This feature is needed by NFSv4.2..." > > > > "NFS will allow for copies between different NFS servers." > > > > It is not clear to me if we are talking about present of future > > > > NFSv4.2 code. If NFSv4.2 currently does not support cross > > > > sb copy (??) than your patch need to enforce same sb > > > > in nfs4_copy_file_range(). If it does support cross sb copy > > > > than please edit the commit message to make that clear. > > > > > > I personally agree with Amir. I think it's far fetched that a file > > > system would know how to handle something that's not of its type. When > > > the copy_file_range() was checked in, there was a comment above the > > > superblock check saying "we might want to relax this in the future". > > > It deemed appropriate then to enforce the check since none of the file > > > systems used it. Now, the future is here, and we are removing the > > > check but proposing a different once because again the future isn't > > > here and having a single check simplifies the code. > > > > Sorry Ok I wrote this too fast. I think I'm changing my mind and > > siding with the check by the file system. > > The reason I think we should remove the check all together is we'd > allow the callers of copy_file_range() to utilize the > do_splice_direct() between file system types even when > copy_file_range() doesn't support cross fs copy. Isn't that > beneficial? Others might argue no, but I think it's good to have that option open. If the consensus is that we do need a check at the vfs layer to validate that the struct files come from the same fstype (as Amir suggests), then I can live with that. We've dealt with internal API changes before so we can do it again. Al is quite correct though that we really do want to compare fstypes rather than op pointers in that case however. > > > But I don't feel strongly about the check (or rather the location of > > > it VFS vs each FS) and what I ultimately need is to removed same sb > > > check. It sounds like if Amir isn't objecting, then the check for same > > > file system type should be removed from VFS. And, for each of the file > > > systems that currently support copy_file_range() -- CIFS, NFS, and > > > overlayfs -- I need to modify them and add a check for the same > > > fs_type. > > > > > > Amir to answer your question, only NFSv4.2 has copy_offload > > > functionality (not earlier NFS versions). Furthermore, existing > > > upstream only supports same sb copy offload. What this patch series > > > adds is support for copy offload across different superblocks but NFS > > > will not support (and would need a check) for copy offload across > > > different file system types. Also I kinda stand behind the ideas > > > stated: 1) this is of interest to NFS (where NFS here is to represent > > > a community, and CIFS is used to represent the other community). 2) > > > NFSv4.2 copy offload a specific feature that needs this functionality. > > > 3rd statement is bad. Only NFSv4.2 will allow copies between different > > > NFS servers (ie., after this patch +series), the emphasis was on "will > > > allow" meaning what this patch will allow to be done (ie, patch's > > > purpose). Or also, if the NFS server exports different exports, then a > > > copy between them (assuming exports of the same file system type). > > > > > > In the next version of the patch, I'll do my best to specified what > > > changed as the consequence of removing the cross sb check (ie, file > > > system types of the passed in file can be from different file > > > systems). I will add wording to the man page and add the suggested > > > wording to the "porting" file. -- Jeff Layton <jlayton@redhat.com> ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-24 11:17 ` Jeff Layton @ 2018-10-24 19:59 ` Olga Kornievskaia 2018-10-25 4:58 ` Amir Goldstein 0 siblings, 1 reply; 39+ messages in thread From: Olga Kornievskaia @ 2018-10-24 19:59 UTC (permalink / raw) To: Jeff Layton Cc: Amir Goldstein, willy, viro, linux-fsdevel, linux-nfs, fweimer, Steve French, Darrick J. Wong, Christoph Hellwig, Linux API On Wed, Oct 24, 2018 at 7:17 AM Jeff Layton <jlayton@redhat.com> wrote: > > On Tue, 2018-10-23 at 13:16 -0400, Olga Kornievskaia wrote: > > On Tue, Oct 23, 2018 at 11:30 AM Olga Kornievskaia > > <olga.kornievskaia@gmail.com> wrote: > > > > > > On Tue, Oct 23, 2018 at 11:03 AM Olga Kornievskaia > > > <olga.kornievskaia@gmail.com> wrote: > > > > > > > > On Tue, Oct 23, 2018 at 2:05 AM Amir Goldstein <amir73il@gmail.com> wrote: > > > > > > > > > > On Tue, Oct 23, 2018 at 2:39 AM Jeff Layton <jlayton@redhat.com> wrote: > > > > > > > > > > > > On Mon, 2018-10-22 at 15:34 -0400, Olga Kornievskaia wrote: > > > > > > > On Mon, Oct 22, 2018 at 3:06 PM Matthew Wilcox <willy@infradead.org> wrote: > > > > > > > > > > > > > > > > On Mon, Oct 22, 2018 at 02:45:04PM -0400, Olga Kornievskaia wrote: > > > > > > > > > On Sat, Oct 20, 2018 at 4:54 AM Amir Goldstein <amir73il@gmail.com> wrote: > > > > > > > > > > Another thing is the commit message claims to: > > > > > > > > > > "Allow copy_file_range to copy between different superblocks but only > > > > > > > > > > of the same file system types" > > > > > > > > > > > > > > > > > > > > But what the patch actually does is: > > > > > > > > > > "Allow copy_file_range() syscall to copy between different filesystems > > > > > > > > > > AND allow calling the filesystems' copy_file_range() method > > > > > > > > > > between different superblocks but only of the same file system types" > > > > > > > > > > > > > > > > > > > > It's probably OK and quite useful to do the former, but maybe man page > > > > > > > > > > should be fixed to explicitly mention that the copy is expected to work > > > > > > > > > > across filesystems since kernel version XXX (?) > > > > > > > > > > > > > > > > > > > > If you don't wish to change cross filesystem type behavior and only > > > > > > > > > > relax cross super block limitation, then you should replace the > > > > > > > > > > same inode->i_sb check above with same inode->i_sb->s_type > > > > > > > > > > check instead of doing the check only for calling the filesystem > > > > > > > > > > copy_file_range() method. > > > > > > > > > > > > > > > > > > Thank you for the feedback. In the next version, I will remove the > > > > > > > > > check for the functions and instead check for the same file system > > > > > > > > > types. > > > > > > > > > > > > > > > > Jeff and I agree that this is the wrong way to go. Instead, the > > > > > > > > cross-device check should be in the individual instances, not the top > > > > > > > > level code. > > > > > > > > > > > > > > So remove the check all together for the VFS (that was my original > > > > > > > patch to begin with (like #1 not this one). So am I missing the point > > > > > > > again, I keep getting different corrections every time. > > > > > > > > > > > > Sorry if I wasn't clear before: > > > > > > > > > > > > Basically, I think Willy and I are both envisioning that some > > > > > > copy_file_range implementations may eventually not be subject to the > > > > > > limitations of the checks you're adding. > > > > > > > > > > > > > > > > Yes. Eventually. And even Matthew is (quote) "dubious" about that ever > > > > > happening. Changing the interface as Matthew proposed has a price > > > > > and we need to compare this price to the alleged backporting price > > > > > that nobody may ever need to pay. > > > > > > > > > > As far as I can tell, passing a struct file * on a file_operations method > > > > > that does not belong to that filesystem in unprecedented (*) and is a far > > > > > more lethal landmine than the alleged backporting landmine. > > > > > > > > > > (*) prior to v4.19-rc1, filesystems could get an overlayfs file, but > > > > > file_inode(file) has always belonged to the filesystem. > > > > > > > > > > Olga, > > > > > > > > > > I do not strongly object to Matthew's proposal, so don't feel > > > > > obligated to choose my side of the argument. I am just trying > > > > > to offer a different perspective. > > > > > > > > > > In any case, my outstanding concerns with the patch are: > > > > > > > > > > 1. If you change syscall to support cross fs type copy (which is > > > > > good IMO) need to document that in commit message > > > > > and possibly follow up later with a note in man page > > > > > > > > > > 2. Commit message says: > > > > > "This feature was of interest of ... NFS" > > > > > "This feature is needed by NFSv4.2..." > > > > > "NFS will allow for copies between different NFS servers." > > > > > It is not clear to me if we are talking about present of future > > > > > NFSv4.2 code. If NFSv4.2 currently does not support cross > > > > > sb copy (??) than your patch need to enforce same sb > > > > > in nfs4_copy_file_range(). If it does support cross sb copy > > > > > than please edit the commit message to make that clear. > > > > > > > > I personally agree with Amir. I think it's far fetched that a file > > > > system would know how to handle something that's not of its type. When > > > > the copy_file_range() was checked in, there was a comment above the > > > > superblock check saying "we might want to relax this in the future". > > > > It deemed appropriate then to enforce the check since none of the file > > > > systems used it. Now, the future is here, and we are removing the > > > > check but proposing a different once because again the future isn't > > > > here and having a single check simplifies the code. > > > > > > Sorry Ok I wrote this too fast. I think I'm changing my mind and > > > siding with the check by the file system. > > > > The reason I think we should remove the check all together is we'd > > allow the callers of copy_file_range() to utilize the > > do_splice_direct() between file system types even when > > copy_file_range() doesn't support cross fs copy. Isn't that > > beneficial? > > Others might argue no, but I think it's good to have that option open. > > If the consensus is that we do need a check at the vfs layer to validate > that the struct files come from the same fstype (as Amir suggests), then > I can live with that. We've dealt with internal API changes before so we > can do it again. > > Al is quite correct though that we really do want to compare fstypes > rather than op pointers in that case however. It feels like folks are now ok with either the check being in the drivers or doing the check in the VFS layer. I'm picking the choice of not doing the check in the VFS layer because it allows for do_splice_direct() by any caller. I'm about to submit the new version of the patches (this time I will include the NFS patch series). We can continue with the discussion on the new version. I have added checks for the CIFS and OverlayFS to be consistent with the previous behavior -- no cross-device copy_offload, I assume if and when those file systems are ready to make use of it they'll remove the check. > > > > But I don't feel strongly about the check (or rather the location of > > > > it VFS vs each FS) and what I ultimately need is to removed same sb > > > > check. It sounds like if Amir isn't objecting, then the check for same > > > > file system type should be removed from VFS. And, for each of the file > > > > systems that currently support copy_file_range() -- CIFS, NFS, and > > > > overlayfs -- I need to modify them and add a check for the same > > > > fs_type. > > > > > > > > Amir to answer your question, only NFSv4.2 has copy_offload > > > > functionality (not earlier NFS versions). Furthermore, existing > > > > upstream only supports same sb copy offload. What this patch series > > > > adds is support for copy offload across different superblocks but NFS > > > > will not support (and would need a check) for copy offload across > > > > different file system types. Also I kinda stand behind the ideas > > > > stated: 1) this is of interest to NFS (where NFS here is to represent > > > > a community, and CIFS is used to represent the other community). 2) > > > > NFSv4.2 copy offload a specific feature that needs this functionality. > > > > 3rd statement is bad. Only NFSv4.2 will allow copies between different > > > > NFS servers (ie., after this patch +series), the emphasis was on "will > > > > allow" meaning what this patch will allow to be done (ie, patch's > > > > purpose). Or also, if the NFS server exports different exports, then a > > > > copy between them (assuming exports of the same file system type). > > > > > > > > In the next version of the patch, I'll do my best to specified what > > > > changed as the consequence of removing the cross sb check (ie, file > > > > system types of the passed in file can be from different file > > > > systems). I will add wording to the man page and add the suggested > > > > wording to the "porting" file. > > -- > Jeff Layton <jlayton@redhat.com> > ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-24 19:59 ` Olga Kornievskaia @ 2018-10-25 4:58 ` Amir Goldstein 2018-10-25 15:58 ` Olga Kornievskaia 0 siblings, 1 reply; 39+ messages in thread From: Amir Goldstein @ 2018-10-25 4:58 UTC (permalink / raw) To: Olga Kornievskaia Cc: Jeff Layton, Matthew Wilcox, Al Viro, linux-fsdevel, Linux NFS Mailing List, fweimer, Steve French, Darrick J. Wong, Christoph Hellwig, linux-api On Wed, Oct 24, 2018 at 10:59 PM Olga Kornievskaia <olga.kornievskaia@gmail.com> wrote: > [...] > It feels like folks are now ok with either the check being in the > drivers or doing the check in the VFS layer. > > I'm picking the choice of not doing the check in the VFS layer because > it allows for do_splice_direct() by any caller. I'm sorry, but this reasoning in flawed and this is not the reason that Matthew promoted not doing same fs type check in vfs. You did not understand the option that I was promoting to begin with. What I suggested was: 1. Remove current same sb check in beginning of vfs_copy_file_range() 2. Check sb && ->clone_file_range 3. Check same sb type && ->copy_file_range 4. Cross fs do_splice_direct() It's fine that you chose not to check for same fs type in VFS before calling copy_file_range() method, but still requires an ACK from Al that he agrees with passing in file * of another filesystem on the interface. > I'm about to submit > the new version of the patches (this time I will include the NFS patch > series). We can continue with the discussion on the new version. > > I have added checks for the CIFS and OverlayFS to be consistent with > the previous behavior -- no cross-device copy_offload, I assume if and > when those file systems are ready to make use of it they'll remove the > check. > Actually overlayfs code is "ready" for cross sb copy, but neither nfs nor cifs are supported as upper file system, so it doesn't matter much. Thanks, Amir. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-25 4:58 ` Amir Goldstein @ 2018-10-25 15:58 ` Olga Kornievskaia 2018-10-25 16:00 ` Olga Kornievskaia 0 siblings, 1 reply; 39+ messages in thread From: Olga Kornievskaia @ 2018-10-25 15:58 UTC (permalink / raw) To: Amir Goldstein Cc: Jeff Layton, willy, viro, linux-fsdevel, linux-nfs, fweimer, Steve French, Darrick J. Wong, Christoph Hellwig, Linux API On Thu, Oct 25, 2018 at 12:59 AM Amir Goldstein <amir73il@gmail.com> wrote: > > On Wed, Oct 24, 2018 at 10:59 PM Olga Kornievskaia > <olga.kornievskaia@gmail.com> wrote: > > > [...] > > It feels like folks are now ok with either the check being in the > > drivers or doing the check in the VFS layer. > > > > I'm picking the choice of not doing the check in the VFS layer because > > it allows for do_splice_direct() by any caller. > > I'm sorry, but this reasoning in flawed and this is not the reason that > Matthew promoted not doing same fs type check in vfs. I stated the reason why I picked to do the check at the driver layer. Looking at your version of the sb type check to be only applied to the copy_file_range indeed makes my argument invalid. I was under the impression that sb type check was being proposed as a standalone check (just like the sb check was standalone). Thus, yes I didn't completely understand what you proposed. > You did not understand the option that I was promoting to begin with. > What I suggested was: > > 1. Remove current same sb check in beginning of vfs_copy_file_range() > 2. Check sb && ->clone_file_range > 3. Check same sb type && ->copy_file_range > 4. Cross fs do_splice_direct() > > It's fine that you chose not to check for same fs type in VFS before calling > copy_file_range() method, but still requires an ACK from Al that he agrees > with passing in file * of another filesystem on the interface. Al, can you please provide a final decision as to which way you would prefer for this to be done. > > I'm about to submit > > the new version of the patches (this time I will include the NFS patch > > series). We can continue with the discussion on the new version. > > > > I have added checks for the CIFS and OverlayFS to be consistent with > > the previous behavior -- no cross-device copy_offload, I assume if and > > when those file systems are ready to make use of it they'll remove the > > check. > > > > Actually overlayfs code is "ready" for cross sb copy, but neither nfs nor > cifs are supported as upper file system, so it doesn't matter much. So then the commit statement is still true. When overlayfs will have upper file systems that do support it, this check can be removed. > > Thanks, > Amir. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-25 15:58 ` Olga Kornievskaia @ 2018-10-25 16:00 ` Olga Kornievskaia 2018-10-25 16:57 ` Amir Goldstein 0 siblings, 1 reply; 39+ messages in thread From: Olga Kornievskaia @ 2018-10-25 16:00 UTC (permalink / raw) To: Amir Goldstein Cc: Jeff Layton, willy, viro, linux-fsdevel, linux-nfs, fweimer, Steve French, Darrick J. Wong, Christoph Hellwig, Linux API On Thu, Oct 25, 2018 at 11:58 AM Olga Kornievskaia <olga.kornievskaia@gmail.com> wrote: > > On Thu, Oct 25, 2018 at 12:59 AM Amir Goldstein <amir73il@gmail.com> wrote: > > > > On Wed, Oct 24, 2018 at 10:59 PM Olga Kornievskaia > > <olga.kornievskaia@gmail.com> wrote: > > > > > [...] > > > It feels like folks are now ok with either the check being in the > > > drivers or doing the check in the VFS layer. > > > > > > I'm picking the choice of not doing the check in the VFS layer because > > > it allows for do_splice_direct() by any caller. > > > > I'm sorry, but this reasoning in flawed and this is not the reason that > > Matthew promoted not doing same fs type check in vfs. > > I stated the reason why I picked to do the check at the driver layer. > Looking at your version of the sb type check to be only applied to the > copy_file_range indeed makes my argument invalid. I was under the > impression that sb type check was being proposed as a standalone check > (just like the sb check was standalone). Thus, yes I didn't completely > understand what you proposed. > > > You did not understand the option that I was promoting to begin with. > > What I suggested was: > > > > 1. Remove current same sb check in beginning of vfs_copy_file_range() > > 2. Check sb && ->clone_file_range > > 3. Check same sb type && ->copy_file_range > > 4. Cross fs do_splice_direct() > > > > It's fine that you chose not to check for same fs type in VFS before calling > > copy_file_range() method, but still requires an ACK from Al that he agrees > > with passing in file * of another filesystem on the interface. > > Al, can you please provide a final decision as to which way you would > prefer for this to be done. > > > > I'm about to submit > > > the new version of the patches (this time I will include the NFS patch > > > series). We can continue with the discussion on the new version. > > > > > > I have added checks for the CIFS and OverlayFS to be consistent with > > > the previous behavior -- no cross-device copy_offload, I assume if and > > > when those file systems are ready to make use of it they'll remove the > > > check. > > > > > > > Actually overlayfs code is "ready" for cross sb copy, but neither nfs nor > > cifs are supported as upper file system, so it doesn't matter much. > > So then the commit statement is still true. When overlayfs will have > upper file systems that do support it, this check can be removed. Ops sorry I meant them as questions. Do you feel that commit message needs to be changed then? > > > > > Thanks, > > Amir. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-25 16:00 ` Olga Kornievskaia @ 2018-10-25 16:57 ` Amir Goldstein 0 siblings, 0 replies; 39+ messages in thread From: Amir Goldstein @ 2018-10-25 16:57 UTC (permalink / raw) To: Olga Kornievskaia Cc: Jeff Layton, Matthew Wilcox, Al Viro, linux-fsdevel, Linux NFS Mailing List, fweimer, Steve French, Darrick J. Wong, Christoph Hellwig, linux-api On Thu, Oct 25, 2018 at 7:00 PM Olga Kornievskaia <olga.kornievskaia@gmail.com> wrote: > > On Thu, Oct 25, 2018 at 11:58 AM Olga Kornievskaia > <olga.kornievskaia@gmail.com> wrote: > > > > On Thu, Oct 25, 2018 at 12:59 AM Amir Goldstein <amir73il@gmail.com> wrote: > > > > > > On Wed, Oct 24, 2018 at 10:59 PM Olga Kornievskaia > > > <olga.kornievskaia@gmail.com> wrote: > > > > > > > [...] > > > > It feels like folks are now ok with either the check being in the > > > > drivers or doing the check in the VFS layer. > > > > > > > > I'm picking the choice of not doing the check in the VFS layer because > > > > it allows for do_splice_direct() by any caller. > > > > > > I'm sorry, but this reasoning in flawed and this is not the reason that > > > Matthew promoted not doing same fs type check in vfs. > > > > I stated the reason why I picked to do the check at the driver layer. > > Looking at your version of the sb type check to be only applied to the > > copy_file_range indeed makes my argument invalid. I was under the > > impression that sb type check was being proposed as a standalone check > > (just like the sb check was standalone). Thus, yes I didn't completely > > understand what you proposed. > > > > > You did not understand the option that I was promoting to begin with. > > > What I suggested was: > > > > > > 1. Remove current same sb check in beginning of vfs_copy_file_range() > > > 2. Check sb && ->clone_file_range > > > 3. Check same sb type && ->copy_file_range > > > 4. Cross fs do_splice_direct() > > > > > > It's fine that you chose not to check for same fs type in VFS before calling > > > copy_file_range() method, but still requires an ACK from Al that he agrees > > > with passing in file * of another filesystem on the interface. > > > > Al, can you please provide a final decision as to which way you would > > prefer for this to be done. > > > > > > I'm about to submit > > > > the new version of the patches (this time I will include the NFS patch > > > > series). We can continue with the discussion on the new version. > > > > > > > > I have added checks for the CIFS and OverlayFS to be consistent with > > > > the previous behavior -- no cross-device copy_offload, I assume if and > > > > when those file systems are ready to make use of it they'll remove the > > > > check. > > > > > > > > > > Actually overlayfs code is "ready" for cross sb copy, but neither nfs nor > > > cifs are supported as upper file system, so it doesn't matter much. > > > > So then the commit statement is still true. When overlayfs will have > > upper file systems that do support it, this check can be removed. > > Ops sorry I meant them as questions. Do you feel that commit message > needs to be changed then? Commit message is fine by me. Thanks, Amir. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-23 15:03 ` Olga Kornievskaia 2018-10-23 15:30 ` Olga Kornievskaia @ 2018-10-23 15:39 ` Matthew Wilcox 2018-10-24 11:32 ` Amir Goldstein 1 sibling, 1 reply; 39+ messages in thread From: Matthew Wilcox @ 2018-10-23 15:39 UTC (permalink / raw) To: Olga Kornievskaia Cc: Amir Goldstein, Jeff Layton, viro, linux-fsdevel, linux-nfs, fweimer, Steve French, Darrick J. Wong, Christoph Hellwig, Linux API On Tue, Oct 23, 2018 at 11:03:02AM -0400, Olga Kornievskaia wrote: > On Tue, Oct 23, 2018 at 2:05 AM Amir Goldstein <amir73il@gmail.com> wrote: > > On Tue, Oct 23, 2018 at 2:39 AM Jeff Layton <jlayton@redhat.com> wrote: > > > Sorry if I wasn't clear before: > > > > > > Basically, I think Willy and I are both envisioning that some > > > copy_file_range implementations may eventually not be subject to the > > > limitations of the checks you're adding. > > > > > > > Yes. Eventually. And even Matthew is (quote) "dubious" about that ever > > happening. Changing the interface as Matthew proposed has a price > > and we need to compare this price to the alleged backporting price > > that nobody may ever need to pay. > > > > As far as I can tell, passing a struct file * on a file_operations method > > that does not belong to that filesystem in unprecedented (*) and is a far > > more lethal landmine than the alleged backporting landmine. > > > > (*) prior to v4.19-rc1, filesystems could get an overlayfs file, but > > file_inode(file) has always belonged to the filesystem. > > > > Olga, > > > > I do not strongly object to Matthew's proposal, so don't feel > > obligated to choose my side of the argument. I am just trying > > to offer a different perspective. > > > > In any case, my outstanding concerns with the patch are: > > > > 1. If you change syscall to support cross fs type copy (which is > > good IMO) need to document that in commit message > > and possibly follow up later with a note in man page > > > > 2. Commit message says: > > "This feature was of interest of ... NFS" > > "This feature is needed by NFSv4.2..." > > "NFS will allow for copies between different NFS servers." > > It is not clear to me if we are talking about present of future > > NFSv4.2 code. If NFSv4.2 currently does not support cross > > sb copy (??) than your patch need to enforce same sb > > in nfs4_copy_file_range(). If it does support cross sb copy > > than please edit the commit message to make that clear. > > I personally agree with Amir. I think it's far fetched that a file > system would know how to handle something that's not of its type. When > the copy_file_range() was checked in, there was a comment above the > superblock check saying "we might want to relax this in the future". > It deemed appropriate then to enforce the check since none of the file > systems used it. Now, the future is here, and we are removing the > check but proposing a different once because again the future isn't > here and having a single check simplifies the code. I've done some more research and found that while NFSv4.2 has its own representation of a copyable range of a file, iSCSI and SMB3 share the same ROD. So it's not at all implausible that some other filesystem might also decide to use the same ROD, perhaps even NFS v4.3. It's clearly crazy to expect filesystem A to know about all the interpretations of filesystem B's file->private. I'd expect us to add a function like: int vfs_get_rod(struct file *src, struct file_rod *rod); and then a filesystem's copy_file_range() would check to see if both src and dest referred to the same server, and if not call vfs_get_rod() to be able to send the ROD to the destination. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-23 15:39 ` Matthew Wilcox @ 2018-10-24 11:32 ` Amir Goldstein 0 siblings, 0 replies; 39+ messages in thread From: Amir Goldstein @ 2018-10-24 11:32 UTC (permalink / raw) To: Matthew Wilcox Cc: Olga Kornievskaia, Jeff Layton, Al Viro, linux-fsdevel, Linux NFS Mailing List, fweimer, Steve French, Darrick J. Wong, Christoph Hellwig, linux-api On Tue, Oct 23, 2018 at 6:39 PM Matthew Wilcox <willy@infradead.org> wrote: > [...] > I've done some more research and found that while NFSv4.2 has its own > representation of a copyable range of a file, iSCSI and SMB3 share the > same ROD. So it's not at all implausible that some other filesystem > might also decide to use the same ROD, perhaps even NFS v4.3. > > It's clearly crazy to expect filesystem A to know about all the > interpretations of filesystem B's file->private. I'd expect us to add > a function like: > > int vfs_get_rod(struct file *src, struct file_rod *rod); > > and then a filesystem's copy_file_range() would check to see if both > src and dest referred to the same server, and if not call vfs_get_rod() > to be able to send the ROD to the destination. > I am not saying cross filesystem type copy won't be possible. I'm just saying we are going to get the API wrong anyway. Surely, it is going to be either cifs or nfs42 that supports cross fs copy first, not both at the same development cycle, so now it seems flaky to use the out_file's copy_file_range() method. You'd want to figure out which of the in/out fs supports cross fs copy and call that method (not enough information). I suspect we will need a new cross_copy_file_range() method. And to answer Olga's question, yes, I do find cross fs copy with do_splice_direct() beneficial. Thanks, Amir. ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v1 00/11] client-side support for "inter" SSC copy @ 2018-10-19 15:29 Olga Kornievskaia 2018-10-19 15:29 ` [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range Olga Kornievskaia 0 siblings, 1 reply; 39+ messages in thread From: Olga Kornievskaia @ 2018-10-19 15:29 UTC (permalink / raw) To: trondmy, anna.schumaker; +Cc: linux-nfs From: Olga Kornievskaia <kolga@netapp.com> This patch series adds client-side support for doing NFSv4.2 "inter" copy offload between different NFS servers. In case of the "inter" SSC copy files reside on different servers and thus under different superblocks and require that VFS removes the restriction that src and dst files must be on the same superblock. NFS's copy_file_range() determines if the copy is "intra" or "inter" and for "inter" it sends the COPY_NOTIFY to the source server. Then, it would send of an asynchronous COPY to the destination server. If an application cancels an in-flight COPY, OFFLOAD_CANCEL is sent to both of the servers. This patch series also include necessary client-side additions that are performed by the destination server. The server needs an NFS open that represents a source file without doing an actual open. Two function nfs42_ssc_open/nfs42_ssc_close() are introduced to accomplish it that make use of the VFS's alloc_file_pseudo() to represent an open. Also this particular open is marked (NFS_SVC_SSC_COPY_STATE) so that if the destination server ever to receive stateid errors on this stateid, it knows not to initiate state recovery (in case when source server reboots). The recovery must be done by the client and a new copy must be initiated. Therefore, in this case the recovery needs to fail with EIO. Anna Schumaker (1): fs: Don't copy beyond the end of the file Olga Kornievskaia (10): VFS permit cross device vfs_copy_file_range NFS test for intra vs inter COPY NFS NFSD defining nl4_servers structure needed by both NFS add COPY_NOTIFY operation NFS add ca_source_server<> to COPY NFS also send OFFLOAD_CANCEL to source server NFS inter ssc open NFS skip recovery of copy open on dest server NFS for "inter" copy treat ESTALE as ENOTSUPP NFS COPY handle ERR_OFFLOAD_DENIED Documentation/filesystems/vfs.txt | 4 +- fs/nfs/nfs42.h | 15 ++- fs/nfs/nfs42proc.c | 129 ++++++++++++++++++++++--- fs/nfs/nfs42xdr.c | 193 +++++++++++++++++++++++++++++++++++++- fs/nfs/nfs4_fs.h | 8 ++ fs/nfs/nfs4file.c | 123 +++++++++++++++++++++++- fs/nfs/nfs4proc.c | 6 +- fs/nfs/nfs4state.c | 14 +++ fs/nfs/nfs4xdr.c | 1 + fs/read_write.c | 16 ++-- include/linux/nfs4.h | 25 +++++ include/linux/nfs_fs_sb.h | 1 + include/linux/nfs_xdr.h | 17 ++++ 13 files changed, 526 insertions(+), 26 deletions(-) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-19 15:29 [PATCH v1 00/11] client-side support for "inter" SSC copy Olga Kornievskaia @ 2018-10-19 15:29 ` Olga Kornievskaia 2018-10-19 16:14 ` Trond Myklebust 0 siblings, 1 reply; 39+ messages in thread From: Olga Kornievskaia @ 2018-10-19 15:29 UTC (permalink / raw) To: trondmy, anna.schumaker; +Cc: linux-nfs From: Olga Kornievskaia <kolga@netapp.com> Allow copy_file_range to copy between different superblocks but only of the same file system types. This feature was of interest to CIFS as well as NFS. 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 <kolga@netapp.com> --- Documentation/filesystems/vfs.txt | 4 +++- fs/read_write.c | 13 ++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt index a6c6a8a..5e520de 100644 --- a/Documentation/filesystems/vfs.txt +++ b/Documentation/filesystems/vfs.txt @@ -958,7 +958,9 @@ otherwise noted. fallocate: called by the VFS to preallocate blocks or punch a hole. - copy_file_range: called by the copy_file_range(2) system call. + 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. clone_file_range: called by the ioctl(2) system call for FICLONERANGE and FICLONE commands. diff --git a/fs/read_write.c b/fs/read_write.c index c60790f..474e740 100644 --- a/fs/read_write.c +++ b/fs/read_write.c @@ -1578,10 +1578,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; @@ -1591,7 +1587,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) { @@ -1600,10 +1597,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 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-19 15:29 ` [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range Olga Kornievskaia @ 2018-10-19 16:14 ` Trond Myklebust 0 siblings, 0 replies; 39+ messages in thread From: Trond Myklebust @ 2018-10-19 16:14 UTC (permalink / raw) To: anna.schumaker, olga.kornievskaia; +Cc: viro, linux-nfs, linux-fsdevel On Fri, 2018-10-19 at 11:29 -0400, Olga Kornievskaia wrote: > From: Olga Kornievskaia <kolga@netapp.com> > > Allow copy_file_range to copy between different superblocks but only > of the same file system types. This feature was of interest to CIFS > as well as NFS. > > 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 <kolga@netapp.com> > --- > Documentation/filesystems/vfs.txt | 4 +++- > fs/read_write.c | 13 ++++++------- > 2 files changed, 9 insertions(+), 8 deletions(-) > > diff --git a/Documentation/filesystems/vfs.txt > b/Documentation/filesystems/vfs.txt > index a6c6a8a..5e520de 100644 > --- a/Documentation/filesystems/vfs.txt > +++ b/Documentation/filesystems/vfs.txt > @@ -958,7 +958,9 @@ otherwise noted. > > fallocate: called by the VFS to preallocate blocks or punch a > hole. > > - copy_file_range: called by the copy_file_range(2) system call. > + 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. > > clone_file_range: called by the ioctl(2) system call for > FICLONERANGE and > FICLONE commands. > diff --git a/fs/read_write.c b/fs/read_write.c > index c60790f..474e740 100644 > --- a/fs/read_write.c > +++ b/fs/read_write.c > @@ -1578,10 +1578,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; > > @@ -1591,7 +1587,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) { > @@ -1600,10 +1597,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; > } > Ditto. This also needs an ACK from the VFS maintainers. Cc: Al and linux-fsdevel -- Trond Myklebust Linux NFS client maintainer, Hammerspace trond.myklebust@hammerspace.com ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range @ 2018-10-19 16:14 ` Trond Myklebust 0 siblings, 0 replies; 39+ messages in thread From: Trond Myklebust @ 2018-10-19 16:14 UTC (permalink / raw) To: anna.schumaker, olga.kornievskaia; +Cc: viro, linux-nfs, linux-fsdevel T24gRnJpLCAyMDE4LTEwLTE5IGF0IDExOjI5IC0wNDAwLCBPbGdhIEtvcm5pZXZza2FpYSB3cm90 ZToNCj4gRnJvbTogT2xnYSBLb3JuaWV2c2thaWEgPGtvbGdhQG5ldGFwcC5jb20+DQo+IA0KPiBB bGxvdyBjb3B5X2ZpbGVfcmFuZ2UgdG8gY29weSBiZXR3ZWVuIGRpZmZlcmVudCBzdXBlcmJsb2Nr cyBidXQgb25seQ0KPiBvZiB0aGUgc2FtZSBmaWxlIHN5c3RlbSB0eXBlcy4gVGhpcyBmZWF0dXJl IHdhcyBvZiBpbnRlcmVzdCB0byBDSUZTDQo+IGFzIHdlbGwgYXMgTkZTLg0KPiANCj4gVGhpcyBm ZWF0dXJlIGlzIG5lZWRlZCBieSBORlN2NC4yIHRvIHBlcmZvcm0gZmlsZSBjb3B5IG9wZXJhdGlv biBvbg0KPiB0aGUgc2FtZSBzZXJ2ZXIgb3IgZmlsZSBjb3B5IGJldHdlZW4gZGlmZmVyZW50IE5G U3Y0LjIgc2VydmVycy4NCj4gDQo+IElmIGEgZmlsZSBzeXN0ZW0ncyBmaWxlb3BlcmF0aW9ucyBj b3B5X2ZpbGVfcmFuZ2Ugb3BlcmF0aW9uIHByb2hpYml0cw0KPiBjcm9zcy1kZXZpY2UgY29waWVz LCBmYWxsIGJhY2sgdG8gZG9fc3BsaWNlX2RpcmVjdC4gVGhpcyB3b3VsZCBiZQ0KPiBuZWVkZWQg Zm9yIHRoZSBORlMgKGRlc3RpbmF0aW9uKSBzZXJ2ZXIgc2lkZSBpbXBsZW1lbnRhdGlvbiBvZiB0 aGUNCj4gZmlsZSBjb3B5IGFuZCBjdXJyZW50bHkgZm9yIENJRlMuDQo+IA0KPiBCZXNpZGVzIE5G UywgdGhlcmUgaXMgb25seSAxIGltcGxlbWVudG9yIG9mIHRoZSBjb3B5X2ZpbGVfcmFuZ2UgRlMN Cj4gb3BlcmF0aW9uIC0tIENJRlMuIENJRlMgYXNzdW1lcyBpbmNvbWluZyBmaWxlIGRlc2NyaXB0 b3JzIGFyZSBib3RoDQo+IENJRlMgYnV0IGl0IHdpbGwgY2hlY2sgaWYgdGhleSBhcmUgY29taW5n IGZyb20gZGlmZmVyZW50IHNlcnZlcnMgYW5kDQo+IHJldHVybiBlcnJvciBjb2RlIHRvIGZhbGwg YmFjayB0byBkb19zcGxpY2VfZGlyZWN0Lg0KPiANCj4gTkZTIHdpbGwgYWxsb3cgZm9yIGNvcGll cyBiZXR3ZWVuIGRpZmZlcmVudCBORlMgc2VydmVycy4NCj4gDQo+IEFkZGluZyB0byB0aGUgdmZz LnR4dCBkb2N1bWVudGF0aW9uIHRvIGV4cGxpY2l0bHkgd2FybiBhYm91dCBhbGxvd2luZw0KPiBm b3IgZGlmZmVyZW50IHN1cGVyYmxvY2tzIG9mIHRoZSBzYW1lIGZpbGUgdHlwZSB0byBiZSBwYXNz ZWQgaW50byB0aGUNCj4gY29weV9maWxlX3JhbmdlIGZvciB0aGUgZnV0dXJlIHVzZXJzIG9mIGNv cHlfZmlsZV9yYW5nZSBtZXRob2QuDQo+IA0KPiBTaWduZWQtb2ZmLWJ5OiBPbGdhIEtvcm5pZXZz a2FpYSA8a29sZ2FAbmV0YXBwLmNvbT4NCj4gLS0tDQo+ICBEb2N1bWVudGF0aW9uL2ZpbGVzeXN0 ZW1zL3Zmcy50eHQgfCAgNCArKystDQo+ICBmcy9yZWFkX3dyaXRlLmMgICAgICAgICAgICAgICAg ICAgfCAxMyArKysrKystLS0tLS0tDQo+ICAyIGZpbGVzIGNoYW5nZWQsIDkgaW5zZXJ0aW9ucygr KSwgOCBkZWxldGlvbnMoLSkNCj4gDQo+IGRpZmYgLS1naXQgYS9Eb2N1bWVudGF0aW9uL2ZpbGVz eXN0ZW1zL3Zmcy50eHQNCj4gYi9Eb2N1bWVudGF0aW9uL2ZpbGVzeXN0ZW1zL3Zmcy50eHQNCj4g aW5kZXggYTZjNmE4YS4uNWU1MjBkZSAxMDA2NDQNCj4gLS0tIGEvRG9jdW1lbnRhdGlvbi9maWxl c3lzdGVtcy92ZnMudHh0DQo+ICsrKyBiL0RvY3VtZW50YXRpb24vZmlsZXN5c3RlbXMvdmZzLnR4 dA0KPiBAQCAtOTU4LDcgKzk1OCw5IEBAIG90aGVyd2lzZSBub3RlZC4NCj4gIA0KPiAgICBmYWxs b2NhdGU6IGNhbGxlZCBieSB0aGUgVkZTIHRvIHByZWFsbG9jYXRlIGJsb2NrcyBvciBwdW5jaCBh DQo+IGhvbGUuDQo+ICANCj4gLSAgY29weV9maWxlX3JhbmdlOiBjYWxsZWQgYnkgdGhlIGNvcHlf ZmlsZV9yYW5nZSgyKSBzeXN0ZW0gY2FsbC4NCj4gKyAgY29weV9maWxlX3JhbmdlOiBjYWxsZWQg YnkgY29weV9maWxlX3JhbmdlKDIpIHN5c3RlbSBjYWxsLiBUaGlzDQo+IG1ldGhvZA0KPiArCQkg ICB3b3JrcyBvbiB0d28gZmlsZSBkZXNjcmlwdG9ycyB0aGF0IG1pZ2h0IHJlc2lkZSBvbg0KPiAr CQkgICBkaWZmZXJlbnQgc3VwZXJibG9ja3Mgb2YgdGhlIHNhbWUgdHlwZSBvZiBmaWxlDQo+IHN5 c3RlbS4NCj4gIA0KPiAgICBjbG9uZV9maWxlX3JhbmdlOiBjYWxsZWQgYnkgdGhlIGlvY3RsKDIp IHN5c3RlbSBjYWxsIGZvcg0KPiBGSUNMT05FUkFOR0UgYW5kDQo+ICAJRklDTE9ORSBjb21tYW5k cy4NCj4gZGlmZiAtLWdpdCBhL2ZzL3JlYWRfd3JpdGUuYyBiL2ZzL3JlYWRfd3JpdGUuYw0KPiBp bmRleCBjNjA3OTBmLi40NzRlNzQwIDEwMDY0NA0KPiAtLS0gYS9mcy9yZWFkX3dyaXRlLmMNCj4g KysrIGIvZnMvcmVhZF93cml0ZS5jDQo+IEBAIC0xNTc4LDEwICsxNTc4LDYgQEAgc3NpemVfdCB2 ZnNfY29weV9maWxlX3JhbmdlKHN0cnVjdCBmaWxlDQo+ICpmaWxlX2luLCBsb2ZmX3QgcG9zX2lu LA0KPiAgCSAgICAoZmlsZV9vdXQtPmZfZmxhZ3MgJiBPX0FQUEVORCkpDQo+ICAJCXJldHVybiAt RUJBREY7DQo+ICANCj4gLQkvKiB0aGlzIGNvdWxkIGJlIHJlbGF4ZWQgb25jZSBhIG1ldGhvZCBz dXBwb3J0cyBjcm9zcy1mcyBjb3BpZXMNCj4gKi8NCj4gLQlpZiAoaW5vZGVfaW4tPmlfc2IgIT0g aW5vZGVfb3V0LT5pX3NiKQ0KPiAtCQlyZXR1cm4gLUVYREVWOw0KPiAtDQo+ICAJaWYgKGxlbiA9 PSAwKQ0KPiAgCQlyZXR1cm4gMDsNCj4gIA0KPiBAQCAtMTU5MSw3ICsxNTg3LDggQEAgc3NpemVf dCB2ZnNfY29weV9maWxlX3JhbmdlKHN0cnVjdCBmaWxlDQo+ICpmaWxlX2luLCBsb2ZmX3QgcG9z X2luLA0KPiAgCSAqIFRyeSBjbG9uaW5nIGZpcnN0LCB0aGlzIGlzIHN1cHBvcnRlZCBieSBtb3Jl IGZpbGUgc3lzdGVtcywNCj4gYW5kDQo+ICAJICogbW9yZSBlZmZpY2llbnQgaWYgYm90aCBjbG9u ZSBhbmQgY29weSBhcmUgc3VwcG9ydGVkIChlLmcuDQo+IE5GUykuDQo+ICAJICovDQo+IC0JaWYg KGZpbGVfaW4tPmZfb3AtPmNsb25lX2ZpbGVfcmFuZ2UpIHsNCj4gKwlpZiAoaW5vZGVfaW4tPmlf c2IgPT0gaW5vZGVfb3V0LT5pX3NiICYmDQo+ICsJCQlmaWxlX2luLT5mX29wLT5jbG9uZV9maWxl X3JhbmdlKSB7DQo+ICAJCXJldCA9IGZpbGVfaW4tPmZfb3AtPmNsb25lX2ZpbGVfcmFuZ2UoZmls ZV9pbiwgcG9zX2luLA0KPiAgCQkJCWZpbGVfb3V0LCBwb3Nfb3V0LCBsZW4pOw0KPiAgCQlpZiAo cmV0ID09IDApIHsNCj4gQEAgLTE2MDAsMTAgKzE1OTcsMTIgQEAgc3NpemVfdCB2ZnNfY29weV9m aWxlX3JhbmdlKHN0cnVjdCBmaWxlDQo+ICpmaWxlX2luLCBsb2ZmX3QgcG9zX2luLA0KPiAgCQl9 DQo+ICAJfQ0KPiAgDQo+IC0JaWYgKGZpbGVfb3V0LT5mX29wLT5jb3B5X2ZpbGVfcmFuZ2UpIHsN Cj4gKwlpZiAoZmlsZV9vdXQtPmZfb3AtPmNvcHlfZmlsZV9yYW5nZSAmJg0KPiArCQkJKGZpbGVf aW4tPmZfb3AtPmNvcHlfZmlsZV9yYW5nZSA9PQ0KPiArCQkJCWZpbGVfb3V0LT5mX29wLT5jb3B5 X2ZpbGVfcmFuZ2UpKSB7DQo+ICAJCXJldCA9IGZpbGVfb3V0LT5mX29wLT5jb3B5X2ZpbGVfcmFu Z2UoZmlsZV9pbiwgcG9zX2luLA0KPiBmaWxlX291dCwNCj4gIAkJCQkJCSAgICAgIHBvc19vdXQs IGxlbiwNCj4gZmxhZ3MpOw0KPiAtCQlpZiAocmV0ICE9IC1FT1BOT1RTVVBQKQ0KPiArCQlpZiAo cmV0ICE9IC1FT1BOT1RTVVBQICYmIHJldCAhPSAtRVhERVYpDQo+ICAJCQlnb3RvIGRvbmU7DQo+ ICAJfQ0KPiAgDQoNCkRpdHRvLiAgVGhpcyBhbHNvIG5lZWRzIGFuIEFDSyBmcm9tIHRoZSBWRlMg bWFpbnRhaW5lcnMuDQoNCkNjOiBBbCBhbmQgbGludXgtZnNkZXZlbA0KLS0gDQpUcm9uZCBNeWts ZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFpbnRhaW5lciwgSGFtbWVyc3BhY2UNCnRyb25kLm15 a2xlYnVzdEBoYW1tZXJzcGFjZS5jb20NCg0KDQo= ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range 2018-10-19 16:14 ` Trond Myklebust (?) @ 2018-10-19 16:26 ` Olga Kornievskaia -1 siblings, 0 replies; 39+ messages in thread From: Olga Kornievskaia @ 2018-10-19 16:26 UTC (permalink / raw) To: Trond Myklebust; +Cc: Anna Schumaker, viro, linux-nfs, linux-fsdevel On Fri, Oct 19, 2018 at 12:14 PM Trond Myklebust <trondmy@hammerspace.com> wrote: > > On Fri, 2018-10-19 at 11:29 -0400, Olga Kornievskaia wrote: > > From: Olga Kornievskaia <kolga@netapp.com> > > > > Allow copy_file_range to copy between different superblocks but only > > of the same file system types. This feature was of interest to CIFS > > as well as NFS. > > > > 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 <kolga@netapp.com> > > --- > > Documentation/filesystems/vfs.txt | 4 +++- > > fs/read_write.c | 13 ++++++------- > > 2 files changed, 9 insertions(+), 8 deletions(-) > > > > diff --git a/Documentation/filesystems/vfs.txt > > b/Documentation/filesystems/vfs.txt > > index a6c6a8a..5e520de 100644 > > --- a/Documentation/filesystems/vfs.txt > > +++ b/Documentation/filesystems/vfs.txt > > @@ -958,7 +958,9 @@ otherwise noted. > > > > fallocate: called by the VFS to preallocate blocks or punch a > > hole. > > > > - copy_file_range: called by the copy_file_range(2) system call. > > + 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. > > > > clone_file_range: called by the ioctl(2) system call for > > FICLONERANGE and > > FICLONE commands. > > diff --git a/fs/read_write.c b/fs/read_write.c > > index c60790f..474e740 100644 > > --- a/fs/read_write.c > > +++ b/fs/read_write.c > > @@ -1578,10 +1578,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; > > > > @@ -1591,7 +1587,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) { > > @@ -1600,10 +1597,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; > > } > > > > Ditto. This also needs an ACK from the VFS maintainers. > > Cc: Al and linux-fsdevel Yeah I sent VFS as separate patches to the linux-fsdevel and included other folks (glibc/CIFS?) that were interested in this functionality. Apologizes for double sent. It was easier to send this as a patch series to just linux-nfs first. > -- > Trond Myklebust > Linux NFS client maintainer, Hammerspace > trond.myklebust@hammerspace.com > > ^ permalink raw reply [flat|nested] 39+ messages in thread
end of thread, other threads:[~2018-10-26 1:31 UTC | newest] Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-19 15:30 [PATCH v1 01/11] fs: Don't copy beyond the end of the file Olga Kornievskaia 2018-10-19 15:30 ` [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range Olga Kornievskaia 2018-10-19 15:54 ` Amir Goldstein 2018-10-19 16:14 ` Amir Goldstein 2018-10-19 17:44 ` Matthew Wilcox 2018-10-19 17:58 ` Amir Goldstein 2018-10-19 16:24 ` Olga Kornievskaia 2018-10-19 17:04 ` Olga Kornievskaia 2018-10-20 1:37 ` Steve French 2018-10-19 17:58 ` Matthew Wilcox 2018-10-19 18:47 ` Olga Kornievskaia 2018-10-19 19:06 ` Matthew Wilcox 2018-10-21 13:01 ` Jeff Layton 2018-10-22 18:39 ` Olga Kornievskaia 2018-10-21 14:10 ` Jeff Layton 2018-10-20 4:05 ` Al Viro 2018-10-20 8:54 ` Amir Goldstein 2018-10-22 18:45 ` Olga Kornievskaia 2018-10-22 19:06 ` Matthew Wilcox 2018-10-22 19:34 ` Olga Kornievskaia 2018-10-22 19:48 ` Amir Goldstein 2018-10-22 20:29 ` Matthew Wilcox 2018-10-22 23:39 ` Jeff Layton 2018-10-23 6:05 ` Amir Goldstein 2018-10-23 15:03 ` Olga Kornievskaia 2018-10-23 15:30 ` Olga Kornievskaia 2018-10-23 17:16 ` Olga Kornievskaia 2018-10-24 11:17 ` Jeff Layton 2018-10-24 19:59 ` Olga Kornievskaia 2018-10-25 4:58 ` Amir Goldstein 2018-10-25 15:58 ` Olga Kornievskaia 2018-10-25 16:00 ` Olga Kornievskaia 2018-10-25 16:57 ` Amir Goldstein 2018-10-23 15:39 ` Matthew Wilcox 2018-10-24 11:32 ` Amir Goldstein -- strict thread matches above, loose matches on Subject: below -- 2018-10-19 15:29 [PATCH v1 00/11] client-side support for "inter" SSC copy Olga Kornievskaia 2018-10-19 15:29 ` [PATCH v1 02/11] VFS permit cross device vfs_copy_file_range Olga Kornievskaia 2018-10-19 16:14 ` Trond Myklebust 2018-10-19 16:14 ` Trond Myklebust 2018-10-19 16:26 ` Olga Kornievskaia
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.