| Message ID | 20181212180559.15249-3-fdmanana@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Btrfs: a few more cleanups and fixes for clone/deduplication | expand |
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.
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.
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