diff mbox

[Version-2,03/12] VFS permit cross device vfs_copy_file_range

Message ID 1471627512-4102-4-git-send-email-andros@netapp.com (mailing list archive)
State RFC, archived
Headers show

Commit Message

Andy Adamson Aug. 19, 2016, 5:25 p.m. UTC
From: Andy Adamson <andros@rhel7-2-ga-3.androsad.fake>

NFSv4.2 inter server to server copy always copies across devices.

Note: both btrfs and nfs have EXDEV checks in their
copy_file_range functions.

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/read_write.c | 5 -----
 1 file changed, 5 deletions(-)

Comments

J. Bruce Fields Aug. 19, 2016, 9:08 p.m. UTC | #1
I thought I remembered Christoph (cc'd) arguing at the 2015 lsf/mm that
there were some more issues that would need to be taken care of before
we could turn on cross-device copies.

But maybe I misremember.  In any case, I don't remember what the issues
were....

--b.

On Fri, Aug 19, 2016 at 01:25:03PM -0400, andros@netapp.com wrote:
> From: Andy Adamson <andros@rhel7-2-ga-3.androsad.fake>
> 
> NFSv4.2 inter server to server copy always copies across devices.
> 
> Note: both btrfs and nfs have EXDEV checks in their
> copy_file_range functions.
> 
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
>  fs/read_write.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/fs/read_write.c b/fs/read_write.c
> index 1cbab4e..a6d3350 100644
> --- a/fs/read_write.c
> +++ b/fs/read_write.c
> @@ -1483,7 +1483,6 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
>  			    size_t len, unsigned int flags)
>  {
>  	struct inode *inode_in = file_inode(file_in);
> -	struct inode *inode_out = file_inode(file_out);
>  	ssize_t ret;
>  
>  	if (flags != 0)
> @@ -1505,10 +1504,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;
>  
> -- 
> 1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoph Hellwig Aug. 20, 2016, 6:18 a.m. UTC | #2
On Fri, Aug 19, 2016 at 05:08:44PM -0400, J. Bruce Fields wrote:
> I thought I remembered Christoph (cc'd) arguing at the 2015 lsf/mm that
> there were some more issues that would need to be taken care of before
> we could turn on cross-device copies.

Yes.  Noi other VFS operation is supported between mountpoint, so
copy_file_range shouldn't be either.

Also a general NAK to any new copy_file_range work before we have
proper xfstests coverage.  And it's not like the coverage is hard,
we already have 100s of tests for clone, which in many way is very
similar and could be partially reused.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
J. Bruce Fields Aug. 22, 2016, 7:27 p.m. UTC | #3
On Sat, Aug 20, 2016 at 08:18:33AM +0200, Christoph Hellwig wrote:
> On Fri, Aug 19, 2016 at 05:08:44PM -0400, J. Bruce Fields wrote:
> > I thought I remembered Christoph (cc'd) arguing at the 2015 lsf/mm that
> > there were some more issues that would need to be taken care of before
> > we could turn on cross-device copies.
> 
> Yes.  Noi other VFS operation is supported between mountpoint, so
> copy_file_range shouldn't be either.

Is there anything specific it would break?

> Also a general NAK to any new copy_file_range work before we have
> proper xfstests coverage.  And it's not like the coverage is hard,
> we already have 100s of tests for clone, which in many way is very
> similar and could be partially reused.

Sounds like a reasonable request.

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Adamson, Andy Aug. 24, 2016, 6:38 p.m. UTC | #4
> On Aug 20, 2016, at 2:18 AM, Christoph Hellwig <hch@lst.de> wrote:

> 

> On Fri, Aug 19, 2016 at 05:08:44PM -0400, J. Bruce Fields wrote:

>> I thought I remembered Christoph (cc'd) arguing at the 2015 lsf/mm that

>> there were some more issues that would need to be taken care of before

>> we could turn on cross-device copies.

> 

> Yes.  Noi other VFS operation is supported between mountpoint, so

> copy_file_range shouldn't be either.


Hi Christoph

As you know (because you reviewed the patch), the original intent of vs_copy_file_range as stated in the syscall submission from Zach Brown (commit: 884a12a5972fc867a93f7adf7a8ac2ade5d38fff ) is to allow copies between file systems “once we get implementations which copy between tile systems safely”

NFSv4 inter server to server copy will provide an implementation which can copy between tile systems safely - so I don’t understand your objection to this eventual use of vfs_copy_file_range.

> 

> Also a general NAK to any new copy_file_range work before we have

> proper xfstests coverage.  And it's not like the coverage is hard,

> we already have 100s of tests for clone, which in many way is very

> similar and could be partially reused.


Yes - agreed. Anna is working on updated xfstests for NFSv4 intra server to server copy as well as expanding the xfstests for other vfs_copy_file_range features. We can look at expanding the tests for the inter server to server case.  Jorge Mora author of NFSTests is also working on a test suite for inter server to server copy.
diff mbox

Patch

diff --git a/fs/read_write.c b/fs/read_write.c
index 1cbab4e..a6d3350 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -1483,7 +1483,6 @@  ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
 			    size_t len, unsigned int flags)
 {
 	struct inode *inode_in = file_inode(file_in);
-	struct inode *inode_out = file_inode(file_out);
 	ssize_t ret;
 
 	if (flags != 0)
@@ -1505,10 +1504,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;