[2/4] Btrfs: use cross mount point check for cloning and deduplication
diff mbox series

Message ID 20181212180559.15249-3-fdmanana@kernel.org
State New
Headers show
Series
  • Btrfs: a few more cleanups and fixes for clone/deduplication
Related show

Commit Message

Filipe Manana Dec. 12, 2018, 6:05 p.m. UTC
From: Filipe Manana <fdmanana@suse.com>

There is no reason why this check was performed for clone operations but
not for deduplication operations, after all deduplication is a special
case of cloning. So make the check happen for deduplication as well.

This check used to be done and got removed by accident in commit
2b3909f8a7fe9 ("btrfs: use new dedupe data function pointer").

Fixes: 2b3909f8a7fe9 ("btrfs: use new dedupe data function pointer").
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/ioctl.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

David Sterba Dec. 13, 2018, 4:02 p.m. UTC | #1
On Wed, Dec 12, 2018 at 06:05:57PM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> There is no reason why this check was performed for clone operations but
> not for deduplication operations, after all deduplication is a special
> case of cloning. So make the check happen for deduplication as well.
> 
> This check used to be done and got removed by accident in commit
> 2b3909f8a7fe9 ("btrfs: use new dedupe data function pointer").
> 
> Fixes: 2b3909f8a7fe9 ("btrfs: use new dedupe data function pointer").
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
>  fs/btrfs/ioctl.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 1e90d10b5638..ffe940ceb80a 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -3912,12 +3912,12 @@ 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;
>  	}
>  
> +	if (file_in->f_path.mnt != file_out->f_path.mnt ||
> +	    inode_in->i_sb != inode_out->i_sb)
> +		return -EXDEV;

I'm checking if this is or is not a change in the dedupe behaviour
regarding dedupe on different mont points.

I can't see the f_path.mnt check anywhere in the VFS calls so after that
patch deduping stops to work in some cases.
David Sterba Jan. 11, 2019, 2:38 p.m. UTC | #2
On Thu, Dec 13, 2018 at 05:02:24PM +0100, David Sterba wrote:
> On Wed, Dec 12, 2018 at 06:05:57PM +0000, fdmanana@kernel.org wrote:
> > From: Filipe Manana <fdmanana@suse.com>
> > 
> > There is no reason why this check was performed for clone operations but
> > not for deduplication operations, after all deduplication is a special
> > case of cloning. So make the check happen for deduplication as well.
> > 
> > This check used to be done and got removed by accident in commit
> > 2b3909f8a7fe9 ("btrfs: use new dedupe data function pointer").
> > 
> > Fixes: 2b3909f8a7fe9 ("btrfs: use new dedupe data function pointer").
> > Signed-off-by: Filipe Manana <fdmanana@suse.com>
> > ---
> >  fs/btrfs/ioctl.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> > index 1e90d10b5638..ffe940ceb80a 100644
> > --- a/fs/btrfs/ioctl.c
> > +++ b/fs/btrfs/ioctl.c
> > @@ -3912,12 +3912,12 @@ 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;
> >  	}
> >  
> > +	if (file_in->f_path.mnt != file_out->f_path.mnt ||
> > +	    inode_in->i_sb != inode_out->i_sb)
> > +		return -EXDEV;
> 
> I'm checking if this is or is not a change in the dedupe behaviour
> regarding dedupe on different mont points.
> 
> I can't see the f_path.mnt check anywhere in the VFS calls so after that
> patch deduping stops to work in some cases.

Dedupe on cross-mount has been broken (or working) since 4.5, that's a
lot of releases with at least 4 stables in between. Changing that back
might break applications, I've asked on irc but none of the widely used
tools depend on that.

As usual we can never know for sure, so I'd like to explore the option
to keep it working. We'd also like to drop the cross-mount limitation
for clone that's something users do want. However this still has some
opposition from VFS people.

Unless there's a technical/stability reason to add the check back, I'd
like to keep it working.

Patch
diff mbox series

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 1e90d10b5638..ffe940ceb80a 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3912,12 +3912,12 @@  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;
 	}
 
+	if (file_in->f_path.mnt != file_out->f_path.mnt ||
+	    inode_in->i_sb != inode_out->i_sb)
+		return -EXDEV;
+
 	if (same_inode)
 		inode_lock(inode_in);
 	else