[PATCH v1 0/3] Clean up fs/nfsd/vfs.c includes

[PATCH v1 0/3] Clean up fs/nfsd/vfs.c includes

[Chuck Lever]

A quick set of clean-ups that permit us to remove more version-
specific material from fs/nfsd/vfs.c.

[ re-posting v1; the first post did not include linux-nfs@ ]

Chuck Lever (3):
  NFSD: Make the write verifier reset helper available outside vfs.c
  NFSD: Move NFSv4-specific CLONE logic into nfsd4_clone()
  NFSD: Remove xdr-related headers from fs/nfsd/vfs.c

 fs/nfsd/nfs4proc.c |  27 ++++++++--
 fs/nfsd/vfs.c      | 129 ++++++++++++++++++++++++++-------------------
 fs/nfsd/vfs.h      |  14 +++--
 3 files changed, 108 insertions(+), 62 deletions(-)

-- 
2.54.0

[PATCH v1 1/3] NFSD: Make the write verifier reset helper available outside vfs.c

[Chuck Lever]

A subsequent patch moves the NFSv4-specific portions of
nfsd4_clone_file_range() out of fs/nfsd/vfs.c and into its caller in
fs/nfsd/nfs4proc.c. One of those portions resets the write verifier
when the post-clone sync fails.

netns.h already exposes nfsd_reset_write_verifier(), but that is the
unconditional reset. commit_reset_write_verifier() wraps it with the
policy that decides which errors warrant a reset: -EAGAIN and -ESTALE
do not indicate a problem with durable storage, so they leave the
verifier alone. A caller outside vfs.c has to apply the same policy,
so make the wrapper visible rather than duplicate its switch.

Rename it to nfsd_maybe_reset_write_verifier() on the way out.

Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfsd/vfs.c | 29 ++++++++++++++++++++---------
 fs/nfsd/vfs.h |  5 +++++
 2 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 6d865f4f9ba3..fad51a899062 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -352,9 +352,20 @@ nfsd_lookup(struct svc_rqst *rqstp, struct svc_fh *fhp, const char *name,
 	return err;
 }
 
-static void
-commit_reset_write_verifier(struct nfsd_net *nn, struct svc_rqst *rqstp,
-			    int err)
+/**
+ * nfsd_maybe_reset_write_verifier - Reset the write verifier after an I/O error
+ * @nn: nfsd namespace holding the write verifier
+ * @rqstp: RPC transaction context
+ * @err: errno reported by the failed operation
+ *
+ * A write verifier reset tells clients that unstable data the server has
+ * already acknowledged might have been lost. Client response is to resend
+ * in-flight dirty data.
+ *
+ * Context: Process context.
+ */
+void nfsd_maybe_reset_write_verifier(struct nfsd_net *nn,
+				     struct svc_rqst *rqstp, int err)
 {
 	switch (err) {
 	case -EAGAIN:
@@ -735,7 +746,7 @@ __be32 nfsd4_clone_file_range(struct svc_rqst *rqstp,
 					&nfsd4_get_cstate(rqstp)->current_fh,
 					dst_pos,
 					count, status);
-			commit_reset_write_verifier(nn, rqstp, status);
+			nfsd_maybe_reset_write_verifier(nn, rqstp, status);
 			ret = nfserrno(status);
 		}
 	}
@@ -1472,21 +1483,21 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		break;
 	}
 	if (host_err < 0) {
-		commit_reset_write_verifier(nn, rqstp, host_err);
+		nfsd_maybe_reset_write_verifier(nn, rqstp, host_err);
 		goto out_nfserr;
 	}
 	nfsd_stats_io_write_add(nn, exp, *cnt);
 	fsnotify_modify(file);
 	host_err = filemap_check_wb_err(file->f_mapping, since);
 	if (host_err < 0) {
-		commit_reset_write_verifier(nn, rqstp, host_err);
+		nfsd_maybe_reset_write_verifier(nn, rqstp, host_err);
 		goto out_nfserr;
 	}
 
 	if (iocb_flags && fhp->fh_use_wgather) {
 		host_err = wait_for_concurrent_writes(file);
 		if (host_err < 0)
-			commit_reset_write_verifier(nn, rqstp, host_err);
+			nfsd_maybe_reset_write_verifier(nn, rqstp, host_err);
 	}
 
 out_nfserr:
@@ -1662,14 +1673,14 @@ nfsd_commit(struct svc_rqst *rqstp, struct svc_fh *fhp, struct nfsd_file *nf,
 			err2 = filemap_check_wb_err(nf->nf_file->f_mapping,
 						    since);
 			if (err2 < 0)
-				commit_reset_write_verifier(nn, rqstp, err2);
+				nfsd_maybe_reset_write_verifier(nn, rqstp, err2);
 			err = nfserrno(err2);
 			break;
 		case -EINVAL:
 			err = nfserr_notsupp;
 			break;
 		default:
-			commit_reset_write_verifier(nn, rqstp, err2);
+			nfsd_maybe_reset_write_verifier(nn, rqstp, err2);
 			err = nfserrno(err2);
 		}
 	} else
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index 3aa4522ca0a4..18171ccc6d16 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -86,7 +86,12 @@ static inline bool nfsd_attrs_valid(struct nfsd_attrs *attrs)
 		attrs->na_pacl || attrs->na_dpacl);
 }
 
+struct nfsd_net;
+
 __be32		nfserrno (int errno);
+void		nfsd_maybe_reset_write_verifier(struct nfsd_net *nn,
+						struct svc_rqst *rqstp,
+						int err);
 __be32		nfsd_cross_mnt(struct svc_rqst *rqstp, struct dentry **dpp,
 			       struct svc_export **expp);
 __be32		nfsd_lookup(struct svc_rqst *, struct svc_fh *,
-- 
2.54.0

[PATCH v1 2/3] NFSD: Move NFSv4-specific CLONE logic into nfsd4_clone()

[Chuck Lever]

nfsd4_clone_file_range() lives in fs/nfsd/vfs.c but reaches into the
NFSv4 compound reply buffer: nfsd4_get_cstate() casts rq_resp to a
struct nfsd4_compoundres to recover the saved and current file
handles a tracepoint wants. That is the only reference to the NFSv4
XDR definitions left in vfs.c, and it puts knowledge of the compound
reply layout in the VFS layer.

Refactor nfsd4_clone_file_range() to remove NFSv4-specific
componentry from fs/nfsd/vfs.c.

Splitting nfsd_clone_file_range() and nfsd_clone_sync_range() lets
nfsd4_clone() distinguish a clone failure from a sync failure, which
it has to do because only the latter invalidates the write verifier.

Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfsd/nfs4proc.c | 27 +++++++++++--
 fs/nfsd/vfs.c      | 99 +++++++++++++++++++++++++---------------------
 fs/nfsd/vfs.h      |  9 +++--
 3 files changed, 84 insertions(+), 51 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 54593f4667f8..e487b5d5d247 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1523,16 +1523,37 @@ nfsd4_clone(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 {
 	struct nfsd4_clone *clone = &u->clone;
 	struct nfsd_file *src, *dst;
+	bool sync_failed = false;
+	errseq_t since;
 	__be32 status;
+	int host_err;
 
 	status = nfsd4_verify_copy(rqstp, cstate, &clone->cl_src_stateid, &src,
 				   &clone->cl_dst_stateid, &dst);
 	if (status)
 		goto out;
 
-	status = nfsd4_clone_file_range(rqstp, src, clone->cl_src_pos,
-			dst, clone->cl_dst_pos, clone->cl_count,
-			EX_ISSYNC(cstate->current_fh.fh_export));
+	host_err = nfsd_clone_file_range(src->nf_file, clone->cl_src_pos,
+					 dst->nf_file, clone->cl_dst_pos,
+					 clone->cl_count, &since);
+	if (!host_err && EX_ISSYNC(cstate->current_fh.fh_export)) {
+		host_err = nfsd_clone_sync_range(src->nf_file, dst->nf_file,
+						 clone->cl_dst_pos,
+						 clone->cl_count, since);
+		sync_failed = host_err < 0;
+	}
+	if (host_err < 0) {
+		trace_nfsd_clone_file_range_err(rqstp, &cstate->save_fh,
+				clone->cl_src_pos, &cstate->current_fh,
+				clone->cl_dst_pos, clone->cl_count, host_err);
+		if (sync_failed) {
+			struct nfsd_net *nn = net_generic(dst->nf_net,
+							  nfsd_net_id);
+
+			nfsd_maybe_reset_write_verifier(nn, rqstp, host_err);
+		}
+	}
+	status = nfserrno(host_err);
 
 	if (!status && (READ_ONCE(dst->nf_file->f_mode) & FMODE_NOCMTIME) != 0)
 		nfsd_update_cmtime_attr(dst->nf_file, 0);
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index fad51a899062..27683f48360f 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -702,56 +702,67 @@ int nfsd4_is_junction(struct dentry *dentry)
 	return 1;
 }
 
-static struct nfsd4_compound_state *nfsd4_get_cstate(struct svc_rqst *rqstp)
+/**
+ * nfsd_clone_file_range - Clone a range of one file into another
+ * @src: file the range is cloned from
+ * @src_pos: offset in @src where the source range begins
+ * @dst: file the range is cloned into
+ * @dst_pos: offset in @dst where the destination range begins
+ * @count: length of the range, or zero to clone through end-of-file
+ * @since: receives @dst's writeback error state, sampled before the clone
+ *
+ * A caller that has to place the cloned data on durable storage passes
+ * @since to nfsd_clone_sync_range() once this call succeeds. Sampling
+ * happens here because a writeback error raised by the clone's own
+ * dirty pages has to fall inside the sampled interval.
+ *
+ * Context: Process context.
+ * Return: zero on success, or a negative errno
+ */
+int nfsd_clone_file_range(struct file *src, u64 src_pos, struct file *dst,
+			  u64 dst_pos, u64 count, errseq_t *since)
 {
-	return &((struct nfsd4_compoundres *)rqstp->rq_resp)->cstate;
+	loff_t cloned;
+
+	*since = READ_ONCE(dst->f_wb_err);
+	cloned = vfs_clone_file_range(src, src_pos, dst, dst_pos, count, 0);
+	if (cloned < 0)
+		return cloned;
+	if (count && cloned != count)
+		return -EINVAL;
+	return 0;
 }
 
-__be32 nfsd4_clone_file_range(struct svc_rqst *rqstp,
-		struct nfsd_file *nf_src, u64 src_pos,
-		struct nfsd_file *nf_dst, u64 dst_pos,
-		u64 count, bool sync)
+/**
+ * nfsd_clone_sync_range - Commit a cloned range to durable storage
+ * @src: file the range was cloned from, whose metadata is committed too
+ * @dst: file the range was cloned into
+ * @dst_pos: offset in @dst where the cloned range begins
+ * @count: length of the range, or zero if the clone ran to end-of-file
+ * @since: @dst's writeback error state as sampled by
+ *	   nfsd_clone_file_range()
+ *
+ * Context: Process context.
+ * Return: zero on success, or a negative errno
+ */
+int nfsd_clone_sync_range(struct file *src, struct file *dst, u64 dst_pos,
+			  u64 count, errseq_t since)
 {
-	struct file *src = nf_src->nf_file;
-	struct file *dst = nf_dst->nf_file;
-	errseq_t since;
-	loff_t cloned;
-	__be32 ret = 0;
+	loff_t dst_end = count ? dst_pos + count - 1 : LLONG_MAX;
+	int status;
 
-	since = READ_ONCE(dst->f_wb_err);
-	cloned = vfs_clone_file_range(src, src_pos, dst, dst_pos, count, 0);
-	if (cloned < 0) {
-		ret = nfserrno(cloned);
-		goto out_err;
+	status = vfs_fsync_range(dst, dst_pos, dst_end, 0);
+	if (!status)
+		status = filemap_check_wb_err(dst->f_mapping, since);
+	if (!status) {
+		/*
+		 * A reflink marks extents shared in the source inode too,
+		 * so the source's metadata has to reach durable storage
+		 * even though its data is untouched.
+		 */
+		status = commit_inode_metadata(file_inode(src));
 	}
-	if (count && cloned != count) {
-		ret = nfserrno(-EINVAL);
-		goto out_err;
-	}
-	if (sync) {
-		loff_t dst_end = count ? dst_pos + count - 1 : LLONG_MAX;
-		int status = vfs_fsync_range(dst, dst_pos, dst_end, 0);
-
-		if (!status)
-			status = filemap_check_wb_err(dst->f_mapping, since);
-		if (!status)
-			status = commit_inode_metadata(file_inode(src));
-		if (status < 0) {
-			struct nfsd_net *nn = net_generic(nf_dst->nf_net,
-							  nfsd_net_id);
-
-			trace_nfsd_clone_file_range_err(rqstp,
-					&nfsd4_get_cstate(rqstp)->save_fh,
-					src_pos,
-					&nfsd4_get_cstate(rqstp)->current_fh,
-					dst_pos,
-					count, status);
-			nfsd_maybe_reset_write_verifier(nn, rqstp, status);
-			ret = nfserrno(status);
-		}
-	}
-out_err:
-	return ret;
+	return status;
 }
 
 ssize_t nfsd_copy_file_range(struct file *src, u64 src_pos, struct file *dst,
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index 18171ccc6d16..f0cb184643f2 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -105,10 +105,11 @@ int nfsd_mountpoint(struct dentry *, struct svc_export *);
 #ifdef CONFIG_NFSD_V4
 __be32		nfsd4_vfs_fallocate(struct svc_rqst *, struct svc_fh *,
 				    struct file *, loff_t, loff_t, int);
-__be32		nfsd4_clone_file_range(struct svc_rqst *rqstp,
-				       struct nfsd_file *nf_src, u64 src_pos,
-				       struct nfsd_file *nf_dst, u64 dst_pos,
-				       u64 count, bool sync);
+int		nfsd_clone_file_range(struct file *src, u64 src_pos,
+				      struct file *dst, u64 dst_pos,
+				      u64 count, errseq_t *since);
+int		nfsd_clone_sync_range(struct file *src, struct file *dst,
+				      u64 dst_pos, u64 count, errseq_t since);
 #endif /* CONFIG_NFSD_V4 */
 __be32		nfsd_create_locked(struct svc_rqst *, struct svc_fh *,
 				struct nfsd_attrs *attrs, int type, dev_t rdev,
-- 
2.54.0

[PATCH v1 3/3] NFSD: Remove xdr-related headers from fs/nfsd/vfs.c

[Chuck Lever]

Clean up: Nothing in fs/nfsd/vfs.c references an NFSv3 XDR definition.
The preceding patch moved the last NFSv4 reference, a struct
nfsd4_compoundres dereference that recovered a pair of file handles for
a tracepoint, into fs/nfsd/nfs4proc.c. Remove both includes so that the
VFS layer no longer names on-the-wire types.

Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfsd/vfs.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 27683f48360f..f9131827d391 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -34,12 +34,9 @@
 #include <linux/sunrpc/xdr.h>
 #include <linux/fileattr.h>
 
-#include "xdr3.h"
-
 #ifdef CONFIG_NFSD_V4
 #include "acl.h"
 #include "idmap.h"
-#include "xdr4.h"
 #endif /* CONFIG_NFSD_V4 */
 
 #include "nfsd.h"
-- 
2.54.0