diff mbox series

[v3,11/11] NFS replace cross device with cross filesystem check in copy_file_range

Message ID 20181025215147.36248-13-olga.kornievskaia@gmail.com (mailing list archive)
State New, archived
Headers show
Series client-side support for "inter" SSC copy | expand

Commit Message

Olga Kornievskaia Oct. 25, 2018, 9:51 p.m. UTC 
From: Olga Kornievskaia <kolga@netapp.com>

VFS copy_file_range was relaxed to allow for cross-device copy.
NFS code was added to support for server to server copy offload.
Add a check to disallow cross file systems copy offload, both
files are expected to be of NFS type.

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfs/nfs4file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Matthew Wilcox Oct. 25, 2018, 10:27 p.m. UTC | #1 
On Thu, Oct 25, 2018 at 05:51:47PM -0400, Olga Kornievskaia wrote:
> -	if (file_in->f_inode->i_sb != file_out->f_inode->i_sb)
> +	if (file_in->f_inode->i_sb->s_type != file_out->f_inode->i_sb->s_type)

Isn't it simpler to do:

	if (file_in->f_inode->i_sb->s_type != &nfs4_fs_type)

But now I've done some grepping, and I wonder whether it's possible
/ desirable to also copy between inodes on a nfs4_remote_fs_type,
nfs4_remote_referral_fs_type, nfs4_referral_fs_type or nfs4_xdev_fs_type.
I haven't kept up with NFS development for a while, so maybe none of
those are possible.
Olga Kornievskaia Oct. 26, 2018, 12:45 p.m. UTC | #2 
On Thu, Oct 25, 2018 at 6:27 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Thu, Oct 25, 2018 at 05:51:47PM -0400, Olga Kornievskaia wrote:
> > -     if (file_in->f_inode->i_sb != file_out->f_inode->i_sb)
> > +     if (file_in->f_inode->i_sb->s_type != file_out->f_inode->i_sb->s_type)
>
> Isn't it simpler to do:
>
>         if (file_in->f_inode->i_sb->s_type != &nfs4_fs_type)

I don't believe that's sufficient. What if "in"  fs was NFS and "out"
fs was CIFS?

> But now I've done some grepping, and I wonder whether it's possible
> / desirable to also copy between inodes on a nfs4_remote_fs_type,
> nfs4_remote_referral_fs_type, nfs4_referral_fs_type or nfs4_xdev_fs_type.
> I haven't kept up with NFS development for a while, so maybe none of
> those are possible.

Thank you for pointing it out. Grepping thru the code, don't believe
the last one is used but for the migration and replication types, I
don't know if there are any caveats that would preclude support for
copy offload I'll check with Chuck Lever. Then the check should be
that "in" and "out" are any of the 4 NFS types.
Matthew Wilcox Oct. 26, 2018, 12:54 p.m. UTC | #3 
On Fri, Oct 26, 2018 at 08:45:38AM -0400, Olga Kornievskaia wrote:
> On Thu, Oct 25, 2018 at 6:27 PM Matthew Wilcox <willy@infradead.org> wrote:
> >
> > On Thu, Oct 25, 2018 at 05:51:47PM -0400, Olga Kornievskaia wrote:
> > > -     if (file_in->f_inode->i_sb != file_out->f_inode->i_sb)
> > > +     if (file_in->f_inode->i_sb->s_type != file_out->f_inode->i_sb->s_type)
> >
> > Isn't it simpler to do:
> >
> >         if (file_in->f_inode->i_sb->s_type != &nfs4_fs_type)
> 
> I don't believe that's sufficient. What if "in"  fs was NFS and "out"
> fs was CIFS?

This is the NFS implementation, so you know that 'out' is an NFS file.

> > But now I've done some grepping, and I wonder whether it's possible
> > / desirable to also copy between inodes on a nfs4_remote_fs_type,
> > nfs4_remote_referral_fs_type, nfs4_referral_fs_type or nfs4_xdev_fs_type.
> > I haven't kept up with NFS development for a while, so maybe none of
> > those are possible.
> 
> Thank you for pointing it out. Grepping thru the code, don't believe
> the last one is used but for the migration and replication types, I
> don't know if there are any caveats that would preclude support for
> copy offload I'll check with Chuck Lever. Then the check should be
> that "in" and "out" are any of the 4 NFS types.
diff mbox series

Patch

diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c
index 4f142db..aab3720 100644
--- a/fs/nfs/nfs4file.c
+++ b/fs/nfs/nfs4file.c
@@ -139,7 +139,7 @@  static ssize_t nfs4_copy_file_range(struct file *file_in, loff_t pos_in,
 	nfs4_stateid *cnrs = NULL;
 	ssize_t ret;
 
-	if (file_in->f_inode->i_sb != file_out->f_inode->i_sb)
+	if (file_in->f_inode->i_sb->s_type != file_out->f_inode->i_sb->s_type)
 		return -EXDEV;
 
 	if (file_inode(file_in) == file_inode(file_out))