[PATCH] nfsd: Ensure CLONE persists data and metadata changes to the target file

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH] nfsd: Ensure CLONE persists data and metadata changes to the target file
@ 2019-11-27 22:05 Trond Myklebust
  2019-11-27 22:22 ` J. Bruce Fields
  2019-11-28 17:53   ` kbuild test robot
  0 siblings, 2 replies; 6+ messages in thread
From: Trond Myklebust @ 2019-11-27 22:05 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: linux-nfs

The NFSv4.2 CLONE operation has implicit persistence requirements on the
target file, since there is no protocol requirement that the client issue
a separate operation to persist data.
For that reason, we should call vfs_fsync_range() on the destination file
after a successful call to vfs_clone_file_range().

Fixes: ffa0160a1039 ("nfsd: implement the NFSv4.2 CLONE operation")
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Cc: stable@vger.kernel.org # v4.5+
---
 fs/nfsd/nfs4proc.c | 3 ++-
 fs/nfsd/vfs.c      | 9 ++++++++-
 fs/nfsd/vfs.h      | 2 +-
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 4e3e77b76411..38c0aeda500e 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1077,7 +1077,8 @@ nfsd4_clone(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		goto out;
 
 	status = nfsd4_clone_file_range(src->nf_file, clone->cl_src_pos,
-			dst->nf_file, clone->cl_dst_pos, clone->cl_count);
+			dst->nf_file, clone->cl_dst_pos, clone->cl_count,
+			EX_ISSYNC(cstate->current_fh.fh_export));
 
 	nfsd_file_put(dst);
 	nfsd_file_put(src);
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index bd0a385df3fc..9d66c4719067 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -525,7 +525,7 @@ __be32 nfsd4_set_nfs4_label(struct svc_rqst *rqstp, struct svc_fh *fhp,
 #endif
 
 __be32 nfsd4_clone_file_range(struct file *src, u64 src_pos, struct file *dst,
-		u64 dst_pos, u64 count)
+		u64 dst_pos, u64 count, bool sync)
 {
 	loff_t cloned;
 
@@ -534,6 +534,13 @@ __be32 nfsd4_clone_file_range(struct file *src, u64 src_pos, struct file *dst,
 		return nfserrno(cloned);
 	if (count && cloned != count)
 		return nfserrno(-EINVAL);
+	if (sync) {
+		loff_t dst_end = count ? dst_pos + count - 1 : LLONG_MAX;
+		int status = vfs_fsync_range(dst, dst_pos, dst_end,
+				commit_is_datasync);
+		if (status < 0)
+			return nfserrno(status);
+	}
 	return 0;
 }
 
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index a13fd9d7e1f5..cc110a10bfe8 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -56,7 +56,7 @@ __be32          nfsd4_set_nfs4_label(struct svc_rqst *, struct svc_fh *,
 __be32		nfsd4_vfs_fallocate(struct svc_rqst *, struct svc_fh *,
 				    struct file *, loff_t, loff_t, int);
 __be32		nfsd4_clone_file_range(struct file *, u64, struct file *,
-			u64, u64);
+				       u64, u64, bool);
 #endif /* CONFIG_NFSD_V4 */
 __be32		nfsd_create_locked(struct svc_rqst *, struct svc_fh *,
 				char *name, int len, struct iattr *attrs,
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] nfsd: Ensure CLONE persists data and metadata changes to the target file
  2019-11-27 22:05 [PATCH] nfsd: Ensure CLONE persists data and metadata changes to the target file Trond Myklebust
@ 2019-11-27 22:22 ` J. Bruce Fields
  2019-11-28 17:53   ` kbuild test robot
  1 sibling, 0 replies; 6+ messages in thread
From: J. Bruce Fields @ 2019-11-27 22:22 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: J. Bruce Fields, linux-nfs

On Wed, Nov 27, 2019 at 05:05:51PM -0500, Trond Myklebust wrote:
> The NFSv4.2 CLONE operation has implicit persistence requirements on the
> target file, since there is no protocol requirement that the client issue
> a separate operation to persist data.
> For that reason, we should call vfs_fsync_range() on the destination file
> after a successful call to vfs_clone_file_range().

Looking at RFC 7862....  So, COPY has a stable_how4 field in the reply,
but CLONE doesn't.  That's interesting!  OK, I guess this makes sense,
then.

--b.

> 
> Fixes: ffa0160a1039 ("nfsd: implement the NFSv4.2 CLONE operation")
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> Cc: stable@vger.kernel.org # v4.5+
> ---
>  fs/nfsd/nfs4proc.c | 3 ++-
>  fs/nfsd/vfs.c      | 9 ++++++++-
>  fs/nfsd/vfs.h      | 2 +-
>  3 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 4e3e77b76411..38c0aeda500e 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1077,7 +1077,8 @@ nfsd4_clone(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  		goto out;
>  
>  	status = nfsd4_clone_file_range(src->nf_file, clone->cl_src_pos,
> -			dst->nf_file, clone->cl_dst_pos, clone->cl_count);
> +			dst->nf_file, clone->cl_dst_pos, clone->cl_count,
> +			EX_ISSYNC(cstate->current_fh.fh_export));
>  
>  	nfsd_file_put(dst);
>  	nfsd_file_put(src);
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index bd0a385df3fc..9d66c4719067 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -525,7 +525,7 @@ __be32 nfsd4_set_nfs4_label(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  #endif
>  
>  __be32 nfsd4_clone_file_range(struct file *src, u64 src_pos, struct file *dst,
> -		u64 dst_pos, u64 count)
> +		u64 dst_pos, u64 count, bool sync)
>  {
>  	loff_t cloned;
>  
> @@ -534,6 +534,13 @@ __be32 nfsd4_clone_file_range(struct file *src, u64 src_pos, struct file *dst,
>  		return nfserrno(cloned);
>  	if (count && cloned != count)
>  		return nfserrno(-EINVAL);
> +	if (sync) {
> +		loff_t dst_end = count ? dst_pos + count - 1 : LLONG_MAX;
> +		int status = vfs_fsync_range(dst, dst_pos, dst_end,
> +				commit_is_datasync);
> +		if (status < 0)
> +			return nfserrno(status);
> +	}
>  	return 0;
>  }
>  
> diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
> index a13fd9d7e1f5..cc110a10bfe8 100644
> --- a/fs/nfsd/vfs.h
> +++ b/fs/nfsd/vfs.h
> @@ -56,7 +56,7 @@ __be32          nfsd4_set_nfs4_label(struct svc_rqst *, struct svc_fh *,
>  __be32		nfsd4_vfs_fallocate(struct svc_rqst *, struct svc_fh *,
>  				    struct file *, loff_t, loff_t, int);
>  __be32		nfsd4_clone_file_range(struct file *, u64, struct file *,
> -			u64, u64);
> +				       u64, u64, bool);
>  #endif /* CONFIG_NFSD_V4 */
>  __be32		nfsd_create_locked(struct svc_rqst *, struct svc_fh *,
>  				char *name, int len, struct iattr *attrs,
> -- 
> 2.23.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] nfsd: Ensure CLONE persists data and metadata changes to the target file
  2019-11-27 22:05 [PATCH] nfsd: Ensure CLONE persists data and metadata changes to the target file Trond Myklebust
@ 2019-11-28 17:53   ` kbuild test robot
  2019-11-28 17:53   ` kbuild test robot
  1 sibling, 0 replies; 6+ messages in thread
From: kbuild test robot @ 2019-11-28 17:53 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: kbuild-all, J. Bruce Fields, linux-nfs

[-- Attachment #1: Type: text/plain, Size: 2305 bytes --]

Hi Trond,

I love your patch! Yet something to improve:

[auto build test ERROR on nfsd/nfsd-next]
[also build test ERROR on v5.4 next-20191128]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Trond-Myklebust/nfsd-Ensure-CLONE-persists-data-and-metadata-changes-to-the-target-file/20191128-061009
base:   git://linux-nfs.org/~bfields/linux.git nfsd-next
config: parisc-defconfig (attached as .config)
compiler: hppa-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=parisc 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   fs//nfsd/vfs.c: In function 'nfsd4_clone_file_range':
>> fs//nfsd/vfs.c:540:5: error: 'commit_is_datasync' undeclared (first use in this function); did you mean 'commit_metadata'?
        commit_is_datasync);
        ^~~~~~~~~~~~~~~~~~
        commit_metadata
   fs//nfsd/vfs.c:540:5: note: each undeclared identifier is reported only once for each function it appears in

vim +540 fs//nfsd/vfs.c

   526	
   527	__be32 nfsd4_clone_file_range(struct file *src, u64 src_pos, struct file *dst,
   528			u64 dst_pos, u64 count, bool sync)
   529	{
   530		loff_t cloned;
   531	
   532		cloned = vfs_clone_file_range(src, src_pos, dst, dst_pos, count, 0);
   533		if (cloned < 0)
   534			return nfserrno(cloned);
   535		if (count && cloned != count)
   536			return nfserrno(-EINVAL);
   537		if (sync) {
   538			loff_t dst_end = count ? dst_pos + count - 1 : LLONG_MAX;
   539			int status = vfs_fsync_range(dst, dst_pos, dst_end,
 > 540					commit_is_datasync);
   541			if (status < 0)
   542				return nfserrno(status);
   543		}
   544		return 0;
   545	}
   546	

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 16278 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] nfsd: Ensure CLONE persists data and metadata changes to the target file
@ 2019-11-28 17:53   ` kbuild test robot
  0 siblings, 0 replies; 6+ messages in thread
From: kbuild test robot @ 2019-11-28 17:53 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2364 bytes --]

Hi Trond,

I love your patch! Yet something to improve:

[auto build test ERROR on nfsd/nfsd-next]
[also build test ERROR on v5.4 next-20191128]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Trond-Myklebust/nfsd-Ensure-CLONE-persists-data-and-metadata-changes-to-the-target-file/20191128-061009
base:   git://linux-nfs.org/~bfields/linux.git nfsd-next
config: parisc-defconfig (attached as .config)
compiler: hppa-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=parisc 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   fs//nfsd/vfs.c: In function 'nfsd4_clone_file_range':
>> fs//nfsd/vfs.c:540:5: error: 'commit_is_datasync' undeclared (first use in this function); did you mean 'commit_metadata'?
        commit_is_datasync);
        ^~~~~~~~~~~~~~~~~~
        commit_metadata
   fs//nfsd/vfs.c:540:5: note: each undeclared identifier is reported only once for each function it appears in

vim +540 fs//nfsd/vfs.c

   526	
   527	__be32 nfsd4_clone_file_range(struct file *src, u64 src_pos, struct file *dst,
   528			u64 dst_pos, u64 count, bool sync)
   529	{
   530		loff_t cloned;
   531	
   532		cloned = vfs_clone_file_range(src, src_pos, dst, dst_pos, count, 0);
   533		if (cloned < 0)
   534			return nfserrno(cloned);
   535		if (count && cloned != count)
   536			return nfserrno(-EINVAL);
   537		if (sync) {
   538			loff_t dst_end = count ? dst_pos + count - 1 : LLONG_MAX;
   539			int status = vfs_fsync_range(dst, dst_pos, dst_end,
 > 540					commit_is_datasync);
   541			if (status < 0)
   542				return nfserrno(status);
   543		}
   544		return 0;
   545	}
   546	

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 16278 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] nfsd: Ensure CLONE persists data and metadata changes to the target file
  2019-11-28 17:53   ` kbuild test robot
  (?)
@ 2019-11-30 19:58   ` J. Bruce Fields
  2019-11-30 21:21     ` Trond Myklebust
  -1 siblings, 1 reply; 6+ messages in thread
From: J. Bruce Fields @ 2019-11-30 19:58 UTC (permalink / raw)
  To: kbuild test robot; +Cc: Trond Myklebust, kbuild-all, J. Bruce Fields, linux-nfs

On Fri, Nov 29, 2019 at 01:53:13AM +0800, kbuild test robot wrote:
>    fs//nfsd/vfs.c: In function 'nfsd4_clone_file_range':
> >> fs//nfsd/vfs.c:540:5: error: 'commit_is_datasync' undeclared (first use in this function); did you mean 'commit_metadata'?
...
>    539			int status = vfs_fsync_range(dst, dst_pos, dst_end,
>  > 540					commit_is_datasync);

I've replaced commit_is_datasync by 0.  Trond, correct me if I got that
wrong.

--b.

>    541			if (status < 0)
>    542				return nfserrno(status);
>    543		}
>    544		return 0;
>    545	}
>    546	
> 
> ---
> 0-DAY kernel test infrastructure                 Open Source Technology Center
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] nfsd: Ensure CLONE persists data and metadata changes to the target file
  2019-11-30 19:58   ` J. Bruce Fields
@ 2019-11-30 21:21     ` Trond Myklebust
  0 siblings, 0 replies; 6+ messages in thread
From: Trond Myklebust @ 2019-11-30 21:21 UTC (permalink / raw)
  To: bfields@fieldses.org, lkp@intel.com
  Cc: kbuild-all@lists.01.org, linux-nfs@vger.kernel.org,
	bfields@redhat.com

On Sat, 2019-11-30 at 14:58 -0500, J. Bruce Fields wrote:
> On Fri, Nov 29, 2019 at 01:53:13AM +0800, kbuild test robot wrote:
> >    fs//nfsd/vfs.c: In function 'nfsd4_clone_file_range':
> > > > fs//nfsd/vfs.c:540:5: error: 'commit_is_datasync' undeclared
> > > > (first use in this function); did you mean 'commit_metadata'?
> ...
> >    539			int status = vfs_fsync_range(dst,
> > dst_pos, dst_end,
> >  > 540					commit_is_datasync);
> 
> I've replaced commit_is_datasync by 0.  Trond, correct me if I got
> that
> wrong.

Doh! Sorry about that. I blame ENOCOFFEE...

Thanks!
  Trond
> 
> --b.
> 
> >    541			if (status < 0)
> >    542				return nfserrno(status);
> >    543		}
> >    544		return 0;
> >    545	}
> >    546	
> > 
> > ---
> > 0-DAY kernel test infrastructure                 Open Source
> > Technology Center
> > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel
> > Corporation
> 
> 
-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-11-30 21:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-11-27 22:05 [PATCH] nfsd: Ensure CLONE persists data and metadata changes to the target file Trond Myklebust
2019-11-27 22:22 ` J. Bruce Fields
2019-11-28 17:53 ` kbuild test robot
2019-11-28 17:53   ` kbuild test robot
2019-11-30 19:58   ` J. Bruce Fields
2019-11-30 21:21     ` Trond Myklebust

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.