[v4,11/11] NFS: replace cross device check in copy_file_range

Message ID 20181026201057.36899-13-olga.kornievskaia@gmail.com
State New
Headers show
Series
  • client-side support for "inter" SSC copy
Related show

Commit Message

Olga Kornievskaia Oct. 26, 2018, 8:10 p.m.
From: Olga Kornievskaia <kolga@netapp.com>

Add a check to disallow cross file systems copy offload, both
files are expected to be of NFS4.2+ type.

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

Comments

Matthew Wilcox Oct. 26, 2018, 9:22 p.m. | #1
On Fri, Oct 26, 2018 at 04:10:57PM -0400, Olga Kornievskaia wrote:
> From: Olga Kornievskaia <kolga@netapp.com>
> 
> Add a check to disallow cross file systems copy offload, both
> files are expected to be of NFS4.2+ type.
> 
> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>

Reviewed-by: Matthew Wilcox <willy@infradead.org>
Jeff Layton Oct. 27, 2018, 11:08 a.m. | #2
On Fri, 2018-10-26 at 16:10 -0400, Olga Kornievskaia wrote:
> From: Olga Kornievskaia <kolga@netapp.com>
> 
> Add a check to disallow cross file systems copy offload, both
> files are expected to be of NFS4.2+ type.
> 
> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> ---
>  fs/nfs/nfs4file.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c
> index 2f31f30..344e9dd 100644
> --- a/fs/nfs/nfs4file.c
> +++ b/fs/nfs/nfs4file.c
> @@ -139,8 +139,14 @@ 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_op != &nfs4_file_operations)
>  		return -EXDEV;
> +	else {

nit: you don't really need the "else" here since the previous block
returns

> +		struct nfs_client *c_in =
> +			(NFS_SERVER(file_inode(file_in)))->nfs_client;
> +		if (c_in->cl_minorversion < 2)
> +			return -EXDEV;
> +	}
>  
>  	if (file_inode(file_in) == file_inode(file_out))
>  		return -EINVAL;

Looks fine though.

Reviewed-by: Jeff Layton <jlayton@kernel.org>
Matthew Wilcox Oct. 27, 2018, 1:26 p.m. | #3
On Sat, Oct 27, 2018 at 07:08:11AM -0400, Jeff Layton wrote:
> >  
> > -	if (file_in->f_inode->i_sb != file_out->f_inode->i_sb)
> > +	if (file_in->f_op != &nfs4_file_operations)
> >  		return -EXDEV;
> > +	else {
> 
> nit: you don't really need the "else" here since the previous block
> returns
> 
> > +		struct nfs_client *c_in =
> > +			(NFS_SERVER(file_inode(file_in)))->nfs_client;
> > +		if (c_in->cl_minorversion < 2)
> > +			return -EXDEV;
> > +	}

Yeah, but if you don't have the else, then you need to declare the c_in
at the beginning of the function instead of in the new block.  Mind you,
if you do that then:

	c_in = NFS_SERVER(file_inode(file_in))->nfs_client;

fits on one line, so it does look a bit neater.
Olga Kornievskaia Oct. 29, 2018, 2:28 p.m. | #4
On Sat, Oct 27, 2018 at 9:26 AM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Sat, Oct 27, 2018 at 07:08:11AM -0400, Jeff Layton wrote:
> > >
> > > -   if (file_in->f_inode->i_sb != file_out->f_inode->i_sb)
> > > +   if (file_in->f_op != &nfs4_file_operations)
> > >             return -EXDEV;
> > > +   else {
> >
> > nit: you don't really need the "else" here since the previous block
> > returns
> >
> > > +           struct nfs_client *c_in =
> > > +                   (NFS_SERVER(file_inode(file_in)))->nfs_client;
> > > +           if (c_in->cl_minorversion < 2)
> > > +                   return -EXDEV;
> > > +   }
>
> Yeah, but if you don't have the else, then you need to declare the c_in
> at the beginning of the function instead of in the new block.  Mind you,
> if you do that then:
>
>         c_in = NFS_SERVER(file_inode(file_in))->nfs_client;
>
> fits on one line, so it does look a bit neater.

My thoughts for the "else" was to be able to get the nfs_client but
yes I could declare for the whole function and assign after the first
"if". I'll change it.

Patch

diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c
index 2f31f30..344e9dd 100644
--- a/fs/nfs/nfs4file.c
+++ b/fs/nfs/nfs4file.c
@@ -139,8 +139,14 @@  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_op != &nfs4_file_operations)
 		return -EXDEV;
+	else {
+		struct nfs_client *c_in =
+			(NFS_SERVER(file_inode(file_in)))->nfs_client;
+		if (c_in->cl_minorversion < 2)
+			return -EXDEV;
+	}
 
 	if (file_inode(file_in) == file_inode(file_out))
 		return -EINVAL;