diff mbox series

[v5,12/12] NFS: replace cross device check in copy_file_range

Message ID 20181029174059.38326-14-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. 29, 2018, 5:40 p.m. UTC 
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.

Reviewed-by: Jeff Layton <jlayton@kernel.org>
Reviewed-by: Matthew Wilcox <willy@infradead.org>
Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfs/nfs4file.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Olga Kornievskaia Oct. 29, 2018, 5:46 p.m. UTC | #1 
Please note this is PATCH v5 (it got added to the thread of PATCH v4).

On Mon, Oct 29, 2018 at 1:41 PM Olga Kornievskaia
<olga.kornievskaia@gmail.com> 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.
>
> Reviewed-by: Jeff Layton <jlayton@kernel.org>
> Reviewed-by: Matthew Wilcox <willy@infradead.org>
> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> ---
>  fs/nfs/nfs4file.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c
> index 989f174..fde4465 100644
> --- a/fs/nfs/nfs4file.c
> +++ b/fs/nfs/nfs4file.c
> @@ -138,11 +138,16 @@ static ssize_t nfs4_copy_file_range(struct file *file_in, loff_t pos_in,
>         struct nl4_server *nss = NULL;
>         nfs4_stateid *cnrs = NULL;
>         ssize_t ret;
> +       struct nfs_client *c_in;
>
>         if (pos_in >= i_size_read(file_inode(file_in)))
>                 return -EINVAL;
>
> -       if (file_inode(file_in)->i_sb != file_inode(file_out)->i_sb)
> +       if (file_in->f_op != &nfs4_file_operations)
> +               return -EXDEV;
> +
> +       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))
> --
> 1.8.3.1
>
Matthew Wilcox Oct. 29, 2018, 5:51 p.m. UTC | #2 
On Mon, Oct 29, 2018 at 01:40:59PM -0400, Olga Kornievskaia wrote:
> +	c_in = (NFS_SERVER(file_inode(file_in)))->nfs_client;

Seems to be an unnecessary pair of brackets there, given existing code:

fs/nfs/delegation.c:    struct nfs_client *clp = NFS_SERVER(inode)->nfs_client;
fs/nfs/dir.c:                                     NFS_SERVER(inode)->dtsize, desc->plus);
fs/nfs/file.c:  if (NFS_SERVER(inode)->flags & NFS_MOUNT_LOCAL_FCNTL)
(etc)

> +	if (c_in->cl_minorversion < 2)

in fact, can't you make this even simpler ...

	if (NFS_SERVER(inode)->nfs_client->cl_minorversion < 2)

I thought you needed to cast it, but code like this:

fs/nfs/nfs4proc.c: NFS_SERVER(dir)->nfs_client->cl_hostname);

would indicate not.
Matthew Wilcox Oct. 29, 2018, 5:52 p.m. UTC | #3 
On Mon, Oct 29, 2018 at 10:51:14AM -0700, Matthew Wilcox wrote:
> On Mon, Oct 29, 2018 at 01:40:59PM -0400, Olga Kornievskaia wrote:
> > +	c_in = (NFS_SERVER(file_inode(file_in)))->nfs_client;
> 
> Seems to be an unnecessary pair of brackets there, given existing code:
> 
> fs/nfs/delegation.c:    struct nfs_client *clp = NFS_SERVER(inode)->nfs_client;
> fs/nfs/dir.c:                                     NFS_SERVER(inode)->dtsize, desc->plus);
> fs/nfs/file.c:  if (NFS_SERVER(inode)->flags & NFS_MOUNT_LOCAL_FCNTL)
> (etc)
> 
> > +	if (c_in->cl_minorversion < 2)
> 
> in fact, can't you make this even simpler ...
> 
> 	if (NFS_SERVER(inode)->nfs_client->cl_minorversion < 2)

Oops.  Typed too fast:

	if (NFS_SERVER(file_inode(file_in)))->nfs_client->cl_minorversion < 2)

> I thought you needed to cast it, but code like this:
> 
> fs/nfs/nfs4proc.c: NFS_SERVER(dir)->nfs_client->cl_hostname);
> 
> would indicate not.
Olga Kornievskaia Oct. 29, 2018, 5:56 p.m. UTC | #4 
On Mon, Oct 29, 2018 at 1:52 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Mon, Oct 29, 2018 at 10:51:14AM -0700, Matthew Wilcox wrote:
> > On Mon, Oct 29, 2018 at 01:40:59PM -0400, Olga Kornievskaia wrote:
> > > +   c_in = (NFS_SERVER(file_inode(file_in)))->nfs_client;
> >
> > Seems to be an unnecessary pair of brackets there, given existing code:
> >
> > fs/nfs/delegation.c:    struct nfs_client *clp = NFS_SERVER(inode)->nfs_client;
> > fs/nfs/dir.c:                                     NFS_SERVER(inode)->dtsize, desc->plus);
> > fs/nfs/file.c:  if (NFS_SERVER(inode)->flags & NFS_MOUNT_LOCAL_FCNTL)
> > (etc)
> >
> > > +   if (c_in->cl_minorversion < 2)
> >
> > in fact, can't you make this even simpler ...
> >
> >       if (NFS_SERVER(inode)->nfs_client->cl_minorversion < 2)
>
> Oops.  Typed too fast:
>
>         if (NFS_SERVER(file_inode(file_in)))->nfs_client->cl_minorversion < 2)
>
> > I thought you needed to cast it, but code like this:
> >
> > fs/nfs/nfs4proc.c: NFS_SERVER(dir)->nfs_client->cl_hostname);
> >
> > would indicate not.

You are right. Then no declaration is needed. I'll change it. Thank you.
diff mbox series

Patch

diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c
index 989f174..fde4465 100644
--- a/fs/nfs/nfs4file.c
+++ b/fs/nfs/nfs4file.c
@@ -138,11 +138,16 @@  static ssize_t nfs4_copy_file_range(struct file *file_in, loff_t pos_in,
 	struct nl4_server *nss = NULL;
 	nfs4_stateid *cnrs = NULL;
 	ssize_t ret;
+	struct nfs_client *c_in;
 
 	if (pos_in >= i_size_read(file_inode(file_in)))
 		return -EINVAL;
 
-	if (file_inode(file_in)->i_sb != file_inode(file_out)->i_sb)
+	if (file_in->f_op != &nfs4_file_operations)
+		return -EXDEV;
+
+	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))