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

[PATCH] 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.

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

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

[Darrick J. Wong]

On Wed, Feb 16, 2022 at 03:21:23PM -0500, Josef Bacik wrote:
> 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.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Seems pretty spiffy to /me/... ;)
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  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
> 

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

[Wang Yugui]

Hi,
Cc: NeilBrown

btrfs cross-vfsmount reflink works well now with these 2 patches.

[PATCH] fs: allow cross-vfsmount reflink/dedupe
[PATCH] btrfs: remove the cross file system checks from remap

But nfs over btrfs still fail to do cross-vfsmount reflink.
need some patch for nfs too?

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2022/02/17

> 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.
> 
> 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

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

[NeilBrown]

On Thu, 17 Feb 2022, Wang Yugui wrote:
> Hi,
> Cc: NeilBrown
> 
> btrfs cross-vfsmount reflink works well now with these 2 patches.
> 
> [PATCH] fs: allow cross-vfsmount reflink/dedupe
> [PATCH] btrfs: remove the cross file system checks from remap
> 
> But nfs over btrfs still fail to do cross-vfsmount reflink.
> need some patch for nfs too?

NFS doesn't support reflinks at all, does it?

NeilBrown

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

[Wang Yugui]

Hi,

> On Thu, 17 Feb 2022, Wang Yugui wrote:
> > Hi,
> > Cc: NeilBrown
> > 
> > btrfs cross-vfsmount reflink works well now with these 2 patches.
> > 
> > [PATCH] fs: allow cross-vfsmount reflink/dedupe
> > [PATCH] btrfs: remove the cross file system checks from remap
> > 
> > But nfs over btrfs still fail to do cross-vfsmount reflink.
> > need some patch for nfs too?
> 
> NFS doesn't support reflinks at all, does it?

NFS support reflinks now.

# df -h /ssd
Filesystem              Type  Size  Used Avail Use% Mounted on
T640:/ssd               nfs4   17T  5.5T   12T  34% /ssd
# /bin/cp --reflink=always /ssd/1.txt /ssd/2.txt
# uname -a
Linux T7610 5.15.24-3.el7.x86_64 #1 SMP Thu Feb 17 12:13:25 CST 2022 x86_64 x86_64 x86_64 GNU/Linux


Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2022/02/17

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

[NeilBrown]

On Thu, 17 Feb 2022, Wang Yugui wrote:
> Hi,
> 
> > On Thu, 17 Feb 2022, Wang Yugui wrote:
> > > Hi,
> > > Cc: NeilBrown
> > > 
> > > btrfs cross-vfsmount reflink works well now with these 2 patches.
> > > 
> > > [PATCH] fs: allow cross-vfsmount reflink/dedupe
> > > [PATCH] btrfs: remove the cross file system checks from remap
> > > 
> > > But nfs over btrfs still fail to do cross-vfsmount reflink.
> > > need some patch for nfs too?
> > 
> > NFS doesn't support reflinks at all, does it?
> 
> NFS support reflinks now.
> 
> # df -h /ssd
> Filesystem              Type  Size  Used Avail Use% Mounted on
> T640:/ssd               nfs4   17T  5.5T   12T  34% /ssd
> # /bin/cp --reflink=always /ssd/1.txt /ssd/2.txt
> # uname -a
> Linux T7610 5.15.24-3.el7.x86_64 #1 SMP Thu Feb 17 12:13:25 CST 2022 x86_64 x86_64 x86_64 GNU/Linux
> 

So it does ..... ahhh, the CLONE command in NFSv4.2.....
This is used by the .remap_file_range file operation.  That operation
only gets called when the "from" and "to" files have the same
superblock.
btrfs has an ....  interesting concept of filesystem identity.  While
different "subvols" have the same superblock locally, when they are
exported over NFS they appear to be different filesystems and so have
different superblocks.  This is in part because btrfs cannot create
properly unique inode numbers across the whole filesystem.
Until btrfs sorts itself out, it will not be able to work with NFS
properly.

NeilBrown

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

[NeilBrown]

On Tue, 22 Feb 2022, NeilBrown wrote:
> On Thu, 17 Feb 2022, Wang Yugui wrote:
> > Hi,
> > 
> > > On Thu, 17 Feb 2022, Wang Yugui wrote:
> > > > Hi,
> > > > Cc: NeilBrown
> > > > 
> > > > btrfs cross-vfsmount reflink works well now with these 2 patches.
> > > > 
> > > > [PATCH] fs: allow cross-vfsmount reflink/dedupe
> > > > [PATCH] btrfs: remove the cross file system checks from remap
> > > > 
> > > > But nfs over btrfs still fail to do cross-vfsmount reflink.
> > > > need some patch for nfs too?
> > > 
> > > NFS doesn't support reflinks at all, does it?
> > 
> > NFS support reflinks now.
> > 
> > # df -h /ssd
> > Filesystem              Type  Size  Used Avail Use% Mounted on
> > T640:/ssd               nfs4   17T  5.5T   12T  34% /ssd
> > # /bin/cp --reflink=always /ssd/1.txt /ssd/2.txt
> > # uname -a
> > Linux T7610 5.15.24-3.el7.x86_64 #1 SMP Thu Feb 17 12:13:25 CST 2022 x86_64 x86_64 x86_64 GNU/Linux
> > 
> 
> So it does ..... ahhh, the CLONE command in NFSv4.2.....
> This is used by the .remap_file_range file operation.  That operation
> only gets called when the "from" and "to" files have the same
> superblock.
> btrfs has an ....  interesting concept of filesystem identity.  While
> different "subvols" have the same superblock locally, when they are
> exported over NFS they appear to be different filesystems and so have
> different superblocks.  This is in part because btrfs cannot create
> properly unique inode numbers across the whole filesystem.
> Until btrfs sorts itself out, it will not be able to work with NFS
> properly.

Actually, that might be a little bit simplistic...

How are you exporting the btfs filesystem on the server.
If you are exporting each subvolume separately, then they probably look
like different filesystems to NFS.  If you export just the top level and
allow the subvolumes to be accessed by name, then they should have the
same superblock and reflink should work.

NeilBrown

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

[Wang Yugui]

Hi,

> On Tue, 22 Feb 2022, NeilBrown wrote:
> > On Thu, 17 Feb 2022, Wang Yugui wrote:
> > > Hi,
> > > 
> > > > On Thu, 17 Feb 2022, Wang Yugui wrote:
> > > > > Hi,
> > > > > Cc: NeilBrown
> > > > > 
> > > > > btrfs cross-vfsmount reflink works well now with these 2 patches.
> > > > > 
> > > > > [PATCH] fs: allow cross-vfsmount reflink/dedupe
> > > > > [PATCH] btrfs: remove the cross file system checks from remap
> > > > > 
> > > > > But nfs over btrfs still fail to do cross-vfsmount reflink.
> > > > > need some patch for nfs too?
> > > > 
> > > > NFS doesn't support reflinks at all, does it?
> > > 
> > > NFS support reflinks now.
> > > 
> > > # df -h /ssd
> > > Filesystem              Type  Size  Used Avail Use% Mounted on
> > > T640:/ssd               nfs4   17T  5.5T   12T  34% /ssd
> > > # /bin/cp --reflink=always /ssd/1.txt /ssd/2.txt
> > > # uname -a
> > > Linux T7610 5.15.24-3.el7.x86_64 #1 SMP Thu Feb 17 12:13:25 CST 2022 x86_64 x86_64 x86_64 GNU/Linux
> > > 
> > 
> > So it does ..... ahhh, the CLONE command in NFSv4.2.....
> > This is used by the .remap_file_range file operation.  That operation
> > only gets called when the "from" and "to" files have the same
> > superblock.
> > btrfs has an ....  interesting concept of filesystem identity.  While
> > different "subvols" have the same superblock locally, when they are
> > exported over NFS they appear to be different filesystems and so have
> > different superblocks.  This is in part because btrfs cannot create
> > properly unique inode numbers across the whole filesystem.
> > Until btrfs sorts itself out, it will not be able to work with NFS
> > properly.
> 
> Actually, that might be a little bit simplistic...
> 
> How are you exporting the btfs filesystem on the server.
> If you are exporting each subvolume separately, then they probably look
> like different filesystems to NFS.  If you export just the top level and
> allow the subvolumes to be accessed by name, then they should have the
> same superblock and reflink should work.
> 
> NeilBrown

struct vfsmount {
	struct dentry *mnt_root;	/* root of the mounted tree */
	struct super_block *mnt_sb;	/* pointer to superblock */
	int mnt_flags;
	struct user_namespace *mnt_userns;
} __randomize_layout;

for local mount of btrfs different subvols, there are 1 same attr and 2
different attr.
same attr:
	struct super_block *mnt_sb
different:
	struct dentry *mnt_root;
	subvol attr; to save it in struct user_namespace?

for nfs mount of btrfs different subvols, there maybe similar way?

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2022/02/22

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

[David Sterba]

On Wed, Feb 16, 2022 at 03:21:23PM -0500, Josef Bacik wrote:
> 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.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

This has some history, people have been asking for this for a long time
(the workaround is to mount the toplevel subvolume elsewhere and do it
from there). I did a review back then what could possibly break on the
VFS level, eg. lack of references of the affected files, or potential
deadlocks. On the filesystem level there's no difference, the extents
are processed in the same way, it does not matter if the VFS entry point
is the same mount or cross-mount. The same-mount limitation seems rather
artificial and has been inherited from the original implementation.

Reviewed-by: David Sterba <dsterba@suse.com>

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

[Nikolay Borisov]

On 16.02.22 г. 22:21 ч., Josef Bacik wrote:
> 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.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

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