[PATCH v2 0/2] fs: allow cross-vfsmount reflink/dedupe

[PATCH v2 0/2] fs: allow cross-vfsmount reflink/dedupe

[Josef Bacik]

Hello,

These patches allow for cross vfsmount reflink/dedupe for everybody and removes
the duplicate checks in btrfs so we can allow it as well.  This limitation seems
to have been copied around from the original implementation from btrfs, but
makes little sense as we just need to make sure both files are on the same super
block.

Btrfs usage across all the distro's I'm aware of regularly use the pattern of
mounting subvolumes for different areas of the file system, and so we need to
allow reflink to happen over vfsmount as a matter of practicality.  It makes no
sense to make cp --reflink=always not work if you are copying something from say
/var to /home if they're differently mounted subvolumes.

The only change since v1 has been adding the Reviewed-by's for the VFS patch,
and then adding an ASSERT() to the btrfs patch to make sure we catch any changes
to this path.

Al if you have no objections we can send this through the btrfs tree.  Thanks,

Josef

Josef Bacik (2):
  btrfs: remove the cross file system checks from remap
  fs: allow cross-vfsmount reflink/dedupe

 fs/btrfs/reflink.c | 4 +---
 fs/ioctl.c         | 4 ----
 fs/remap_range.c   | 7 +------
 3 files changed, 2 insertions(+), 13 deletions(-)

-- 
2.26.3

[PATCH v2 1/2] btrfs: remove the cross file system checks from remap

[Josef Bacik]

The sb check is already done in do_clone_file_range, and the mnt check
(which will hopefully go away in a subsequent patch) is done in
ioctl_file_clone().  Remove the check in our code and put an ASSERT() to
make sure it doesn't change underneath us.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/reflink.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/btrfs/reflink.c b/fs/btrfs/reflink.c
index a3930da4eb3f..6fed103f1000 100644
--- a/fs/btrfs/reflink.c
+++ b/fs/btrfs/reflink.c
@@ -772,9 +772,7 @@ static int btrfs_remap_file_range_prep(struct file *file_in, loff_t pos_in,
 		if (btrfs_root_readonly(root_out))
 			return -EROFS;
 
-		if (file_in->f_path.mnt != file_out->f_path.mnt ||
-		    inode_in->i_sb != inode_out->i_sb)
-			return -EXDEV;
+		ASSERT(inode_in->i_sb == inode_out->i_sb);
 	}
 
 	/* Don't make the dst file partly checksummed */
-- 
2.26.3

Re: [PATCH v2 1/2] btrfs: remove the cross file system checks from remap

[Nikolay Borisov]

On 18.02.22 г. 16:38 ч., Josef Bacik wrote:
> The sb check is already done in do_clone_file_range, and the mnt check
> (which will hopefully go away in a subsequent patch) is done in
> ioctl_file_clone().  Remove the check in our code and put an ASSERT() to
> make sure it doesn't change underneath us.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>   fs/btrfs/reflink.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/reflink.c b/fs/btrfs/reflink.c
> index a3930da4eb3f..6fed103f1000 100644
> --- a/fs/btrfs/reflink.c
> +++ b/fs/btrfs/reflink.c
> @@ -772,9 +772,7 @@ static int btrfs_remap_file_range_prep(struct file *file_in, loff_t pos_in,
>   		if (btrfs_root_readonly(root_out))
>   			return -EROFS;
>   
> -		if (file_in->f_path.mnt != file_out->f_path.mnt ||
> -		    inode_in->i_sb != inode_out->i_sb)
> -			return -EXDEV;
> +		ASSERT(inode_in->i_sb == inode_out->i_sb);
>   	}
>   
>   	/* Don't make the dst file partly checksummed */

[PATCH v2 2/2] fs: allow cross-vfsmount reflink/dedupe

[Josef Bacik]

Currently we disallow reflink and dedupe if the two files aren't on the
same vfsmount.  However we really only need to disallow it if they're
not on the same super block.  It is very common for btrfs to have a main
subvolume that is mounted and then different subvolumes mounted at
different locations.  It's allowed to reflink between these volumes, but
the vfsmount check disallows this.  Instead fix dedupe to check for the
same superblock, and simply remove the vfsmount check for reflink as it
already does the superblock check.

Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/ioctl.c       | 4 ----
 fs/remap_range.c | 7 +------
 2 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/fs/ioctl.c b/fs/ioctl.c
index 1ed097e94af2..090bf47606ab 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -236,9 +236,6 @@ static long ioctl_file_clone(struct file *dst_file, unsigned long srcfd,
 
 	if (!src_file.file)
 		return -EBADF;
-	ret = -EXDEV;
-	if (src_file.file->f_path.mnt != dst_file->f_path.mnt)
-		goto fdput;
 	cloned = vfs_clone_file_range(src_file.file, off, dst_file, destoff,
 				      olen, 0);
 	if (cloned < 0)
@@ -247,7 +244,6 @@ static long ioctl_file_clone(struct file *dst_file, unsigned long srcfd,
 		ret = -EINVAL;
 	else
 		ret = 0;
-fdput:
 	fdput(src_file);
 	return ret;
 }
diff --git a/fs/remap_range.c b/fs/remap_range.c
index 231159682907..bc5fb006dc79 100644
--- a/fs/remap_range.c
+++ b/fs/remap_range.c
@@ -362,11 +362,6 @@ loff_t do_clone_file_range(struct file *file_in, loff_t pos_in,
 
 	WARN_ON_ONCE(remap_flags & REMAP_FILE_DEDUP);
 
-	/*
-	 * FICLONE/FICLONERANGE ioctls enforce that src and dest files are on
-	 * the same mount. Practically, they only need to be on the same file
-	 * system.
-	 */
 	if (file_inode(file_in)->i_sb != file_inode(file_out)->i_sb)
 		return -EXDEV;
 
@@ -458,7 +453,7 @@ loff_t vfs_dedupe_file_range_one(struct file *src_file, loff_t src_pos,
 		goto out_drop_write;
 
 	ret = -EXDEV;
-	if (src_file->f_path.mnt != dst_file->f_path.mnt)
+	if (file_inode(src_file)->i_sb != file_inode(dst_file)->i_sb)
 		goto out_drop_write;
 
 	ret = -EISDIR;
-- 
2.26.3