linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/15] nfsd_dispatch() clean up
@ 2020-10-01 22:58 Chuck Lever
  2020-10-01 22:58 ` [PATCH v3 01/15] nfsd: rq_lease_breaker cleanup Chuck Lever
                   ` (15 more replies)
  0 siblings, 16 replies; 19+ messages in thread
From: Chuck Lever @ 2020-10-01 22:58 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

Hi Bruce-

Here's the latest version of the nfsd_dispatch clean up series,
building on the "non-controversial" patches I posted last week.

The purpose of this series is three-fold:

o Prepare to add NFS procedure tracepoints
o Prepare to eventually deprecate NFSv2
o Minor optimizations of the dispatcher hot path


Changes since v2:
- Fixed crasher caused by invoking NFSv2 ROOT or WRITECACHE
- Hoisted encoding of NFS status code into XDR Reply encoders
- Numerous bug fixes, clean ups, and patch re-ordering

Changes since v1:
- Pulled in latest version of rq_lease_breaker cleanup
- Added patches to make NFSv2 error encoding similar to NFSv3
- Clarified nfsd_dispatch's new documenting comment
- Renamed a variable

---

Chuck Lever (14):
      NFSD: Add missing NFSv2 .pc_func methods
      lockd: Replace PROC() macro with open code
      NFSACL: Replace PROC() macro with open code
      NFSD: Encoder and decoder functions are always present
      NFSD: Clean up switch statement in nfsd_dispatch()
      NFSD: Clean up stale comments in nfsd_dispatch()
      NFSD: Clean up nfsd_dispatch() variables
      NFSD: Refactor nfsd_dispatch() error paths
      NFSD: Remove vestigial typedefs
      NFSD: Fix .pc_release method for NFSv2
      NFSD: Call NFSv2 encoders on error returns
      NFSD: Remove the RETURN_STATUS() macro
      NFSD: Map nfserr_wrongsec outside of nfsd_dispatch
      NFSD: Hoist status code encoding into XDR encoder functions

J. Bruce Fields (1):
      nfsd: rq_lease_breaker cleanup


 fs/lockd/svc4proc.c         | 248 ++++++++++++++++++++++++-------
 fs/lockd/svcproc.c          | 250 ++++++++++++++++++++++++-------
 fs/nfsd/export.c            |   2 +-
 fs/nfsd/nfs2acl.c           | 160 +++++++++++++-------
 fs/nfsd/nfs3acl.c           |  88 ++++++-----
 fs/nfsd/nfs3proc.c          | 238 +++++++++++++++---------------
 fs/nfsd/nfs3xdr.c           |  25 +++-
 fs/nfsd/nfs4proc.c          |   6 +-
 fs/nfsd/nfs4xdr.c           |  11 +-
 fs/nfsd/nfsproc.c           | 283 ++++++++++++++++++++----------------
 fs/nfsd/nfssvc.c            | 121 ++++++++-------
 fs/nfsd/nfsxdr.c            |  52 ++++++-
 fs/nfsd/xdr.h               |  16 +-
 fs/nfsd/xdr3.h              |   1 +
 fs/nfsd/xdr4.h              |   1 +
 include/uapi/linux/nfsacl.h |   2 +
 16 files changed, 984 insertions(+), 520 deletions(-)

--
Chuck Lever


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

* [PATCH v3 01/15] nfsd: rq_lease_breaker cleanup
  2020-10-01 22:58 [PATCH v3 00/15] nfsd_dispatch() clean up Chuck Lever
@ 2020-10-01 22:58 ` Chuck Lever
  2020-10-01 22:58 ` [PATCH v3 02/15] NFSD: Add missing NFSv2 .pc_func methods Chuck Lever
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Chuck Lever @ 2020-10-01 22:58 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

From: J. Bruce Fields <bfields@redhat.com>

Since only the v4 code cares about it, maybe it's better to leave
rq_lease_breaker out of the common dispatch code?

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c |    3 ++-
 fs/nfsd/nfssvc.c    |    1 -
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 767dc1b27e91..603c0a227a64 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4601,7 +4601,8 @@ static bool nfsd_breaker_owns_lease(struct file_lock *fl)
 	if (!i_am_nfsd())
 		return NULL;
 	rqst = kthread_data(current);
-	if (!rqst->rq_lease_breaker)
+	/* Note rq_prog == NFS_ACL_PROGRAM is also possible: */
+	if (rqst->rq_prog != NFS_PROGRAM || rqst->rq_vers < 4)
 		return NULL;
 	clp = *(rqst->rq_lease_breaker);
 	return dl->dl_stid.sc_client == clp;
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index f7f6473578af..f6bc94cab9da 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -1016,7 +1016,6 @@ nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)
 		*statp = rpc_garbage_args;
 		return 1;
 	}
-	rqstp->rq_lease_breaker = NULL;
 	/*
 	 * Give the xdr decoder a chance to change this if it wants
 	 * (necessary in the NFSv4.0 compound case)



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

* [PATCH v3 02/15] NFSD: Add missing NFSv2 .pc_func methods
  2020-10-01 22:58 [PATCH v3 00/15] nfsd_dispatch() clean up Chuck Lever
  2020-10-01 22:58 ` [PATCH v3 01/15] nfsd: rq_lease_breaker cleanup Chuck Lever
@ 2020-10-01 22:58 ` Chuck Lever
  2020-10-01 22:59 ` [PATCH v3 03/15] lockd: Replace PROC() macro with open code Chuck Lever
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Chuck Lever @ 2020-10-01 22:58 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

There's no protection in nfsd_dispatch() against a NULL .pc_func
helpers. A malicious NFS client can trigger a crash by invoking the
unused/unsupported NFSv2 ROOT or WRITECACHE procedures.

The current NFSD dispatcher does not support returning a void reply
to a non-NULL procedure, so the reply to both of these is wrong, for
the moment.

Cc: <stable@vger.kernel.org>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfsproc.c |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c
index 6e0b066480c5..6d1b3af40a4f 100644
--- a/fs/nfsd/nfsproc.c
+++ b/fs/nfsd/nfsproc.c
@@ -118,6 +118,13 @@ nfsd_proc_setattr(struct svc_rqst *rqstp)
 	return nfsd_return_attrs(nfserr, resp);
 }
 
+/* Obsolete, replaced by MNTPROC_MNT. */
+static __be32
+nfsd_proc_root(struct svc_rqst *rqstp)
+{
+	return nfs_ok;
+}
+
 /*
  * Look up a path name component
  * Note: the dentry in the resp->fh may be negative if the file
@@ -203,6 +210,13 @@ nfsd_proc_read(struct svc_rqst *rqstp)
 	return fh_getattr(&resp->fh, &resp->stat);
 }
 
+/* Reserved */
+static __be32
+nfsd_proc_writecache(struct svc_rqst *rqstp)
+{
+	return nfs_ok;
+}
+
 /*
  * Write data to a file
  * N.B. After this call resp->fh needs an fh_put
@@ -617,6 +631,7 @@ static const struct svc_procedure nfsd_procedures2[18] = {
 		.pc_xdrressize = ST+AT,
 	},
 	[NFSPROC_ROOT] = {
+		.pc_func = nfsd_proc_root,
 		.pc_decode = nfssvc_decode_void,
 		.pc_encode = nfssvc_encode_void,
 		.pc_argsize = sizeof(struct nfsd_void),
@@ -654,6 +669,7 @@ static const struct svc_procedure nfsd_procedures2[18] = {
 		.pc_xdrressize = ST+AT+1+NFSSVC_MAXBLKSIZE_V2/4,
 	},
 	[NFSPROC_WRITECACHE] = {
+		.pc_func = nfsd_proc_writecache,
 		.pc_decode = nfssvc_decode_void,
 		.pc_encode = nfssvc_encode_void,
 		.pc_argsize = sizeof(struct nfsd_void),



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

* [PATCH v3 03/15] lockd: Replace PROC() macro with open code
  2020-10-01 22:58 [PATCH v3 00/15] nfsd_dispatch() clean up Chuck Lever
  2020-10-01 22:58 ` [PATCH v3 01/15] nfsd: rq_lease_breaker cleanup Chuck Lever
  2020-10-01 22:58 ` [PATCH v3 02/15] NFSD: Add missing NFSv2 .pc_func methods Chuck Lever
@ 2020-10-01 22:59 ` Chuck Lever
  2020-10-01 22:59 ` [PATCH v3 04/15] NFSACL: " Chuck Lever
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Chuck Lever @ 2020-10-01 22:59 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

Clean up: Follow-up on ten-year-old commit b9081d90f5b9 ("NFS: kill
off complicated macro 'PROC'") by performing the same conversion in
the lockd code. To reduce the chance of error, I copied the original
C preprocessor output and then made some minor edits.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/lockd/svc4proc.c |  248 +++++++++++++++++++++++++++++++++++++++++----------
 fs/lockd/svcproc.c  |  250 +++++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 398 insertions(+), 100 deletions(-)

diff --git a/fs/lockd/svc4proc.c b/fs/lockd/svc4proc.c
index e4d3f783e06a..fa41dda39925 100644
--- a/fs/lockd/svc4proc.c
+++ b/fs/lockd/svc4proc.c
@@ -486,65 +486,215 @@ nlm4svc_proc_granted_res(struct svc_rqst *rqstp)
         return rpc_success;
 }
 
+static __be32
+nlm4svc_proc_unused(struct svc_rqst *rqstp)
+{
+	return rpc_proc_unavail;
+}
+
 
 /*
  * NLM Server procedures.
  */
 
-#define nlm4svc_encode_norep	nlm4svc_encode_void
-#define nlm4svc_decode_norep	nlm4svc_decode_void
-#define nlm4svc_decode_testres	nlm4svc_decode_void
-#define nlm4svc_decode_lockres	nlm4svc_decode_void
-#define nlm4svc_decode_unlockres	nlm4svc_decode_void
-#define nlm4svc_decode_cancelres	nlm4svc_decode_void
-#define nlm4svc_decode_grantedres	nlm4svc_decode_void
-
-#define nlm4svc_proc_none	nlm4svc_proc_null
-#define nlm4svc_proc_test_res	nlm4svc_proc_null
-#define nlm4svc_proc_lock_res	nlm4svc_proc_null
-#define nlm4svc_proc_cancel_res	nlm4svc_proc_null
-#define nlm4svc_proc_unlock_res	nlm4svc_proc_null
-
 struct nlm_void			{ int dummy; };
 
-#define PROC(name, xargt, xrest, argt, rest, respsize)	\
- { .pc_func	= nlm4svc_proc_##name,	\
-   .pc_decode	= nlm4svc_decode_##xargt,	\
-   .pc_encode	= nlm4svc_encode_##xrest,	\
-   .pc_release	= NULL,					\
-   .pc_argsize	= sizeof(struct nlm_##argt),		\
-   .pc_ressize	= sizeof(struct nlm_##rest),		\
-   .pc_xdrressize = respsize,				\
- }
 #define	Ck	(1+XDR_QUADLEN(NLM_MAXCOOKIELEN))	/* cookie */
 #define	No	(1+1024/4)				/* netobj */
 #define	St	1					/* status */
 #define	Rg	4					/* range (offset + length) */
-const struct svc_procedure nlmsvc_procedures4[] = {
-  PROC(null,		void,		void,		void,	void, 1),
-  PROC(test,		testargs,	testres,	args,	res, Ck+St+2+No+Rg),
-  PROC(lock,		lockargs,	res,		args,	res, Ck+St),
-  PROC(cancel,		cancargs,	res,		args,	res, Ck+St),
-  PROC(unlock,		unlockargs,	res,		args,	res, Ck+St),
-  PROC(granted,		testargs,	res,		args,	res, Ck+St),
-  PROC(test_msg,	testargs,	norep,		args,	void, 1),
-  PROC(lock_msg,	lockargs,	norep,		args,	void, 1),
-  PROC(cancel_msg,	cancargs,	norep,		args,	void, 1),
-  PROC(unlock_msg,	unlockargs,	norep,		args,	void, 1),
-  PROC(granted_msg,	testargs,	norep,		args,	void, 1),
-  PROC(test_res,	testres,	norep,		res,	void, 1),
-  PROC(lock_res,	lockres,	norep,		res,	void, 1),
-  PROC(cancel_res,	cancelres,	norep,		res,	void, 1),
-  PROC(unlock_res,	unlockres,	norep,		res,	void, 1),
-  PROC(granted_res,	res,		norep,		res,	void, 1),
-  /* statd callback */
-  PROC(sm_notify,	reboot,		void,		reboot,	void, 1),
-  PROC(none,		void,		void,		void,	void, 0),
-  PROC(none,		void,		void,		void,	void, 0),
-  PROC(none,		void,		void,		void,	void, 0),
-  PROC(share,		shareargs,	shareres,	args,	res, Ck+St+1),
-  PROC(unshare,		shareargs,	shareres,	args,	res, Ck+St+1),
-  PROC(nm_lock,		lockargs,	res,		args,	res, Ck+St),
-  PROC(free_all,	notify,		void,		args,	void, 1),
 
+const struct svc_procedure nlmsvc_procedures4[24] = {
+	[NLMPROC_NULL] = {
+		.pc_func = nlm4svc_proc_null,
+		.pc_decode = nlm4svc_decode_void,
+		.pc_encode = nlm4svc_encode_void,
+		.pc_argsize = sizeof(struct nlm_void),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
+	[NLMPROC_TEST] = {
+		.pc_func = nlm4svc_proc_test,
+		.pc_decode = nlm4svc_decode_testargs,
+		.pc_encode = nlm4svc_encode_testres,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_res),
+		.pc_xdrressize = Ck+St+2+No+Rg,
+	},
+	[NLMPROC_LOCK] = {
+		.pc_func = nlm4svc_proc_lock,
+		.pc_decode = nlm4svc_decode_lockargs,
+		.pc_encode = nlm4svc_encode_res,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_res),
+		.pc_xdrressize = Ck+St,
+	},
+	[NLMPROC_CANCEL] = {
+		.pc_func = nlm4svc_proc_cancel,
+		.pc_decode = nlm4svc_decode_cancargs,
+		.pc_encode = nlm4svc_encode_res,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_res),
+		.pc_xdrressize = Ck+St,
+	},
+	[NLMPROC_UNLOCK] = {
+		.pc_func = nlm4svc_proc_unlock,
+		.pc_decode = nlm4svc_decode_unlockargs,
+		.pc_encode = nlm4svc_encode_res,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_res),
+		.pc_xdrressize = Ck+St,
+	},
+	[NLMPROC_GRANTED] = {
+		.pc_func = nlm4svc_proc_granted,
+		.pc_decode = nlm4svc_decode_testargs,
+		.pc_encode = nlm4svc_encode_res,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_res),
+		.pc_xdrressize = Ck+St,
+	},
+	[NLMPROC_TEST_MSG] = {
+		.pc_func = nlm4svc_proc_test_msg,
+		.pc_decode = nlm4svc_decode_testargs,
+		.pc_encode = nlm4svc_encode_void,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
+	[NLMPROC_LOCK_MSG] = {
+		.pc_func = nlm4svc_proc_lock_msg,
+		.pc_decode = nlm4svc_decode_lockargs,
+		.pc_encode = nlm4svc_encode_void,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
+	[NLMPROC_CANCEL_MSG] = {
+		.pc_func = nlm4svc_proc_cancel_msg,
+		.pc_decode = nlm4svc_decode_cancargs,
+		.pc_encode = nlm4svc_encode_void,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
+	[NLMPROC_UNLOCK_MSG] = {
+		.pc_func = nlm4svc_proc_unlock_msg,
+		.pc_decode = nlm4svc_decode_unlockargs,
+		.pc_encode = nlm4svc_encode_void,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
+	[NLMPROC_GRANTED_MSG] = {
+		.pc_func = nlm4svc_proc_granted_msg,
+		.pc_decode = nlm4svc_decode_testargs,
+		.pc_encode = nlm4svc_encode_void,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
+	[NLMPROC_TEST_RES] = {
+		.pc_func = nlm4svc_proc_null,
+		.pc_decode = nlm4svc_decode_void,
+		.pc_encode = nlm4svc_encode_void,
+		.pc_argsize = sizeof(struct nlm_res),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
+	[NLMPROC_LOCK_RES] = {
+		.pc_func = nlm4svc_proc_null,
+		.pc_decode = nlm4svc_decode_void,
+		.pc_encode = nlm4svc_encode_void,
+		.pc_argsize = sizeof(struct nlm_res),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
+	[NLMPROC_CANCEL_RES] = {
+		.pc_func = nlm4svc_proc_null,
+		.pc_decode = nlm4svc_decode_void,
+		.pc_encode = nlm4svc_encode_void,
+		.pc_argsize = sizeof(struct nlm_res),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
+	[NLMPROC_UNLOCK_RES] = {
+		.pc_func = nlm4svc_proc_null,
+		.pc_decode = nlm4svc_decode_void,
+		.pc_encode = nlm4svc_encode_void,
+		.pc_argsize = sizeof(struct nlm_res),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
+	[NLMPROC_GRANTED_RES] = {
+		.pc_func = nlm4svc_proc_granted_res,
+		.pc_decode = nlm4svc_decode_res,
+		.pc_encode = nlm4svc_encode_void,
+		.pc_argsize = sizeof(struct nlm_res),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
+	[NLMPROC_NSM_NOTIFY] = {
+		.pc_func = nlm4svc_proc_sm_notify,
+		.pc_decode = nlm4svc_decode_reboot,
+		.pc_encode = nlm4svc_encode_void,
+		.pc_argsize = sizeof(struct nlm_reboot),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
+	[17] = {
+		.pc_func = nlm4svc_proc_unused,
+		.pc_decode = nlm4svc_decode_void,
+		.pc_encode = nlm4svc_encode_void,
+		.pc_argsize = sizeof(struct nlm_void),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = 0,
+	},
+	[18] = {
+		.pc_func = nlm4svc_proc_unused,
+		.pc_decode = nlm4svc_decode_void,
+		.pc_encode = nlm4svc_encode_void,
+		.pc_argsize = sizeof(struct nlm_void),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = 0,
+	},
+	[19] = {
+		.pc_func = nlm4svc_proc_unused,
+		.pc_decode = nlm4svc_decode_void,
+		.pc_encode = nlm4svc_encode_void,
+		.pc_argsize = sizeof(struct nlm_void),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = 0,
+	},
+	[NLMPROC_SHARE] = {
+		.pc_func = nlm4svc_proc_share,
+		.pc_decode = nlm4svc_decode_shareargs,
+		.pc_encode = nlm4svc_encode_shareres,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_res),
+		.pc_xdrressize = Ck+St+1,
+	},
+	[NLMPROC_UNSHARE] = {
+		.pc_func = nlm4svc_proc_unshare,
+		.pc_decode = nlm4svc_decode_shareargs,
+		.pc_encode = nlm4svc_encode_shareres,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_res),
+		.pc_xdrressize = Ck+St+1,
+	},
+	[NLMPROC_NM_LOCK] = {
+		.pc_func = nlm4svc_proc_nm_lock,
+		.pc_decode = nlm4svc_decode_lockargs,
+		.pc_encode = nlm4svc_encode_res,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_res),
+		.pc_xdrressize = Ck+St,
+	},
+	[NLMPROC_FREE_ALL] = {
+		.pc_func = nlm4svc_proc_free_all,
+		.pc_decode = nlm4svc_decode_notify,
+		.pc_encode = nlm4svc_encode_void,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
 };
diff --git a/fs/lockd/svcproc.c b/fs/lockd/svcproc.c
index d0bb7a6bf005..50855f2c1f4b 100644
--- a/fs/lockd/svcproc.c
+++ b/fs/lockd/svcproc.c
@@ -529,66 +529,214 @@ nlmsvc_proc_granted_res(struct svc_rqst *rqstp)
 	return rpc_success;
 }
 
+static __be32
+nlmsvc_proc_unused(struct svc_rqst *rqstp)
+{
+	return rpc_proc_unavail;
+}
+
 /*
  * NLM Server procedures.
  */
 
-#define nlmsvc_encode_norep	nlmsvc_encode_void
-#define nlmsvc_decode_norep	nlmsvc_decode_void
-#define nlmsvc_decode_testres	nlmsvc_decode_void
-#define nlmsvc_decode_lockres	nlmsvc_decode_void
-#define nlmsvc_decode_unlockres	nlmsvc_decode_void
-#define nlmsvc_decode_cancelres	nlmsvc_decode_void
-#define nlmsvc_decode_grantedres	nlmsvc_decode_void
-
-#define nlmsvc_proc_none	nlmsvc_proc_null
-#define nlmsvc_proc_test_res	nlmsvc_proc_null
-#define nlmsvc_proc_lock_res	nlmsvc_proc_null
-#define nlmsvc_proc_cancel_res	nlmsvc_proc_null
-#define nlmsvc_proc_unlock_res	nlmsvc_proc_null
-
 struct nlm_void			{ int dummy; };
 
-#define PROC(name, xargt, xrest, argt, rest, respsize)	\
- { .pc_func	= nlmsvc_proc_##name,			\
-   .pc_decode	= nlmsvc_decode_##xargt,		\
-   .pc_encode	= nlmsvc_encode_##xrest,		\
-   .pc_release	= NULL,					\
-   .pc_argsize	= sizeof(struct nlm_##argt),		\
-   .pc_ressize	= sizeof(struct nlm_##rest),		\
-   .pc_xdrressize = respsize,				\
- }
-
 #define	Ck	(1+XDR_QUADLEN(NLM_MAXCOOKIELEN))	/* cookie */
 #define	St	1				/* status */
 #define	No	(1+1024/4)			/* Net Obj */
 #define	Rg	2				/* range - offset + size */
 
-const struct svc_procedure nlmsvc_procedures[] = {
-  PROC(null,		void,		void,		void,	void, 1),
-  PROC(test,		testargs,	testres,	args,	res, Ck+St+2+No+Rg),
-  PROC(lock,		lockargs,	res,		args,	res, Ck+St),
-  PROC(cancel,		cancargs,	res,		args,	res, Ck+St),
-  PROC(unlock,		unlockargs,	res,		args,	res, Ck+St),
-  PROC(granted,		testargs,	res,		args,	res, Ck+St),
-  PROC(test_msg,	testargs,	norep,		args,	void, 1),
-  PROC(lock_msg,	lockargs,	norep,		args,	void, 1),
-  PROC(cancel_msg,	cancargs,	norep,		args,	void, 1),
-  PROC(unlock_msg,	unlockargs,	norep,		args,	void, 1),
-  PROC(granted_msg,	testargs,	norep,		args,	void, 1),
-  PROC(test_res,	testres,	norep,		res,	void, 1),
-  PROC(lock_res,	lockres,	norep,		res,	void, 1),
-  PROC(cancel_res,	cancelres,	norep,		res,	void, 1),
-  PROC(unlock_res,	unlockres,	norep,		res,	void, 1),
-  PROC(granted_res,	res,		norep,		res,	void, 1),
-  /* statd callback */
-  PROC(sm_notify,	reboot,		void,		reboot,	void, 1),
-  PROC(none,		void,		void,		void,	void, 1),
-  PROC(none,		void,		void,		void,	void, 1),
-  PROC(none,		void,		void,		void,	void, 1),
-  PROC(share,		shareargs,	shareres,	args,	res, Ck+St+1),
-  PROC(unshare,		shareargs,	shareres,	args,	res, Ck+St+1),
-  PROC(nm_lock,		lockargs,	res,		args,	res, Ck+St),
-  PROC(free_all,	notify,		void,		args,	void, 0),
-
+const struct svc_procedure nlmsvc_procedures[24] = {
+	[NLMPROC_NULL] = {
+		.pc_func = nlmsvc_proc_null,
+		.pc_decode = nlmsvc_decode_void,
+		.pc_encode = nlmsvc_encode_void,
+		.pc_argsize = sizeof(struct nlm_void),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
+	[NLMPROC_TEST] = {
+		.pc_func = nlmsvc_proc_test,
+		.pc_decode = nlmsvc_decode_testargs,
+		.pc_encode = nlmsvc_encode_testres,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_res),
+		.pc_xdrressize = Ck+St+2+No+Rg,
+	},
+	[NLMPROC_LOCK] = {
+		.pc_func = nlmsvc_proc_lock,
+		.pc_decode = nlmsvc_decode_lockargs,
+		.pc_encode = nlmsvc_encode_res,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_res),
+		.pc_xdrressize = Ck+St,
+	},
+	[NLMPROC_CANCEL] = {
+		.pc_func = nlmsvc_proc_cancel,
+		.pc_decode = nlmsvc_decode_cancargs,
+		.pc_encode = nlmsvc_encode_res,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_res),
+		.pc_xdrressize = Ck+St,
+	},
+	[NLMPROC_UNLOCK] = {
+		.pc_func = nlmsvc_proc_unlock,
+		.pc_decode = nlmsvc_decode_unlockargs,
+		.pc_encode = nlmsvc_encode_res,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_res),
+		.pc_xdrressize = Ck+St,
+	},
+	[NLMPROC_GRANTED] = {
+		.pc_func = nlmsvc_proc_granted,
+		.pc_decode = nlmsvc_decode_testargs,
+		.pc_encode = nlmsvc_encode_res,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_res),
+		.pc_xdrressize = Ck+St,
+	},
+	[NLMPROC_TEST_MSG] = {
+		.pc_func = nlmsvc_proc_test_msg,
+		.pc_decode = nlmsvc_decode_testargs,
+		.pc_encode = nlmsvc_encode_void,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
+	[NLMPROC_LOCK_MSG] = {
+		.pc_func = nlmsvc_proc_lock_msg,
+		.pc_decode = nlmsvc_decode_lockargs,
+		.pc_encode = nlmsvc_encode_void,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
+	[NLMPROC_CANCEL_MSG] = {
+		.pc_func = nlmsvc_proc_cancel_msg,
+		.pc_decode = nlmsvc_decode_cancargs,
+		.pc_encode = nlmsvc_encode_void,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
+	[NLMPROC_UNLOCK_MSG] = {
+		.pc_func = nlmsvc_proc_unlock_msg,
+		.pc_decode = nlmsvc_decode_unlockargs,
+		.pc_encode = nlmsvc_encode_void,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
+	[NLMPROC_GRANTED_MSG] = {
+		.pc_func = nlmsvc_proc_granted_msg,
+		.pc_decode = nlmsvc_decode_testargs,
+		.pc_encode = nlmsvc_encode_void,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
+	[NLMPROC_TEST_RES] = {
+		.pc_func = nlmsvc_proc_null,
+		.pc_decode = nlmsvc_decode_void,
+		.pc_encode = nlmsvc_encode_void,
+		.pc_argsize = sizeof(struct nlm_res),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
+	[NLMPROC_LOCK_RES] = {
+		.pc_func = nlmsvc_proc_null,
+		.pc_decode = nlmsvc_decode_void,
+		.pc_encode = nlmsvc_encode_void,
+		.pc_argsize = sizeof(struct nlm_res),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
+	[NLMPROC_CANCEL_RES] = {
+		.pc_func = nlmsvc_proc_null,
+		.pc_decode = nlmsvc_decode_void,
+		.pc_encode = nlmsvc_encode_void,
+		.pc_argsize = sizeof(struct nlm_res),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
+	[NLMPROC_UNLOCK_RES] = {
+		.pc_func = nlmsvc_proc_null,
+		.pc_decode = nlmsvc_decode_void,
+		.pc_encode = nlmsvc_encode_void,
+		.pc_argsize = sizeof(struct nlm_res),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
+	[NLMPROC_GRANTED_RES] = {
+		.pc_func = nlmsvc_proc_granted_res,
+		.pc_decode = nlmsvc_decode_res,
+		.pc_encode = nlmsvc_encode_void,
+		.pc_argsize = sizeof(struct nlm_res),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
+	[NLMPROC_NSM_NOTIFY] = {
+		.pc_func = nlmsvc_proc_sm_notify,
+		.pc_decode = nlmsvc_decode_reboot,
+		.pc_encode = nlmsvc_encode_void,
+		.pc_argsize = sizeof(struct nlm_reboot),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
+	[17] = {
+		.pc_func = nlmsvc_proc_unused,
+		.pc_decode = nlmsvc_decode_void,
+		.pc_encode = nlmsvc_encode_void,
+		.pc_argsize = sizeof(struct nlm_void),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
+	[18] = {
+		.pc_func = nlmsvc_proc_unused,
+		.pc_decode = nlmsvc_decode_void,
+		.pc_encode = nlmsvc_encode_void,
+		.pc_argsize = sizeof(struct nlm_void),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
+	[19] = {
+		.pc_func = nlmsvc_proc_unused,
+		.pc_decode = nlmsvc_decode_void,
+		.pc_encode = nlmsvc_encode_void,
+		.pc_argsize = sizeof(struct nlm_void),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = St,
+	},
+	[NLMPROC_SHARE] = {
+		.pc_func = nlmsvc_proc_share,
+		.pc_decode = nlmsvc_decode_shareargs,
+		.pc_encode = nlmsvc_encode_shareres,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_res),
+		.pc_xdrressize = Ck+St+1,
+	},
+	[NLMPROC_UNSHARE] = {
+		.pc_func = nlmsvc_proc_unshare,
+		.pc_decode = nlmsvc_decode_shareargs,
+		.pc_encode = nlmsvc_encode_shareres,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_res),
+		.pc_xdrressize = Ck+St+1,
+	},
+	[NLMPROC_NM_LOCK] = {
+		.pc_func = nlmsvc_proc_nm_lock,
+		.pc_decode = nlmsvc_decode_lockargs,
+		.pc_encode = nlmsvc_encode_res,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_res),
+		.pc_xdrressize = Ck+St,
+	},
+	[NLMPROC_FREE_ALL] = {
+		.pc_func = nlmsvc_proc_free_all,
+		.pc_decode = nlmsvc_decode_notify,
+		.pc_encode = nlmsvc_encode_void,
+		.pc_argsize = sizeof(struct nlm_args),
+		.pc_ressize = sizeof(struct nlm_void),
+		.pc_xdrressize = 0,
+	},
 };



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

* [PATCH v3 04/15] NFSACL: Replace PROC() macro with open code
  2020-10-01 22:58 [PATCH v3 00/15] nfsd_dispatch() clean up Chuck Lever
                   ` (2 preceding siblings ...)
  2020-10-01 22:59 ` [PATCH v3 03/15] lockd: Replace PROC() macro with open code Chuck Lever
@ 2020-10-01 22:59 ` Chuck Lever
  2020-10-01 22:59 ` [PATCH v3 05/15] NFSD: Encoder and decoder functions are always present Chuck Lever
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Chuck Lever @ 2020-10-01 22:59 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

Clean up: Follow-up on ten-year-old commit b9081d90f5b9 ("NFS: kill
off complicated macro 'PROC'") by performing the same conversion in
the NFSACL code. To reduce the chance of error, I copied the original
C preprocessor output and then made some minor edits.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfs2acl.c           |   72 +++++++++++++++++++++++++++++--------------
 fs/nfsd/nfs3acl.c           |   49 +++++++++++++++++------------
 include/uapi/linux/nfsacl.h |    2 +
 3 files changed, 80 insertions(+), 43 deletions(-)

diff --git a/fs/nfsd/nfs2acl.c b/fs/nfsd/nfs2acl.c
index cbab1d2d8a75..8d20e0d74417 100644
--- a/fs/nfsd/nfs2acl.c
+++ b/fs/nfsd/nfs2acl.c
@@ -347,36 +347,62 @@ static void nfsaclsvc_release_access(struct svc_rqst *rqstp)
 	fh_put(&resp->fh);
 }
 
-#define nfsaclsvc_decode_voidargs	NULL
-#define nfsaclsvc_release_void		NULL
-#define nfsd3_fhandleargs	nfsd_fhandle
-#define nfsd3_attrstatres	nfsd_attrstat
-#define nfsd3_voidres		nfsd3_voidargs
 struct nfsd3_voidargs { int dummy; };
 
-#define PROC(name, argt, rest, relt, cache, respsize)			\
-{									\
-	.pc_func	= nfsacld_proc_##name,				\
-	.pc_decode	= nfsaclsvc_decode_##argt##args,		\
-	.pc_encode	= nfsaclsvc_encode_##rest##res,			\
-	.pc_release	= nfsaclsvc_release_##relt,	\
-	.pc_argsize	= sizeof(struct nfsd3_##argt##args),		\
-	.pc_ressize	= sizeof(struct nfsd3_##rest##res),		\
-	.pc_cachetype	= cache,					\
-	.pc_xdrressize	= respsize,					\
-}
-
 #define ST 1		/* status*/
 #define AT 21		/* attributes */
 #define pAT (1+AT)	/* post attributes - conditional */
 #define ACL (1+NFS_ACL_MAX_ENTRIES*3)  /* Access Control List */
 
-static const struct svc_procedure nfsd_acl_procedures2[] = {
-  PROC(null,	void,		void,		void,	  RC_NOCACHE, ST),
-  PROC(getacl,	getacl,		getacl,		getacl,	  RC_NOCACHE, ST+1+2*(1+ACL)),
-  PROC(setacl,	setacl,		attrstat,	attrstat, RC_NOCACHE, ST+AT),
-  PROC(getattr, fhandle,	attrstat,	attrstat, RC_NOCACHE, ST+AT),
-  PROC(access,	access,		access,		access,   RC_NOCACHE, ST+AT+1),
+static const struct svc_procedure nfsd_acl_procedures2[5] = {
+	[ACLPROC2_NULL] = {
+		.pc_func = nfsacld_proc_null,
+		.pc_encode = nfsaclsvc_encode_voidres,
+		.pc_argsize = sizeof(struct nfsd3_voidargs),
+		.pc_ressize = sizeof(struct nfsd3_voidargs),
+		.pc_cachetype = RC_NOCACHE,
+		.pc_xdrressize = ST,
+	},
+	[ACLPROC2_GETACL] = {
+		.pc_func = nfsacld_proc_getacl,
+		.pc_decode = nfsaclsvc_decode_getaclargs,
+		.pc_encode = nfsaclsvc_encode_getaclres,
+		.pc_release = nfsaclsvc_release_getacl,
+		.pc_argsize = sizeof(struct nfsd3_getaclargs),
+		.pc_ressize = sizeof(struct nfsd3_getaclres),
+		.pc_cachetype = RC_NOCACHE,
+		.pc_xdrressize = ST+1+2*(1+ACL),
+	},
+	[ACLPROC2_SETACL] = {
+		.pc_func = nfsacld_proc_setacl,
+		.pc_decode = nfsaclsvc_decode_setaclargs,
+		.pc_encode = nfsaclsvc_encode_attrstatres,
+		.pc_release = nfsaclsvc_release_attrstat,
+		.pc_argsize = sizeof(struct nfsd3_setaclargs),
+		.pc_ressize = sizeof(struct nfsd_attrstat),
+		.pc_cachetype = RC_NOCACHE,
+		.pc_xdrressize = ST+AT,
+	},
+	[ACLPROC2_GETATTR] = {
+		.pc_func = nfsacld_proc_getattr,
+		.pc_decode = nfsaclsvc_decode_fhandleargs,
+		.pc_encode = nfsaclsvc_encode_attrstatres,
+		.pc_release = nfsaclsvc_release_attrstat,
+		.pc_argsize = sizeof(struct nfsd_fhandle),
+		.pc_ressize = sizeof(struct nfsd_attrstat),
+		.pc_cachetype = RC_NOCACHE,
+		.pc_xdrressize = ST+AT,
+	},
+	[ACLPROC2_ACCESS] = {
+		.pc_func = nfsacld_proc_access,
+		.pc_decode = nfsaclsvc_decode_accessargs,
+		.pc_encode = nfsaclsvc_encode_accessres,
+		.pc_release = nfsaclsvc_release_access,
+		.pc_argsize = sizeof(struct nfsd3_accessargs),
+		.pc_ressize = sizeof(struct nfsd3_accessres),
+		.pc_cachetype = RC_NOCACHE,
+		.pc_xdrressize = ST+AT+1,
+	},
 };
 
 static unsigned int nfsd_acl_count2[ARRAY_SIZE(nfsd_acl_procedures2)];
diff --git a/fs/nfsd/nfs3acl.c b/fs/nfsd/nfs3acl.c
index 13bca4a2f89d..292acb2e529c 100644
--- a/fs/nfsd/nfs3acl.c
+++ b/fs/nfsd/nfs3acl.c
@@ -235,33 +235,42 @@ static void nfs3svc_release_getacl(struct svc_rqst *rqstp)
 	posix_acl_release(resp->acl_default);
 }
 
-#define nfs3svc_decode_voidargs		NULL
-#define nfs3svc_release_void		NULL
-#define nfsd3_setaclres			nfsd3_attrstat
-#define nfsd3_voidres			nfsd3_voidargs
 struct nfsd3_voidargs { int dummy; };
 
-#define PROC(name, argt, rest, relt, cache, respsize)			\
-{									\
-	.pc_func	= nfsd3_proc_##name,				\
-	.pc_decode	= nfs3svc_decode_##argt##args,			\
-	.pc_encode	= nfs3svc_encode_##rest##res,			\
-	.pc_release	= nfs3svc_release_##relt,			\
-	.pc_argsize	= sizeof(struct nfsd3_##argt##args),		\
-	.pc_ressize	= sizeof(struct nfsd3_##rest##res),		\
-	.pc_cachetype	= cache,					\
-	.pc_xdrressize	= respsize,					\
-}
-
 #define ST 1		/* status*/
 #define AT 21		/* attributes */
 #define pAT (1+AT)	/* post attributes - conditional */
 #define ACL (1+NFS_ACL_MAX_ENTRIES*3)  /* Access Control List */
 
-static const struct svc_procedure nfsd_acl_procedures3[] = {
-  PROC(null,	void,		void,		void,	  RC_NOCACHE, ST),
-  PROC(getacl,	getacl,		getacl,		getacl,	  RC_NOCACHE, ST+1+2*(1+ACL)),
-  PROC(setacl,	setacl,		setacl,		fhandle,  RC_NOCACHE, ST+pAT),
+static const struct svc_procedure nfsd_acl_procedures3[3] = {
+	[ACLPROC3_NULL] = {
+		.pc_func = nfsd3_proc_null,
+		.pc_encode = nfs3svc_encode_voidres,
+		.pc_argsize = sizeof(struct nfsd3_voidargs),
+		.pc_ressize = sizeof(struct nfsd3_voidargs),
+		.pc_cachetype = RC_NOCACHE,
+		.pc_xdrressize = ST,
+	},
+	[ACLPROC3_GETACL] = {
+		.pc_func = nfsd3_proc_getacl,
+		.pc_decode = nfs3svc_decode_getaclargs,
+		.pc_encode = nfs3svc_encode_getaclres,
+		.pc_release = nfs3svc_release_getacl,
+		.pc_argsize = sizeof(struct nfsd3_getaclargs),
+		.pc_ressize = sizeof(struct nfsd3_getaclres),
+		.pc_cachetype = RC_NOCACHE,
+		.pc_xdrressize = ST+1+2*(1+ACL),
+	},
+	[ACLPROC3_SETACL] = {
+		.pc_func = nfsd3_proc_setacl,
+		.pc_decode = nfs3svc_decode_setaclargs,
+		.pc_encode = nfs3svc_encode_setaclres,
+		.pc_release = nfs3svc_release_fhandle,
+		.pc_argsize = sizeof(struct nfsd3_setaclargs),
+		.pc_ressize = sizeof(struct nfsd3_attrstat),
+		.pc_cachetype = RC_NOCACHE,
+		.pc_xdrressize = ST+pAT,
+	},
 };
 
 static unsigned int nfsd_acl_count3[ARRAY_SIZE(nfsd_acl_procedures3)];
diff --git a/include/uapi/linux/nfsacl.h b/include/uapi/linux/nfsacl.h
index ca9a8501ff30..2c2ad204d3b0 100644
--- a/include/uapi/linux/nfsacl.h
+++ b/include/uapi/linux/nfsacl.h
@@ -9,11 +9,13 @@
 
 #define NFS_ACL_PROGRAM	100227
 
+#define ACLPROC2_NULL		0
 #define ACLPROC2_GETACL		1
 #define ACLPROC2_SETACL		2
 #define ACLPROC2_GETATTR	3
 #define ACLPROC2_ACCESS		4
 
+#define ACLPROC3_NULL		0
 #define ACLPROC3_GETACL		1
 #define ACLPROC3_SETACL		2
 



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

* [PATCH v3 05/15] NFSD: Encoder and decoder functions are always present
  2020-10-01 22:58 [PATCH v3 00/15] nfsd_dispatch() clean up Chuck Lever
                   ` (3 preceding siblings ...)
  2020-10-01 22:59 ` [PATCH v3 04/15] NFSACL: " Chuck Lever
@ 2020-10-01 22:59 ` Chuck Lever
  2020-10-01 22:59 ` [PATCH v3 06/15] NFSD: Clean up switch statement in nfsd_dispatch() Chuck Lever
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Chuck Lever @ 2020-10-01 22:59 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

nfsd_dispatch() is a hot path. Let's optimize the XDR method calls
for the by-far common case, which is that the XDR methods are indeed
present.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfs2acl.c  |    6 ++++++
 fs/nfsd/nfs3acl.c  |    1 +
 fs/nfsd/nfs3proc.c |    1 +
 fs/nfsd/nfs3xdr.c  |    6 ++++++
 fs/nfsd/nfs4proc.c |    1 +
 fs/nfsd/nfs4xdr.c  |    6 ++++++
 fs/nfsd/nfssvc.c   |    5 ++---
 fs/nfsd/xdr3.h     |    1 +
 fs/nfsd/xdr4.h     |    1 +
 9 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/nfs2acl.c b/fs/nfsd/nfs2acl.c
index 8d20e0d74417..3c8b9250dc4a 100644
--- a/fs/nfsd/nfs2acl.c
+++ b/fs/nfsd/nfs2acl.c
@@ -183,6 +183,11 @@ static __be32 nfsacld_proc_access(struct svc_rqst *rqstp)
 /*
  * XDR decode functions
  */
+static int nfsaclsvc_decode_voidarg(struct svc_rqst *rqstp, __be32 *p)
+{
+	return 1;
+}
+
 static int nfsaclsvc_decode_getaclargs(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd3_getaclargs *argp = rqstp->rq_argp;
@@ -357,6 +362,7 @@ struct nfsd3_voidargs { int dummy; };
 static const struct svc_procedure nfsd_acl_procedures2[5] = {
 	[ACLPROC2_NULL] = {
 		.pc_func = nfsacld_proc_null,
+		.pc_decode = nfsaclsvc_decode_voidarg,
 		.pc_encode = nfsaclsvc_encode_voidres,
 		.pc_argsize = sizeof(struct nfsd3_voidargs),
 		.pc_ressize = sizeof(struct nfsd3_voidargs),
diff --git a/fs/nfsd/nfs3acl.c b/fs/nfsd/nfs3acl.c
index 292acb2e529c..614168675c17 100644
--- a/fs/nfsd/nfs3acl.c
+++ b/fs/nfsd/nfs3acl.c
@@ -245,6 +245,7 @@ struct nfsd3_voidargs { int dummy; };
 static const struct svc_procedure nfsd_acl_procedures3[3] = {
 	[ACLPROC3_NULL] = {
 		.pc_func = nfsd3_proc_null,
+		.pc_decode = nfs3svc_decode_voidarg,
 		.pc_encode = nfs3svc_encode_voidres,
 		.pc_argsize = sizeof(struct nfsd3_voidargs),
 		.pc_ressize = sizeof(struct nfsd3_voidargs),
diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
index 288bc76b4574..3d09959c7042 100644
--- a/fs/nfsd/nfs3proc.c
+++ b/fs/nfsd/nfs3proc.c
@@ -716,6 +716,7 @@ struct nfsd3_voidargs { int dummy; };
 static const struct svc_procedure nfsd_procedures3[22] = {
 	[NFS3PROC_NULL] = {
 		.pc_func = nfsd3_proc_null,
+		.pc_decode = nfs3svc_decode_voidarg,
 		.pc_encode = nfs3svc_encode_voidres,
 		.pc_argsize = sizeof(struct nfsd3_voidargs),
 		.pc_ressize = sizeof(struct nfsd3_voidres),
diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
index aae514d40b64..e540fd1a29d8 100644
--- a/fs/nfsd/nfs3xdr.c
+++ b/fs/nfsd/nfs3xdr.c
@@ -304,6 +304,12 @@ void fill_post_wcc(struct svc_fh *fhp)
 /*
  * XDR decode functions
  */
+int
+nfs3svc_decode_voidarg(struct svc_rqst *rqstp, __be32 *p)
+{
+	return 1;
+}
+
 int
 nfs3svc_decode_fhandle(struct svc_rqst *rqstp, __be32 *p)
 {
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index eaf50eafa935..b99c050797db 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -3279,6 +3279,7 @@ struct nfsd4_voidargs { int dummy; };
 static const struct svc_procedure nfsd_procedures4[2] = {
 	[NFSPROC4_NULL] = {
 		.pc_func = nfsd4_proc_null,
+		.pc_decode = nfs4svc_decode_voidarg,
 		.pc_encode = nfs4svc_encode_voidres,
 		.pc_argsize = sizeof(struct nfsd4_voidargs),
 		.pc_ressize = sizeof(struct nfsd4_voidres),
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index f14d90a95fe3..758d8154a5b3 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -5156,6 +5156,12 @@ void nfsd4_release_compoundargs(struct svc_rqst *rqstp)
 	}
 }
 
+int
+nfs4svc_decode_voidarg(struct svc_rqst *rqstp, __be32 *p)
+{
+	return 1;
+}
+
 int
 nfs4svc_decode_compoundargs(struct svc_rqst *rqstp, __be32 *p)
 {
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index f6bc94cab9da..b2d20920a998 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -1022,8 +1022,7 @@ nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)
 	 */
 	rqstp->rq_cachetype = proc->pc_cachetype;
 	/* Decode arguments */
-	if (proc->pc_decode &&
-	    !proc->pc_decode(rqstp, (__be32*)rqstp->rq_arg.head[0].iov_base)) {
+	if (!proc->pc_decode(rqstp, (__be32 *)rqstp->rq_arg.head[0].iov_base)) {
 		dprintk("nfsd: failed to decode arguments!\n");
 		*statp = rpc_garbage_args;
 		return 1;
@@ -1062,7 +1061,7 @@ nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)
 	 * For NFSv2, additional info is never returned in case of an error.
 	 */
 	if (!(nfserr && rqstp->rq_vers == 2)) {
-		if (proc->pc_encode && !proc->pc_encode(rqstp, nfserrp)) {
+		if (!proc->pc_encode(rqstp, nfserrp)) {
 			/* Failed to encode result. Release cache entry */
 			dprintk("nfsd: failed to encode result!\n");
 			nfsd_cache_update(rqstp, RC_NOCACHE, NULL);
diff --git a/fs/nfsd/xdr3.h b/fs/nfsd/xdr3.h
index 4155fd71714c..ae6fa6c9cb46 100644
--- a/fs/nfsd/xdr3.h
+++ b/fs/nfsd/xdr3.h
@@ -273,6 +273,7 @@ union nfsd3_xdrstore {
 
 #define NFS3_SVC_XDRSIZE		sizeof(union nfsd3_xdrstore)
 
+int nfs3svc_decode_voidarg(struct svc_rqst *, __be32 *);
 int nfs3svc_decode_fhandle(struct svc_rqst *, __be32 *);
 int nfs3svc_decode_sattrargs(struct svc_rqst *, __be32 *);
 int nfs3svc_decode_diropargs(struct svc_rqst *, __be32 *);
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index 66499fb6b567..679d40af1bbb 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -781,6 +781,7 @@ set_change_info(struct nfsd4_change_info *cinfo, struct svc_fh *fhp)
 
 
 bool nfsd4_mach_creds_match(struct nfs4_client *cl, struct svc_rqst *rqstp);
+int nfs4svc_decode_voidarg(struct svc_rqst *, __be32 *);
 int nfs4svc_encode_voidres(struct svc_rqst *, __be32 *);
 int nfs4svc_decode_compoundargs(struct svc_rqst *, __be32 *);
 int nfs4svc_encode_compoundres(struct svc_rqst *, __be32 *);



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

* [PATCH v3 06/15] NFSD: Clean up switch statement in nfsd_dispatch()
  2020-10-01 22:58 [PATCH v3 00/15] nfsd_dispatch() clean up Chuck Lever
                   ` (4 preceding siblings ...)
  2020-10-01 22:59 ` [PATCH v3 05/15] NFSD: Encoder and decoder functions are always present Chuck Lever
@ 2020-10-01 22:59 ` Chuck Lever
  2020-10-01 22:59 ` [PATCH v3 07/15] NFSD: Clean up stale comments " Chuck Lever
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Chuck Lever @ 2020-10-01 22:59 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

Reorder the arms so the compiler places checks for the most frequent
case first.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfssvc.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index b2d20920a998..3cdefb2294ce 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -1030,12 +1030,12 @@ nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)
 
 	/* Check whether we have this call in the cache. */
 	switch (nfsd_cache_lookup(rqstp)) {
-	case RC_DROPIT:
-		return 0;
+	case RC_DOIT:
+		break;
 	case RC_REPLY:
 		return 1;
-	case RC_DOIT:;
-		/* do it */
+	case RC_DROPIT:
+		return 0;
 	}
 
 	/* need to grab the location to store the status, as



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

* [PATCH v3 07/15] NFSD: Clean up stale comments in nfsd_dispatch()
  2020-10-01 22:58 [PATCH v3 00/15] nfsd_dispatch() clean up Chuck Lever
                   ` (5 preceding siblings ...)
  2020-10-01 22:59 ` [PATCH v3 06/15] NFSD: Clean up switch statement in nfsd_dispatch() Chuck Lever
@ 2020-10-01 22:59 ` Chuck Lever
  2020-10-01 22:59 ` [PATCH v3 08/15] NFSD: Clean up nfsd_dispatch() variables Chuck Lever
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Chuck Lever @ 2020-10-01 22:59 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

Add a documenting comment for the function. Remove comments that
simply describe obvious aspects of the code, but leave comments
that explain the differences in processing of each NFS version.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfssvc.c |   26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index 3cdefb2294ce..9be2e14b3bca 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -1000,8 +1000,18 @@ static bool nfs_request_too_big(struct svc_rqst *rqstp,
 	return rqstp->rq_arg.len > PAGE_SIZE;
 }
 
-int
-nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)
+/**
+ * nfsd_dispatch - Process an NFS or NFSACL Request
+ * @rqstp: incoming request
+ * @statp: pointer to location of accept_stat field in RPC Reply buffer
+ *
+ * This RPC dispatcher integrates the NFS server's duplicate reply cache.
+ *
+ * Return values:
+ *  %0: Processing complete; do not send a Reply
+ *  %1: Processing complete; send Reply in rqstp->rq_res
+ */
+int nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)
 {
 	const struct svc_procedure *proc;
 	__be32			nfserr;
@@ -1021,14 +1031,12 @@ nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)
 	 * (necessary in the NFSv4.0 compound case)
 	 */
 	rqstp->rq_cachetype = proc->pc_cachetype;
-	/* Decode arguments */
 	if (!proc->pc_decode(rqstp, (__be32 *)rqstp->rq_arg.head[0].iov_base)) {
 		dprintk("nfsd: failed to decode arguments!\n");
 		*statp = rpc_garbage_args;
 		return 1;
 	}
 
-	/* Check whether we have this call in the cache. */
 	switch (nfsd_cache_lookup(rqstp)) {
 	case RC_DOIT:
 		break;
@@ -1038,14 +1046,14 @@ nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)
 		return 0;
 	}
 
-	/* need to grab the location to store the status, as
-	 * nfsv4 does some encoding while processing 
+	/*
+	 * Need to grab the location to store the status, as
+	 * NFSv4 does some encoding while processing
 	 */
 	nfserrp = rqstp->rq_res.head[0].iov_base
 		+ rqstp->rq_res.head[0].iov_len;
 	rqstp->rq_res.head[0].iov_len += sizeof(__be32);
 
-	/* Now call the procedure handler, and encode NFS status. */
 	nfserr = proc->pc_func(rqstp);
 	nfserr = map_new_errors(rqstp->rq_vers, nfserr);
 	if (nfserr == nfserr_dropit || test_bit(RQ_DROPME, &rqstp->rq_flags)) {
@@ -1057,12 +1065,11 @@ nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)
 	if (rqstp->rq_proc != 0)
 		*nfserrp++ = nfserr;
 
-	/* Encode result.
+	/*
 	 * For NFSv2, additional info is never returned in case of an error.
 	 */
 	if (!(nfserr && rqstp->rq_vers == 2)) {
 		if (!proc->pc_encode(rqstp, nfserrp)) {
-			/* Failed to encode result. Release cache entry */
 			dprintk("nfsd: failed to encode result!\n");
 			nfsd_cache_update(rqstp, RC_NOCACHE, NULL);
 			*statp = rpc_system_err;
@@ -1070,7 +1077,6 @@ nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)
 		}
 	}
 
-	/* Store reply in cache. */
 	nfsd_cache_update(rqstp, rqstp->rq_cachetype, statp + 1);
 	return 1;
 }



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

* [PATCH v3 08/15] NFSD: Clean up nfsd_dispatch() variables
  2020-10-01 22:58 [PATCH v3 00/15] nfsd_dispatch() clean up Chuck Lever
                   ` (6 preceding siblings ...)
  2020-10-01 22:59 ` [PATCH v3 07/15] NFSD: Clean up stale comments " Chuck Lever
@ 2020-10-01 22:59 ` Chuck Lever
  2020-10-01 22:59 ` [PATCH v3 09/15] NFSD: Refactor nfsd_dispatch() error paths Chuck Lever
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Chuck Lever @ 2020-10-01 22:59 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

For consistency and code legibility, use a similar organization of
variables as svc_generic_dispatch().

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfssvc.c |   15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index 9be2e14b3bca..aa516838ce0b 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -1013,13 +1013,13 @@ static bool nfs_request_too_big(struct svc_rqst *rqstp,
  */
 int nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)
 {
-	const struct svc_procedure *proc;
-	__be32			nfserr;
-	__be32			*nfserrp;
+	const struct svc_procedure *proc = rqstp->rq_procinfo;
+	struct kvec *argv = &rqstp->rq_arg.head[0];
+	struct kvec *resv = &rqstp->rq_res.head[0];
+	__be32 nfserr, *nfserrp;
 
 	dprintk("nfsd_dispatch: vers %d proc %d\n",
 				rqstp->rq_vers, rqstp->rq_proc);
-	proc = rqstp->rq_procinfo;
 
 	if (nfs_request_too_big(rqstp, proc)) {
 		dprintk("nfsd: NFSv%d argument too large\n", rqstp->rq_vers);
@@ -1031,7 +1031,7 @@ int nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)
 	 * (necessary in the NFSv4.0 compound case)
 	 */
 	rqstp->rq_cachetype = proc->pc_cachetype;
-	if (!proc->pc_decode(rqstp, (__be32 *)rqstp->rq_arg.head[0].iov_base)) {
+	if (!proc->pc_decode(rqstp, argv->iov_base)) {
 		dprintk("nfsd: failed to decode arguments!\n");
 		*statp = rpc_garbage_args;
 		return 1;
@@ -1050,9 +1050,8 @@ int nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)
 	 * Need to grab the location to store the status, as
 	 * NFSv4 does some encoding while processing
 	 */
-	nfserrp = rqstp->rq_res.head[0].iov_base
-		+ rqstp->rq_res.head[0].iov_len;
-	rqstp->rq_res.head[0].iov_len += sizeof(__be32);
+	nfserrp = resv->iov_base + resv->iov_len;
+	resv->iov_len += sizeof(__be32);
 
 	nfserr = proc->pc_func(rqstp);
 	nfserr = map_new_errors(rqstp->rq_vers, nfserr);



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

* [PATCH v3 09/15] NFSD: Refactor nfsd_dispatch() error paths
  2020-10-01 22:58 [PATCH v3 00/15] nfsd_dispatch() clean up Chuck Lever
                   ` (7 preceding siblings ...)
  2020-10-01 22:59 ` [PATCH v3 08/15] NFSD: Clean up nfsd_dispatch() variables Chuck Lever
@ 2020-10-01 22:59 ` Chuck Lever
  2020-10-01 22:59 ` [PATCH v3 10/15] NFSD: Remove vestigial typedefs Chuck Lever
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Chuck Lever @ 2020-10-01 22:59 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

nfsd_dispatch() is a hot path. Ensure the compiler takes the
processing of rare error cases out of line.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfssvc.c |   60 ++++++++++++++++++++++++++++++++----------------------
 1 file changed, 35 insertions(+), 25 deletions(-)

diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index aa516838ce0b..e793344227c9 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -1021,29 +1021,24 @@ int nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)
 	dprintk("nfsd_dispatch: vers %d proc %d\n",
 				rqstp->rq_vers, rqstp->rq_proc);
 
-	if (nfs_request_too_big(rqstp, proc)) {
-		dprintk("nfsd: NFSv%d argument too large\n", rqstp->rq_vers);
-		*statp = rpc_garbage_args;
-		return 1;
-	}
+	if (nfs_request_too_big(rqstp, proc))
+		goto out_too_large;
+
 	/*
 	 * Give the xdr decoder a chance to change this if it wants
 	 * (necessary in the NFSv4.0 compound case)
 	 */
 	rqstp->rq_cachetype = proc->pc_cachetype;
-	if (!proc->pc_decode(rqstp, argv->iov_base)) {
-		dprintk("nfsd: failed to decode arguments!\n");
-		*statp = rpc_garbage_args;
-		return 1;
-	}
+	if (!proc->pc_decode(rqstp, argv->iov_base))
+		goto out_decode_err;
 
 	switch (nfsd_cache_lookup(rqstp)) {
 	case RC_DOIT:
 		break;
 	case RC_REPLY:
-		return 1;
+		goto out_cached_reply;
 	case RC_DROPIT:
-		return 0;
+		goto out_dropit;
 	}
 
 	/*
@@ -1055,11 +1050,8 @@ int nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)
 
 	nfserr = proc->pc_func(rqstp);
 	nfserr = map_new_errors(rqstp->rq_vers, nfserr);
-	if (nfserr == nfserr_dropit || test_bit(RQ_DROPME, &rqstp->rq_flags)) {
-		dprintk("nfsd: Dropping request; may be revisited later\n");
-		nfsd_cache_update(rqstp, RC_NOCACHE, NULL);
-		return 0;
-	}
+	if (nfserr == nfserr_dropit || test_bit(RQ_DROPME, &rqstp->rq_flags))
+		goto out_update_drop;
 
 	if (rqstp->rq_proc != 0)
 		*nfserrp++ = nfserr;
@@ -1067,16 +1059,34 @@ int nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)
 	/*
 	 * For NFSv2, additional info is never returned in case of an error.
 	 */
-	if (!(nfserr && rqstp->rq_vers == 2)) {
-		if (!proc->pc_encode(rqstp, nfserrp)) {
-			dprintk("nfsd: failed to encode result!\n");
-			nfsd_cache_update(rqstp, RC_NOCACHE, NULL);
-			*statp = rpc_system_err;
-			return 1;
-		}
-	}
+	if (!(nfserr && rqstp->rq_vers == 2))
+		if (!proc->pc_encode(rqstp, nfserrp))
+			goto out_encode_err;
 
 	nfsd_cache_update(rqstp, rqstp->rq_cachetype, statp + 1);
+out_cached_reply:
+	return 1;
+
+out_too_large:
+	dprintk("nfsd: NFSv%d argument too large\n", rqstp->rq_vers);
+	*statp = rpc_garbage_args;
+	return 1;
+
+out_decode_err:
+	dprintk("nfsd: failed to decode arguments!\n");
+	*statp = rpc_garbage_args;
+	return 1;
+
+out_update_drop:
+	dprintk("nfsd: Dropping request; may be revisited later\n");
+	nfsd_cache_update(rqstp, RC_NOCACHE, NULL);
+out_dropit:
+	return 0;
+
+out_encode_err:
+	dprintk("nfsd: failed to encode result!\n");
+	nfsd_cache_update(rqstp, RC_NOCACHE, NULL);
+	*statp = rpc_system_err;
 	return 1;
 }
 



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

* [PATCH v3 10/15] NFSD: Remove vestigial typedefs
  2020-10-01 22:58 [PATCH v3 00/15] nfsd_dispatch() clean up Chuck Lever
                   ` (8 preceding siblings ...)
  2020-10-01 22:59 ` [PATCH v3 09/15] NFSD: Refactor nfsd_dispatch() error paths Chuck Lever
@ 2020-10-01 22:59 ` Chuck Lever
  2020-10-01 22:59 ` [PATCH v3 11/15] NFSD: Fix .pc_release method for NFSv2 Chuck Lever
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Chuck Lever @ 2020-10-01 22:59 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

Clean up: These are not used.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfsproc.c |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c
index 6d1b3af40a4f..2a4c4178acf1 100644
--- a/fs/nfsd/nfsproc.c
+++ b/fs/nfsd/nfsproc.c
@@ -11,9 +11,6 @@
 #include "xdr.h"
 #include "vfs.h"
 
-typedef struct svc_rqst	svc_rqst;
-typedef struct svc_buf	svc_buf;
-
 #define NFSDDBG_FACILITY		NFSDDBG_PROC
 
 



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

* [PATCH v3 11/15] NFSD: Fix .pc_release method for NFSv2
  2020-10-01 22:58 [PATCH v3 00/15] nfsd_dispatch() clean up Chuck Lever
                   ` (9 preceding siblings ...)
  2020-10-01 22:59 ` [PATCH v3 10/15] NFSD: Remove vestigial typedefs Chuck Lever
@ 2020-10-01 22:59 ` Chuck Lever
  2020-10-01 22:59 ` [PATCH v3 12/15] NFSD: Call NFSv2 encoders on error returns Chuck Lever
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Chuck Lever @ 2020-10-01 22:59 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

nfsd_release_fhandle() assumes that rqstp->rq_resp always points to
an nfsd_fhandle struct. In fact, no NFSv2 procedure uses struct
nfsd_fhandle as its response structure.

So far that has been "safe" to do because the res structs put the
resp->fh field at that same offset as struct nfsd_fhandle. I don't
think that's a guarantee, though, and there is certainly nothing
preventing a developer from altering the fields in those structures.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfsproc.c |   14 +++++++-------
 fs/nfsd/nfsxdr.c  |   19 ++++++++++++++++---
 fs/nfsd/xdr.h     |    4 +++-
 3 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c
index 2a4c4178acf1..c349e1dac3ff 100644
--- a/fs/nfsd/nfsproc.c
+++ b/fs/nfsd/nfsproc.c
@@ -611,7 +611,7 @@ static const struct svc_procedure nfsd_procedures2[18] = {
 		.pc_func = nfsd_proc_getattr,
 		.pc_decode = nfssvc_decode_fhandle,
 		.pc_encode = nfssvc_encode_attrstat,
-		.pc_release = nfssvc_release_fhandle,
+		.pc_release = nfssvc_release_attrstat,
 		.pc_argsize = sizeof(struct nfsd_fhandle),
 		.pc_ressize = sizeof(struct nfsd_attrstat),
 		.pc_cachetype = RC_NOCACHE,
@@ -621,7 +621,7 @@ static const struct svc_procedure nfsd_procedures2[18] = {
 		.pc_func = nfsd_proc_setattr,
 		.pc_decode = nfssvc_decode_sattrargs,
 		.pc_encode = nfssvc_encode_attrstat,
-		.pc_release = nfssvc_release_fhandle,
+		.pc_release = nfssvc_release_attrstat,
 		.pc_argsize = sizeof(struct nfsd_sattrargs),
 		.pc_ressize = sizeof(struct nfsd_attrstat),
 		.pc_cachetype = RC_REPLBUFF,
@@ -640,7 +640,7 @@ static const struct svc_procedure nfsd_procedures2[18] = {
 		.pc_func = nfsd_proc_lookup,
 		.pc_decode = nfssvc_decode_diropargs,
 		.pc_encode = nfssvc_encode_diropres,
-		.pc_release = nfssvc_release_fhandle,
+		.pc_release = nfssvc_release_diropres,
 		.pc_argsize = sizeof(struct nfsd_diropargs),
 		.pc_ressize = sizeof(struct nfsd_diropres),
 		.pc_cachetype = RC_NOCACHE,
@@ -659,7 +659,7 @@ static const struct svc_procedure nfsd_procedures2[18] = {
 		.pc_func = nfsd_proc_read,
 		.pc_decode = nfssvc_decode_readargs,
 		.pc_encode = nfssvc_encode_readres,
-		.pc_release = nfssvc_release_fhandle,
+		.pc_release = nfssvc_release_readres,
 		.pc_argsize = sizeof(struct nfsd_readargs),
 		.pc_ressize = sizeof(struct nfsd_readres),
 		.pc_cachetype = RC_NOCACHE,
@@ -678,7 +678,7 @@ static const struct svc_procedure nfsd_procedures2[18] = {
 		.pc_func = nfsd_proc_write,
 		.pc_decode = nfssvc_decode_writeargs,
 		.pc_encode = nfssvc_encode_attrstat,
-		.pc_release = nfssvc_release_fhandle,
+		.pc_release = nfssvc_release_attrstat,
 		.pc_argsize = sizeof(struct nfsd_writeargs),
 		.pc_ressize = sizeof(struct nfsd_attrstat),
 		.pc_cachetype = RC_REPLBUFF,
@@ -688,7 +688,7 @@ static const struct svc_procedure nfsd_procedures2[18] = {
 		.pc_func = nfsd_proc_create,
 		.pc_decode = nfssvc_decode_createargs,
 		.pc_encode = nfssvc_encode_diropres,
-		.pc_release = nfssvc_release_fhandle,
+		.pc_release = nfssvc_release_diropres,
 		.pc_argsize = sizeof(struct nfsd_createargs),
 		.pc_ressize = sizeof(struct nfsd_diropres),
 		.pc_cachetype = RC_REPLBUFF,
@@ -734,7 +734,7 @@ static const struct svc_procedure nfsd_procedures2[18] = {
 		.pc_func = nfsd_proc_mkdir,
 		.pc_decode = nfssvc_decode_createargs,
 		.pc_encode = nfssvc_encode_diropres,
-		.pc_release = nfssvc_release_fhandle,
+		.pc_release = nfssvc_release_diropres,
 		.pc_argsize = sizeof(struct nfsd_createargs),
 		.pc_ressize = sizeof(struct nfsd_diropres),
 		.pc_cachetype = RC_REPLBUFF,
diff --git a/fs/nfsd/nfsxdr.c b/fs/nfsd/nfsxdr.c
index b51fe515f06f..39c004ec7d85 100644
--- a/fs/nfsd/nfsxdr.c
+++ b/fs/nfsd/nfsxdr.c
@@ -561,10 +561,23 @@ nfssvc_encode_entry(void *ccdv, const char *name,
 /*
  * XDR release functions
  */
-void
-nfssvc_release_fhandle(struct svc_rqst *rqstp)
+void nfssvc_release_attrstat(struct svc_rqst *rqstp)
 {
-	struct nfsd_fhandle *resp = rqstp->rq_resp;
+	struct nfsd_attrstat *resp = rqstp->rq_resp;
+
+	fh_put(&resp->fh);
+}
+
+void nfssvc_release_diropres(struct svc_rqst *rqstp)
+{
+	struct nfsd_diropres *resp = rqstp->rq_resp;
+
+	fh_put(&resp->fh);
+}
+
+void nfssvc_release_readres(struct svc_rqst *rqstp)
+{
+	struct nfsd_readres *resp = rqstp->rq_resp;
 
 	fh_put(&resp->fh);
 }
diff --git a/fs/nfsd/xdr.h b/fs/nfsd/xdr.h
index ea7cca3a64b7..3d3e16d48268 100644
--- a/fs/nfsd/xdr.h
+++ b/fs/nfsd/xdr.h
@@ -156,7 +156,9 @@ int nfssvc_encode_readdirres(struct svc_rqst *, __be32 *);
 int nfssvc_encode_entry(void *, const char *name,
 			int namlen, loff_t offset, u64 ino, unsigned int);
 
-void nfssvc_release_fhandle(struct svc_rqst *);
+void nfssvc_release_attrstat(struct svc_rqst *rqstp);
+void nfssvc_release_diropres(struct svc_rqst *rqstp);
+void nfssvc_release_readres(struct svc_rqst *rqstp);
 
 /* Helper functions for NFSv2 ACL code */
 __be32 *nfs2svc_encode_fattr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp, struct kstat *stat);



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

* [PATCH v3 12/15] NFSD: Call NFSv2 encoders on error returns
  2020-10-01 22:58 [PATCH v3 00/15] nfsd_dispatch() clean up Chuck Lever
                   ` (10 preceding siblings ...)
  2020-10-01 22:59 ` [PATCH v3 11/15] NFSD: Fix .pc_release method for NFSv2 Chuck Lever
@ 2020-10-01 22:59 ` Chuck Lever
  2020-10-01 22:59 ` [PATCH v3 13/15] NFSD: Remove the RETURN_STATUS() macro Chuck Lever
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Chuck Lever @ 2020-10-01 22:59 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

Remove special dispatcher logic for NFSv2 error responses. These are
rare to the point of becoming extinct, but all NFS responses have to
pay the cost of the extra conditional branches.

With this change, the NFSv2 error cases now get proper
xdr_ressize_check() calls.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfs2acl.c |   75 ++++++++++++---------
 fs/nfsd/nfsproc.c |  185 +++++++++++++++++++++++++++++------------------------
 fs/nfsd/nfssvc.c  |    8 +-
 fs/nfsd/nfsxdr.c  |   18 +++++
 fs/nfsd/xdr.h     |    7 ++
 5 files changed, 171 insertions(+), 122 deletions(-)

diff --git a/fs/nfsd/nfs2acl.c b/fs/nfsd/nfs2acl.c
index 3c8b9250dc4a..6f46afdb0616 100644
--- a/fs/nfsd/nfs2acl.c
+++ b/fs/nfsd/nfs2acl.c
@@ -14,7 +14,6 @@
 #include "vfs.h"
 
 #define NFSDDBG_FACILITY		NFSDDBG_PROC
-#define RETURN_STATUS(st)	{ resp->status = (st); return (st); }
 
 /*
  * NULL call.
@@ -35,24 +34,25 @@ static __be32 nfsacld_proc_getacl(struct svc_rqst *rqstp)
 	struct posix_acl *acl;
 	struct inode *inode;
 	svc_fh *fh;
-	__be32 nfserr = 0;
 
 	dprintk("nfsd: GETACL(2acl)   %s\n", SVCFH_fmt(&argp->fh));
 
 	fh = fh_copy(&resp->fh, &argp->fh);
-	nfserr = fh_verify(rqstp, &resp->fh, 0, NFSD_MAY_NOP);
-	if (nfserr)
-		RETURN_STATUS(nfserr);
+	resp->status = fh_verify(rqstp, &resp->fh, 0, NFSD_MAY_NOP);
+	if (resp->status != nfs_ok)
+		goto out;
 
 	inode = d_inode(fh->fh_dentry);
 
-	if (argp->mask & ~NFS_ACL_MASK)
-		RETURN_STATUS(nfserr_inval);
+	if (argp->mask & ~NFS_ACL_MASK) {
+		resp->status = nfserr_inval;
+		goto out;
+	}
 	resp->mask = argp->mask;
 
-	nfserr = fh_getattr(fh, &resp->stat);
-	if (nfserr)
-		RETURN_STATUS(nfserr);
+	resp->status = fh_getattr(fh, &resp->stat);
+	if (resp->status != nfs_ok)
+		goto out;
 
 	if (resp->mask & (NFS_ACL|NFS_ACLCNT)) {
 		acl = get_acl(inode, ACL_TYPE_ACCESS);
@@ -61,7 +61,7 @@ static __be32 nfsacld_proc_getacl(struct svc_rqst *rqstp)
 			acl = posix_acl_from_mode(inode->i_mode, GFP_KERNEL);
 		}
 		if (IS_ERR(acl)) {
-			nfserr = nfserrno(PTR_ERR(acl));
+			resp->status = nfserrno(PTR_ERR(acl));
 			goto fail;
 		}
 		resp->acl_access = acl;
@@ -71,19 +71,20 @@ static __be32 nfsacld_proc_getacl(struct svc_rqst *rqstp)
 		   of a non-directory! */
 		acl = get_acl(inode, ACL_TYPE_DEFAULT);
 		if (IS_ERR(acl)) {
-			nfserr = nfserrno(PTR_ERR(acl));
+			resp->status = nfserrno(PTR_ERR(acl));
 			goto fail;
 		}
 		resp->acl_default = acl;
 	}
 
 	/* resp->acl_{access,default} are released in nfssvc_release_getacl. */
-	RETURN_STATUS(0);
+out:
+	return resp->status;
 
 fail:
 	posix_acl_release(resp->acl_access);
 	posix_acl_release(resp->acl_default);
-	RETURN_STATUS(nfserr);
+	goto out;
 }
 
 /*
@@ -95,14 +96,13 @@ static __be32 nfsacld_proc_setacl(struct svc_rqst *rqstp)
 	struct nfsd_attrstat *resp = rqstp->rq_resp;
 	struct inode *inode;
 	svc_fh *fh;
-	__be32 nfserr = 0;
 	int error;
 
 	dprintk("nfsd: SETACL(2acl)   %s\n", SVCFH_fmt(&argp->fh));
 
 	fh = fh_copy(&resp->fh, &argp->fh);
-	nfserr = fh_verify(rqstp, &resp->fh, 0, NFSD_MAY_SATTR);
-	if (nfserr)
+	resp->status = fh_verify(rqstp, &resp->fh, 0, NFSD_MAY_SATTR);
+	if (resp->status != nfs_ok)
 		goto out;
 
 	inode = d_inode(fh->fh_dentry);
@@ -124,19 +124,20 @@ static __be32 nfsacld_proc_setacl(struct svc_rqst *rqstp)
 
 	fh_drop_write(fh);
 
-	nfserr = fh_getattr(fh, &resp->stat);
+	resp->status = fh_getattr(fh, &resp->stat);
 
 out:
 	/* argp->acl_{access,default} may have been allocated in
 	   nfssvc_decode_setaclargs. */
 	posix_acl_release(argp->acl_access);
 	posix_acl_release(argp->acl_default);
-	return nfserr;
+	return resp->status;
+
 out_drop_lock:
 	fh_unlock(fh);
 	fh_drop_write(fh);
 out_errno:
-	nfserr = nfserrno(error);
+	resp->status = nfserrno(error);
 	goto out;
 }
 
@@ -147,15 +148,16 @@ static __be32 nfsacld_proc_getattr(struct svc_rqst *rqstp)
 {
 	struct nfsd_fhandle *argp = rqstp->rq_argp;
 	struct nfsd_attrstat *resp = rqstp->rq_resp;
-	__be32 nfserr;
+
 	dprintk("nfsd: GETATTR  %s\n", SVCFH_fmt(&argp->fh));
 
 	fh_copy(&resp->fh, &argp->fh);
-	nfserr = fh_verify(rqstp, &resp->fh, 0, NFSD_MAY_NOP);
-	if (nfserr)
-		return nfserr;
-	nfserr = fh_getattr(&resp->fh, &resp->stat);
-	return nfserr;
+	resp->status = fh_verify(rqstp, &resp->fh, 0, NFSD_MAY_NOP);
+	if (resp->status != nfs_ok)
+		goto out;
+	resp->status = fh_getattr(&resp->fh, &resp->stat);
+out:
+	return resp->status;
 }
 
 /*
@@ -165,7 +167,6 @@ static __be32 nfsacld_proc_access(struct svc_rqst *rqstp)
 {
 	struct nfsd3_accessargs *argp = rqstp->rq_argp;
 	struct nfsd3_accessres *resp = rqstp->rq_resp;
-	__be32 nfserr;
 
 	dprintk("nfsd: ACCESS(2acl)   %s 0x%x\n",
 			SVCFH_fmt(&argp->fh),
@@ -173,11 +174,12 @@ static __be32 nfsacld_proc_access(struct svc_rqst *rqstp)
 
 	fh_copy(&resp->fh, &argp->fh);
 	resp->access = argp->access;
-	nfserr = nfsd_access(rqstp, &resp->fh, &resp->access, NULL);
-	if (nfserr)
-		return nfserr;
-	nfserr = fh_getattr(&resp->fh, &resp->stat);
-	return nfserr;
+	resp->status = nfsd_access(rqstp, &resp->fh, &resp->access, NULL);
+	if (resp->status != nfs_ok)
+		goto out;
+	resp->status = fh_getattr(&resp->fh, &resp->stat);
+out:
+	return resp->status;
 }
 
 /*
@@ -273,6 +275,9 @@ static int nfsaclsvc_encode_getaclres(struct svc_rqst *rqstp, __be32 *p)
 	int n;
 	int w;
 
+	if (resp->status != nfs_ok)
+		return xdr_ressize_check(rqstp, p);
+
 	/*
 	 * Since this is version 2, the check for nfserr in
 	 * nfsd_dispatch actually ensures the following cannot happen.
@@ -312,6 +317,9 @@ static int nfsaclsvc_encode_attrstatres(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd_attrstat *resp = rqstp->rq_resp;
 
+	if (resp->status != nfs_ok)
+		return xdr_ressize_check(rqstp, p);
+
 	p = nfs2svc_encode_fattr(rqstp, p, &resp->fh, &resp->stat);
 	return xdr_ressize_check(rqstp, p);
 }
@@ -321,6 +329,9 @@ static int nfsaclsvc_encode_accessres(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd3_accessres *resp = rqstp->rq_resp;
 
+	if (resp->status != nfs_ok)
+		return xdr_ressize_check(rqstp, p);
+
 	p = nfs2svc_encode_fattr(rqstp, p, &resp->fh, &resp->stat);
 	*p++ = htonl(resp->access);
 	return xdr_ressize_check(rqstp, p);
diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c
index c349e1dac3ff..526170319ecf 100644
--- a/fs/nfsd/nfsproc.c
+++ b/fs/nfsd/nfsproc.c
@@ -13,25 +13,12 @@
 
 #define NFSDDBG_FACILITY		NFSDDBG_PROC
 
-
 static __be32
 nfsd_proc_null(struct svc_rqst *rqstp)
 {
 	return nfs_ok;
 }
 
-static __be32
-nfsd_return_attrs(__be32 err, struct nfsd_attrstat *resp)
-{
-	if (err) return err;
-	return fh_getattr(&resp->fh, &resp->stat);
-}
-static __be32
-nfsd_return_dirop(__be32 err, struct nfsd_diropres *resp)
-{
-	if (err) return err;
-	return fh_getattr(&resp->fh, &resp->stat);
-}
 /*
  * Get a file's attributes
  * N.B. After this call resp->fh needs an fh_put
@@ -41,13 +28,17 @@ nfsd_proc_getattr(struct svc_rqst *rqstp)
 {
 	struct nfsd_fhandle *argp = rqstp->rq_argp;
 	struct nfsd_attrstat *resp = rqstp->rq_resp;
-	__be32 nfserr;
+
 	dprintk("nfsd: GETATTR  %s\n", SVCFH_fmt(&argp->fh));
 
 	fh_copy(&resp->fh, &argp->fh);
-	nfserr = fh_verify(rqstp, &resp->fh, 0,
-			NFSD_MAY_NOP | NFSD_MAY_BYPASS_GSS_ON_ROOT);
-	return nfsd_return_attrs(nfserr, resp);
+	resp->status = fh_verify(rqstp, &resp->fh, 0,
+				 NFSD_MAY_NOP | NFSD_MAY_BYPASS_GSS_ON_ROOT);
+	if (resp->status != nfs_ok)
+		goto out;
+	resp->status = fh_getattr(&resp->fh, &resp->stat);
+out:
+	return resp->status;
 }
 
 /*
@@ -61,7 +52,6 @@ nfsd_proc_setattr(struct svc_rqst *rqstp)
 	struct nfsd_attrstat *resp = rqstp->rq_resp;
 	struct iattr *iap = &argp->attrs;
 	struct svc_fh *fhp;
-	__be32 nfserr;
 
 	dprintk("nfsd: SETATTR  %s, valid=%x, size=%ld\n",
 		SVCFH_fmt(&argp->fh),
@@ -93,9 +83,9 @@ nfsd_proc_setattr(struct svc_rqst *rqstp)
 		 */
 		time64_t delta = iap->ia_atime.tv_sec - ktime_get_real_seconds();
 
-		nfserr = fh_verify(rqstp, fhp, 0, NFSD_MAY_NOP);
-		if (nfserr)
-			goto done;
+		resp->status = fh_verify(rqstp, fhp, 0, NFSD_MAY_NOP);
+		if (resp->status != nfs_ok)
+			return resp->status;
 
 		if (delta < 0)
 			delta = -delta;
@@ -110,9 +100,13 @@ nfsd_proc_setattr(struct svc_rqst *rqstp)
 		}
 	}
 
-	nfserr = nfsd_setattr(rqstp, fhp, iap, 0, (time64_t)0);
-done:
-	return nfsd_return_attrs(nfserr, resp);
+	resp->status = nfsd_setattr(rqstp, fhp, iap, 0, (time64_t)0);
+	if (resp->status != nfs_ok)
+		goto out;
+
+	resp->status = fh_getattr(&resp->fh, &resp->stat);
+out:
+	return resp->status;
 }
 
 /* Obsolete, replaced by MNTPROC_MNT. */
@@ -133,17 +127,20 @@ nfsd_proc_lookup(struct svc_rqst *rqstp)
 {
 	struct nfsd_diropargs *argp = rqstp->rq_argp;
 	struct nfsd_diropres *resp = rqstp->rq_resp;
-	__be32	nfserr;
 
 	dprintk("nfsd: LOOKUP   %s %.*s\n",
 		SVCFH_fmt(&argp->fh), argp->len, argp->name);
 
 	fh_init(&resp->fh, NFS_FHSIZE);
-	nfserr = nfsd_lookup(rqstp, &argp->fh, argp->name, argp->len,
-				 &resp->fh);
-
+	resp->status = nfsd_lookup(rqstp, &argp->fh, argp->name, argp->len,
+				   &resp->fh);
 	fh_put(&argp->fh);
-	return nfsd_return_dirop(nfserr, resp);
+	if (resp->status != nfs_ok)
+		goto out;
+
+	resp->status = fh_getattr(&resp->fh, &resp->stat);
+out:
+	return resp->status;
 }
 
 /*
@@ -154,16 +151,15 @@ nfsd_proc_readlink(struct svc_rqst *rqstp)
 {
 	struct nfsd_readlinkargs *argp = rqstp->rq_argp;
 	struct nfsd_readlinkres *resp = rqstp->rq_resp;
-	__be32	nfserr;
 
 	dprintk("nfsd: READLINK %s\n", SVCFH_fmt(&argp->fh));
 
 	/* Read the symlink. */
 	resp->len = NFS_MAXPATHLEN;
-	nfserr = nfsd_readlink(rqstp, &argp->fh, argp->buffer, &resp->len);
+	resp->status = nfsd_readlink(rqstp, &argp->fh, argp->buffer, &resp->len);
 
 	fh_put(&argp->fh);
-	return nfserr;
+	return resp->status;
 }
 
 /*
@@ -175,7 +171,6 @@ nfsd_proc_read(struct svc_rqst *rqstp)
 {
 	struct nfsd_readargs *argp = rqstp->rq_argp;
 	struct nfsd_readres *resp = rqstp->rq_resp;
-	__be32	nfserr;
 	u32 eof;
 
 	dprintk("nfsd: READ    %s %d bytes at %d\n",
@@ -197,14 +192,17 @@ nfsd_proc_read(struct svc_rqst *rqstp)
 	svc_reserve_auth(rqstp, (19<<2) + argp->count + 4);
 
 	resp->count = argp->count;
-	nfserr = nfsd_read(rqstp, fh_copy(&resp->fh, &argp->fh),
-				  argp->offset,
-			   	  rqstp->rq_vec, argp->vlen,
-				  &resp->count,
-				  &eof);
-
-	if (nfserr) return nfserr;
-	return fh_getattr(&resp->fh, &resp->stat);
+	resp->status = nfsd_read(rqstp, fh_copy(&resp->fh, &argp->fh),
+				 argp->offset,
+				 rqstp->rq_vec, argp->vlen,
+				 &resp->count,
+				 &eof);
+	if (resp->status != nfs_ok)
+		goto out;
+
+	resp->status = fh_getattr(&resp->fh, &resp->stat);
+out:
+	return resp->status;
 }
 
 /* Reserved */
@@ -223,7 +221,6 @@ nfsd_proc_write(struct svc_rqst *rqstp)
 {
 	struct nfsd_writeargs *argp = rqstp->rq_argp;
 	struct nfsd_attrstat *resp = rqstp->rq_resp;
-	__be32	nfserr;
 	unsigned long cnt = argp->len;
 	unsigned int nvecs;
 
@@ -233,12 +230,20 @@ nfsd_proc_write(struct svc_rqst *rqstp)
 
 	nvecs = svc_fill_write_vector(rqstp, rqstp->rq_arg.pages,
 				      &argp->first, cnt);
-	if (!nvecs)
-		return nfserr_io;
-	nfserr = nfsd_write(rqstp, fh_copy(&resp->fh, &argp->fh),
-			    argp->offset, rqstp->rq_vec, nvecs,
-			    &cnt, NFS_DATA_SYNC, NULL);
-	return nfsd_return_attrs(nfserr, resp);
+	if (!nvecs) {
+		resp->status = nfserr_io;
+		goto out;
+	}
+
+	resp->status = nfsd_write(rqstp, fh_copy(&resp->fh, &argp->fh),
+				  argp->offset, rqstp->rq_vec, nvecs,
+				  &cnt, NFS_DATA_SYNC, NULL);
+	if (resp->status != nfs_ok)
+		goto out;
+
+	resp->status = fh_getattr(&resp->fh, &resp->stat);
+out:
+	return resp->status;
 }
 
 /*
@@ -258,7 +263,6 @@ nfsd_proc_create(struct svc_rqst *rqstp)
 	struct inode	*inode;
 	struct dentry	*dchild;
 	int		type, mode;
-	__be32		nfserr;
 	int		hosterr;
 	dev_t		rdev = 0, wanted = new_decode_dev(attr->ia_size);
 
@@ -266,40 +270,40 @@ nfsd_proc_create(struct svc_rqst *rqstp)
 		SVCFH_fmt(dirfhp), argp->len, argp->name);
 
 	/* First verify the parent file handle */
-	nfserr = fh_verify(rqstp, dirfhp, S_IFDIR, NFSD_MAY_EXEC);
-	if (nfserr)
+	resp->status = fh_verify(rqstp, dirfhp, S_IFDIR, NFSD_MAY_EXEC);
+	if (resp->status != nfs_ok)
 		goto done; /* must fh_put dirfhp even on error */
 
 	/* Check for NFSD_MAY_WRITE in nfsd_create if necessary */
 
-	nfserr = nfserr_exist;
+	resp->status = nfserr_exist;
 	if (isdotent(argp->name, argp->len))
 		goto done;
 	hosterr = fh_want_write(dirfhp);
 	if (hosterr) {
-		nfserr = nfserrno(hosterr);
+		resp->status = nfserrno(hosterr);
 		goto done;
 	}
 
 	fh_lock_nested(dirfhp, I_MUTEX_PARENT);
 	dchild = lookup_one_len(argp->name, dirfhp->fh_dentry, argp->len);
 	if (IS_ERR(dchild)) {
-		nfserr = nfserrno(PTR_ERR(dchild));
+		resp->status = nfserrno(PTR_ERR(dchild));
 		goto out_unlock;
 	}
 	fh_init(newfhp, NFS_FHSIZE);
-	nfserr = fh_compose(newfhp, dirfhp->fh_export, dchild, dirfhp);
-	if (!nfserr && d_really_is_negative(dchild))
-		nfserr = nfserr_noent;
+	resp->status = fh_compose(newfhp, dirfhp->fh_export, dchild, dirfhp);
+	if (!resp->status && d_really_is_negative(dchild))
+		resp->status = nfserr_noent;
 	dput(dchild);
-	if (nfserr) {
-		if (nfserr != nfserr_noent)
+	if (resp->status) {
+		if (resp->status != nfserr_noent)
 			goto out_unlock;
 		/*
 		 * If the new file handle wasn't verified, we can't tell
 		 * whether the file exists or not. Time to bail ...
 		 */
-		nfserr = nfserr_acces;
+		resp->status = nfserr_acces;
 		if (!newfhp->fh_dentry) {
 			printk(KERN_WARNING 
 				"nfsd_proc_create: file handle not verified\n");
@@ -332,11 +336,11 @@ nfsd_proc_create(struct svc_rqst *rqstp)
 					 *   echo thing > device-special-file-or-pipe
 					 * by doing a CREATE with type==0
 					 */
-					nfserr = nfsd_permission(rqstp,
+					resp->status = nfsd_permission(rqstp,
 								 newfhp->fh_export,
 								 newfhp->fh_dentry,
 								 NFSD_MAY_WRITE|NFSD_MAY_LOCAL_ACCESS);
-					if (nfserr && nfserr != nfserr_rofs)
+					if (resp->status && resp->status != nfserr_rofs)
 						goto out_unlock;
 				}
 			} else
@@ -372,16 +376,17 @@ nfsd_proc_create(struct svc_rqst *rqstp)
 		attr->ia_valid &= ~ATTR_SIZE;
 
 		/* Make sure the type and device matches */
-		nfserr = nfserr_exist;
+		resp->status = nfserr_exist;
 		if (inode && type != (inode->i_mode & S_IFMT))
 			goto out_unlock;
 	}
 
-	nfserr = 0;
+	resp->status = nfs_ok;
 	if (!inode) {
 		/* File doesn't exist. Create it and set attrs */
-		nfserr = nfsd_create_locked(rqstp, dirfhp, argp->name,
-					argp->len, attr, type, rdev, newfhp);
+		resp->status = nfsd_create_locked(rqstp, dirfhp, argp->name,
+						  argp->len, attr, type, rdev,
+						  newfhp);
 	} else if (type == S_IFREG) {
 		dprintk("nfsd:   existing %s, valid=%x, size=%ld\n",
 			argp->name, attr->ia_valid, (long) attr->ia_size);
@@ -391,7 +396,8 @@ nfsd_proc_create(struct svc_rqst *rqstp)
 		 */
 		attr->ia_valid &= ATTR_SIZE;
 		if (attr->ia_valid)
-			nfserr = nfsd_setattr(rqstp, newfhp, attr, 0, (time64_t)0);
+			resp->status = nfsd_setattr(rqstp, newfhp, attr, 0,
+						    (time64_t)0);
 	}
 
 out_unlock:
@@ -400,7 +406,11 @@ nfsd_proc_create(struct svc_rqst *rqstp)
 	fh_drop_write(dirfhp);
 done:
 	fh_put(dirfhp);
-	return nfsd_return_dirop(nfserr, resp);
+	if (resp->status != nfs_ok)
+		goto out;
+	resp->status = fh_getattr(&resp->fh, &resp->stat);
+out:
+	return resp->status;
 }
 
 static __be32
@@ -463,14 +473,18 @@ nfsd_proc_symlink(struct svc_rqst *rqstp)
 	struct svc_fh	newfh;
 	__be32		nfserr;
 
-	if (argp->tlen > NFS_MAXPATHLEN)
-		return nfserr_nametoolong;
+	if (argp->tlen > NFS_MAXPATHLEN) {
+		nfserr = nfserr_nametoolong;
+		goto out;
+	}
 
 	argp->tname = svc_fill_symlink_pathname(rqstp, &argp->first,
 						page_address(rqstp->rq_arg.pages[0]),
 						argp->tlen);
-	if (IS_ERR(argp->tname))
-		return nfserrno(PTR_ERR(argp->tname));
+	if (IS_ERR(argp->tname)) {
+		nfserr = nfserrno(PTR_ERR(argp->tname));
+		goto out;
+	}
 
 	dprintk("nfsd: SYMLINK  %s %.*s -> %.*s\n",
 		SVCFH_fmt(&argp->ffh), argp->flen, argp->fname,
@@ -483,6 +497,7 @@ nfsd_proc_symlink(struct svc_rqst *rqstp)
 	kfree(argp->tname);
 	fh_put(&argp->ffh);
 	fh_put(&newfh);
+out:
 	return nfserr;
 }
 
@@ -495,7 +510,6 @@ nfsd_proc_mkdir(struct svc_rqst *rqstp)
 {
 	struct nfsd_createargs *argp = rqstp->rq_argp;
 	struct nfsd_diropres *resp = rqstp->rq_resp;
-	__be32	nfserr;
 
 	dprintk("nfsd: MKDIR    %s %.*s\n", SVCFH_fmt(&argp->fh), argp->len, argp->name);
 
@@ -506,10 +520,15 @@ nfsd_proc_mkdir(struct svc_rqst *rqstp)
 
 	argp->attrs.ia_valid &= ~ATTR_SIZE;
 	fh_init(&resp->fh, NFS_FHSIZE);
-	nfserr = nfsd_create(rqstp, &argp->fh, argp->name, argp->len,
-				    &argp->attrs, S_IFDIR, 0, &resp->fh);
+	resp->status = nfsd_create(rqstp, &argp->fh, argp->name, argp->len,
+				   &argp->attrs, S_IFDIR, 0, &resp->fh);
 	fh_put(&argp->fh);
-	return nfsd_return_dirop(nfserr, resp);
+	if (resp->status != nfs_ok)
+		goto out;
+
+	resp->status = fh_getattr(&resp->fh, &resp->stat);
+out:
+	return resp->status;
 }
 
 /*
@@ -537,7 +556,6 @@ nfsd_proc_readdir(struct svc_rqst *rqstp)
 	struct nfsd_readdirargs *argp = rqstp->rq_argp;
 	struct nfsd_readdirres *resp = rqstp->rq_resp;
 	int		count;
-	__be32		nfserr;
 	loff_t		offset;
 
 	dprintk("nfsd: READDIR  %s %d bytes at %d\n",
@@ -558,15 +576,15 @@ nfsd_proc_readdir(struct svc_rqst *rqstp)
 	resp->common.err = nfs_ok;
 	/* Read directory and encode entries on the fly */
 	offset = argp->cookie;
-	nfserr = nfsd_readdir(rqstp, &argp->fh, &offset, 
-			      &resp->common, nfssvc_encode_entry);
+	resp->status = nfsd_readdir(rqstp, &argp->fh, &offset,
+				    &resp->common, nfssvc_encode_entry);
 
 	resp->count = resp->buffer - argp->buffer;
 	if (resp->offset)
 		*resp->offset = htonl(offset);
 
 	fh_put(&argp->fh);
-	return nfserr;
+	return resp->status;
 }
 
 /*
@@ -577,14 +595,13 @@ nfsd_proc_statfs(struct svc_rqst *rqstp)
 {
 	struct nfsd_fhandle *argp = rqstp->rq_argp;
 	struct nfsd_statfsres *resp = rqstp->rq_resp;
-	__be32	nfserr;
 
 	dprintk("nfsd: STATFS   %s\n", SVCFH_fmt(&argp->fh));
 
-	nfserr = nfsd_statfs(rqstp, &argp->fh, &resp->stats,
-			NFSD_MAY_BYPASS_GSS_ON_ROOT);
+	resp->status = nfsd_statfs(rqstp, &argp->fh, &resp->stats,
+				   NFSD_MAY_BYPASS_GSS_ON_ROOT);
 	fh_put(&argp->fh);
-	return nfserr;
+	return resp->status;
 }
 
 /*
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index e793344227c9..4aa8db879ca2 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -1056,12 +1056,8 @@ int nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)
 	if (rqstp->rq_proc != 0)
 		*nfserrp++ = nfserr;
 
-	/*
-	 * For NFSv2, additional info is never returned in case of an error.
-	 */
-	if (!(nfserr && rqstp->rq_vers == 2))
-		if (!proc->pc_encode(rqstp, nfserrp))
-			goto out_encode_err;
+	if (!proc->pc_encode(rqstp, nfserrp))
+		goto out_encode_err;
 
 	nfsd_cache_update(rqstp, rqstp->rq_cachetype, statp + 1);
 out_cached_reply:
diff --git a/fs/nfsd/nfsxdr.c b/fs/nfsd/nfsxdr.c
index 39c004ec7d85..952e71c95d4e 100644
--- a/fs/nfsd/nfsxdr.c
+++ b/fs/nfsd/nfsxdr.c
@@ -434,6 +434,9 @@ nfssvc_encode_attrstat(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd_attrstat *resp = rqstp->rq_resp;
 
+	if (resp->status != nfs_ok)
+		return xdr_ressize_check(rqstp, p);
+
 	p = encode_fattr(rqstp, p, &resp->fh, &resp->stat);
 	return xdr_ressize_check(rqstp, p);
 }
@@ -443,6 +446,9 @@ nfssvc_encode_diropres(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd_diropres *resp = rqstp->rq_resp;
 
+	if (resp->status != nfs_ok)
+		return xdr_ressize_check(rqstp, p);
+
 	p = encode_fh(p, &resp->fh);
 	p = encode_fattr(rqstp, p, &resp->fh, &resp->stat);
 	return xdr_ressize_check(rqstp, p);
@@ -453,6 +459,9 @@ nfssvc_encode_readlinkres(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd_readlinkres *resp = rqstp->rq_resp;
 
+	if (resp->status != nfs_ok)
+		return xdr_ressize_check(rqstp, p);
+
 	*p++ = htonl(resp->len);
 	xdr_ressize_check(rqstp, p);
 	rqstp->rq_res.page_len = resp->len;
@@ -470,6 +479,9 @@ nfssvc_encode_readres(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd_readres *resp = rqstp->rq_resp;
 
+	if (resp->status != nfs_ok)
+		return xdr_ressize_check(rqstp, p);
+
 	p = encode_fattr(rqstp, p, &resp->fh, &resp->stat);
 	*p++ = htonl(resp->count);
 	xdr_ressize_check(rqstp, p);
@@ -490,6 +502,9 @@ nfssvc_encode_readdirres(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd_readdirres *resp = rqstp->rq_resp;
 
+	if (resp->status != nfs_ok)
+		return xdr_ressize_check(rqstp, p);
+
 	xdr_ressize_check(rqstp, p);
 	p = resp->buffer;
 	*p++ = 0;			/* no more entries */
@@ -505,6 +520,9 @@ nfssvc_encode_statfsres(struct svc_rqst *rqstp, __be32 *p)
 	struct nfsd_statfsres *resp = rqstp->rq_resp;
 	struct kstatfs	*stat = &resp->stats;
 
+	if (resp->status != nfs_ok)
+		return xdr_ressize_check(rqstp, p);
+
 	*p++ = htonl(NFSSVC_MAXBLKSIZE_V2);	/* max transfer size */
 	*p++ = htonl(stat->f_bsize);
 	*p++ = htonl(stat->f_blocks);
diff --git a/fs/nfsd/xdr.h b/fs/nfsd/xdr.h
index 3d3e16d48268..17221931815f 100644
--- a/fs/nfsd/xdr.h
+++ b/fs/nfsd/xdr.h
@@ -83,26 +83,32 @@ struct nfsd_readdirargs {
 };
 
 struct nfsd_attrstat {
+	__be32			status;
 	struct svc_fh		fh;
 	struct kstat		stat;
 };
 
 struct nfsd_diropres  {
+	__be32			status;
 	struct svc_fh		fh;
 	struct kstat		stat;
 };
 
 struct nfsd_readlinkres {
+	__be32			status;
 	int			len;
 };
 
 struct nfsd_readres {
+	__be32			status;
 	struct svc_fh		fh;
 	unsigned long		count;
 	struct kstat		stat;
 };
 
 struct nfsd_readdirres {
+	__be32			status;
+
 	int			count;
 
 	struct readdir_cd	common;
@@ -112,6 +118,7 @@ struct nfsd_readdirres {
 };
 
 struct nfsd_statfsres {
+	__be32			status;
 	struct kstatfs		stats;
 };
 



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

* [PATCH v3 13/15] NFSD: Remove the RETURN_STATUS() macro
  2020-10-01 22:58 [PATCH v3 00/15] nfsd_dispatch() clean up Chuck Lever
                   ` (11 preceding siblings ...)
  2020-10-01 22:59 ` [PATCH v3 12/15] NFSD: Call NFSv2 encoders on error returns Chuck Lever
@ 2020-10-01 22:59 ` Chuck Lever
  2020-10-01 23:00 ` [PATCH v3 14/15] NFSD: Map nfserr_wrongsec outside of nfsd_dispatch Chuck Lever
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Chuck Lever @ 2020-10-01 22:59 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

Refactor: I'm about to change the return value from .pc_func. Clear
the way by replacing the RETURN_STATUS() macro with logic that
plants the status code directly into the response structure.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfs3acl.c  |   33 ++++---
 fs/nfsd/nfs3proc.c |  235 +++++++++++++++++++++++++---------------------------
 2 files changed, 130 insertions(+), 138 deletions(-)

diff --git a/fs/nfsd/nfs3acl.c b/fs/nfsd/nfs3acl.c
index 614168675c17..3fee24dee98c 100644
--- a/fs/nfsd/nfs3acl.c
+++ b/fs/nfsd/nfs3acl.c
@@ -13,8 +13,6 @@
 #include "xdr3.h"
 #include "vfs.h"
 
-#define RETURN_STATUS(st)	{ resp->status = (st); return (st); }
-
 /*
  * NULL call.
  */
@@ -34,17 +32,18 @@ static __be32 nfsd3_proc_getacl(struct svc_rqst *rqstp)
 	struct posix_acl *acl;
 	struct inode *inode;
 	svc_fh *fh;
-	__be32 nfserr = 0;
 
 	fh = fh_copy(&resp->fh, &argp->fh);
-	nfserr = fh_verify(rqstp, &resp->fh, 0, NFSD_MAY_NOP);
-	if (nfserr)
-		RETURN_STATUS(nfserr);
+	resp->status = fh_verify(rqstp, &resp->fh, 0, NFSD_MAY_NOP);
+	if (resp->status != nfs_ok)
+		goto out;
 
 	inode = d_inode(fh->fh_dentry);
 
-	if (argp->mask & ~NFS_ACL_MASK)
-		RETURN_STATUS(nfserr_inval);
+	if (argp->mask & ~NFS_ACL_MASK) {
+		resp->status = nfserr_inval;
+		goto out;
+	}
 	resp->mask = argp->mask;
 
 	if (resp->mask & (NFS_ACL|NFS_ACLCNT)) {
@@ -54,7 +53,7 @@ static __be32 nfsd3_proc_getacl(struct svc_rqst *rqstp)
 			acl = posix_acl_from_mode(inode->i_mode, GFP_KERNEL);
 		}
 		if (IS_ERR(acl)) {
-			nfserr = nfserrno(PTR_ERR(acl));
+			resp->status = nfserrno(PTR_ERR(acl));
 			goto fail;
 		}
 		resp->acl_access = acl;
@@ -64,19 +63,20 @@ static __be32 nfsd3_proc_getacl(struct svc_rqst *rqstp)
 		   of a non-directory! */
 		acl = get_acl(inode, ACL_TYPE_DEFAULT);
 		if (IS_ERR(acl)) {
-			nfserr = nfserrno(PTR_ERR(acl));
+			resp->status = nfserrno(PTR_ERR(acl));
 			goto fail;
 		}
 		resp->acl_default = acl;
 	}
 
 	/* resp->acl_{access,default} are released in nfs3svc_release_getacl. */
-	RETURN_STATUS(0);
+out:
+	return resp->status;
 
 fail:
 	posix_acl_release(resp->acl_access);
 	posix_acl_release(resp->acl_default);
-	RETURN_STATUS(nfserr);
+	goto out;
 }
 
 /*
@@ -88,12 +88,11 @@ static __be32 nfsd3_proc_setacl(struct svc_rqst *rqstp)
 	struct nfsd3_attrstat *resp = rqstp->rq_resp;
 	struct inode *inode;
 	svc_fh *fh;
-	__be32 nfserr = 0;
 	int error;
 
 	fh = fh_copy(&resp->fh, &argp->fh);
-	nfserr = fh_verify(rqstp, &resp->fh, 0, NFSD_MAY_SATTR);
-	if (nfserr)
+	resp->status = fh_verify(rqstp, &resp->fh, 0, NFSD_MAY_SATTR);
+	if (resp->status != nfs_ok)
 		goto out;
 
 	inode = d_inode(fh->fh_dentry);
@@ -113,13 +112,13 @@ static __be32 nfsd3_proc_setacl(struct svc_rqst *rqstp)
 	fh_unlock(fh);
 	fh_drop_write(fh);
 out_errno:
-	nfserr = nfserrno(error);
+	resp->status = nfserrno(error);
 out:
 	/* argp->acl_{access,default} may have been allocated in
 	   nfs3svc_decode_setaclargs. */
 	posix_acl_release(argp->acl_access);
 	posix_acl_release(argp->acl_default);
-	RETURN_STATUS(nfserr);
+	return resp->status;
 }
 
 /*
diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
index 3d09959c7042..1d2c149e5ff4 100644
--- a/fs/nfsd/nfs3proc.c
+++ b/fs/nfsd/nfs3proc.c
@@ -15,8 +15,6 @@
 
 #define NFSDDBG_FACILITY		NFSDDBG_PROC
 
-#define RETURN_STATUS(st)	{ resp->status = (st); return (st); }
-
 static int	nfs3_ftypes[] = {
 	0,			/* NF3NON */
 	S_IFREG,		/* NF3REG */
@@ -45,20 +43,19 @@ nfsd3_proc_getattr(struct svc_rqst *rqstp)
 {
 	struct nfsd_fhandle *argp = rqstp->rq_argp;
 	struct nfsd3_attrstat *resp = rqstp->rq_resp;
-	__be32	nfserr;
 
 	dprintk("nfsd: GETATTR(3)  %s\n",
 		SVCFH_fmt(&argp->fh));
 
 	fh_copy(&resp->fh, &argp->fh);
-	nfserr = fh_verify(rqstp, &resp->fh, 0,
-			NFSD_MAY_NOP | NFSD_MAY_BYPASS_GSS_ON_ROOT);
-	if (nfserr)
-		RETURN_STATUS(nfserr);
-
-	nfserr = fh_getattr(&resp->fh, &resp->stat);
-
-	RETURN_STATUS(nfserr);
+	resp->status = fh_verify(rqstp, &resp->fh, 0,
+				 NFSD_MAY_NOP | NFSD_MAY_BYPASS_GSS_ON_ROOT);
+	if (resp->status != nfs_ok)
+		goto out;
+
+	resp->status = fh_getattr(&resp->fh, &resp->stat);
+out:
+	return resp->status;
 }
 
 /*
@@ -69,15 +66,14 @@ nfsd3_proc_setattr(struct svc_rqst *rqstp)
 {
 	struct nfsd3_sattrargs *argp = rqstp->rq_argp;
 	struct nfsd3_attrstat *resp = rqstp->rq_resp;
-	__be32	nfserr;
 
 	dprintk("nfsd: SETATTR(3)  %s\n",
 				SVCFH_fmt(&argp->fh));
 
 	fh_copy(&resp->fh, &argp->fh);
-	nfserr = nfsd_setattr(rqstp, &resp->fh, &argp->attrs,
-			      argp->check_guard, argp->guardtime);
-	RETURN_STATUS(nfserr);
+	resp->status = nfsd_setattr(rqstp, &resp->fh, &argp->attrs,
+				    argp->check_guard, argp->guardtime);
+	return resp->status;
 }
 
 /*
@@ -88,7 +84,6 @@ nfsd3_proc_lookup(struct svc_rqst *rqstp)
 {
 	struct nfsd3_diropargs *argp = rqstp->rq_argp;
 	struct nfsd3_diropres  *resp = rqstp->rq_resp;
-	__be32	nfserr;
 
 	dprintk("nfsd: LOOKUP(3)   %s %.*s\n",
 				SVCFH_fmt(&argp->fh),
@@ -98,11 +93,10 @@ nfsd3_proc_lookup(struct svc_rqst *rqstp)
 	fh_copy(&resp->dirfh, &argp->fh);
 	fh_init(&resp->fh, NFS3_FHSIZE);
 
-	nfserr = nfsd_lookup(rqstp, &resp->dirfh,
-				    argp->name,
-				    argp->len,
-				    &resp->fh);
-	RETURN_STATUS(nfserr);
+	resp->status = nfsd_lookup(rqstp, &resp->dirfh,
+				   argp->name, argp->len,
+				   &resp->fh);
+	return resp->status;
 }
 
 /*
@@ -113,7 +107,6 @@ nfsd3_proc_access(struct svc_rqst *rqstp)
 {
 	struct nfsd3_accessargs *argp = rqstp->rq_argp;
 	struct nfsd3_accessres *resp = rqstp->rq_resp;
-	__be32	nfserr;
 
 	dprintk("nfsd: ACCESS(3)   %s 0x%x\n",
 				SVCFH_fmt(&argp->fh),
@@ -121,8 +114,8 @@ nfsd3_proc_access(struct svc_rqst *rqstp)
 
 	fh_copy(&resp->fh, &argp->fh);
 	resp->access = argp->access;
-	nfserr = nfsd_access(rqstp, &resp->fh, &resp->access, NULL);
-	RETURN_STATUS(nfserr);
+	resp->status = nfsd_access(rqstp, &resp->fh, &resp->access, NULL);
+	return resp->status;
 }
 
 /*
@@ -133,15 +126,14 @@ nfsd3_proc_readlink(struct svc_rqst *rqstp)
 {
 	struct nfsd3_readlinkargs *argp = rqstp->rq_argp;
 	struct nfsd3_readlinkres *resp = rqstp->rq_resp;
-	__be32 nfserr;
 
 	dprintk("nfsd: READLINK(3) %s\n", SVCFH_fmt(&argp->fh));
 
 	/* Read the symlink. */
 	fh_copy(&resp->fh, &argp->fh);
 	resp->len = NFS3_MAXPATHLEN;
-	nfserr = nfsd_readlink(rqstp, &resp->fh, argp->buffer, &resp->len);
-	RETURN_STATUS(nfserr);
+	resp->status = nfsd_readlink(rqstp, &resp->fh, argp->buffer, &resp->len);
+	return resp->status;
 }
 
 /*
@@ -152,7 +144,6 @@ nfsd3_proc_read(struct svc_rqst *rqstp)
 {
 	struct nfsd3_readargs *argp = rqstp->rq_argp;
 	struct nfsd3_readres *resp = rqstp->rq_resp;
-	__be32	nfserr;
 	u32	max_blocksize = svc_max_payload(rqstp);
 	unsigned long cnt = min(argp->count, max_blocksize);
 
@@ -169,12 +160,10 @@ nfsd3_proc_read(struct svc_rqst *rqstp)
 	svc_reserve_auth(rqstp, ((1 + NFS3_POST_OP_ATTR_WORDS + 3)<<2) + resp->count +4);
 
 	fh_copy(&resp->fh, &argp->fh);
-	nfserr = nfsd_read(rqstp, &resp->fh,
-				  argp->offset,
-			   	  rqstp->rq_vec, argp->vlen,
-				  &resp->count,
-				  &resp->eof);
-	RETURN_STATUS(nfserr);
+	resp->status = nfsd_read(rqstp, &resp->fh, argp->offset,
+				 rqstp->rq_vec, argp->vlen, &resp->count,
+				 &resp->eof);
+	return resp->status;
 }
 
 /*
@@ -185,7 +174,6 @@ nfsd3_proc_write(struct svc_rqst *rqstp)
 {
 	struct nfsd3_writeargs *argp = rqstp->rq_argp;
 	struct nfsd3_writeres *resp = rqstp->rq_resp;
-	__be32	nfserr;
 	unsigned long cnt = argp->len;
 	unsigned int nvecs;
 
@@ -199,13 +187,16 @@ nfsd3_proc_write(struct svc_rqst *rqstp)
 	resp->committed = argp->stable;
 	nvecs = svc_fill_write_vector(rqstp, rqstp->rq_arg.pages,
 				      &argp->first, cnt);
-	if (!nvecs)
-		RETURN_STATUS(nfserr_io);
-	nfserr = nfsd_write(rqstp, &resp->fh, argp->offset,
-			    rqstp->rq_vec, nvecs, &cnt,
-			    resp->committed, resp->verf);
+	if (!nvecs) {
+		resp->status = nfserr_io;
+		goto out;
+	}
+	resp->status = nfsd_write(rqstp, &resp->fh, argp->offset,
+				  rqstp->rq_vec, nvecs, &cnt,
+				  resp->committed, resp->verf);
 	resp->count = cnt;
-	RETURN_STATUS(nfserr);
+out:
+	return resp->status;
 }
 
 /*
@@ -220,7 +211,6 @@ nfsd3_proc_create(struct svc_rqst *rqstp)
 	struct nfsd3_diropres *resp = rqstp->rq_resp;
 	svc_fh		*dirfhp, *newfhp = NULL;
 	struct iattr	*attr;
-	__be32		nfserr;
 
 	dprintk("nfsd: CREATE(3)   %s %.*s\n",
 				SVCFH_fmt(&argp->fh),
@@ -241,11 +231,10 @@ nfsd3_proc_create(struct svc_rqst *rqstp)
 	}
 
 	/* Now create the file and set attributes */
-	nfserr = do_nfsd_create(rqstp, dirfhp, argp->name, argp->len,
-				attr, newfhp,
-				argp->createmode, (u32 *)argp->verf, NULL, NULL);
-
-	RETURN_STATUS(nfserr);
+	resp->status = do_nfsd_create(rqstp, dirfhp, argp->name, argp->len,
+				      attr, newfhp, argp->createmode,
+				      (u32 *)argp->verf, NULL, NULL);
+	return resp->status;
 }
 
 /*
@@ -256,7 +245,6 @@ nfsd3_proc_mkdir(struct svc_rqst *rqstp)
 {
 	struct nfsd3_createargs *argp = rqstp->rq_argp;
 	struct nfsd3_diropres *resp = rqstp->rq_resp;
-	__be32	nfserr;
 
 	dprintk("nfsd: MKDIR(3)    %s %.*s\n",
 				SVCFH_fmt(&argp->fh),
@@ -266,10 +254,10 @@ nfsd3_proc_mkdir(struct svc_rqst *rqstp)
 	argp->attrs.ia_valid &= ~ATTR_SIZE;
 	fh_copy(&resp->dirfh, &argp->fh);
 	fh_init(&resp->fh, NFS3_FHSIZE);
-	nfserr = nfsd_create(rqstp, &resp->dirfh, argp->name, argp->len,
-				    &argp->attrs, S_IFDIR, 0, &resp->fh);
+	resp->status = nfsd_create(rqstp, &resp->dirfh, argp->name, argp->len,
+				   &argp->attrs, S_IFDIR, 0, &resp->fh);
 	fh_unlock(&resp->dirfh);
-	RETURN_STATUS(nfserr);
+	return resp->status;
 }
 
 static __be32
@@ -277,18 +265,23 @@ nfsd3_proc_symlink(struct svc_rqst *rqstp)
 {
 	struct nfsd3_symlinkargs *argp = rqstp->rq_argp;
 	struct nfsd3_diropres *resp = rqstp->rq_resp;
-	__be32	nfserr;
 
-	if (argp->tlen == 0)
-		RETURN_STATUS(nfserr_inval);
-	if (argp->tlen > NFS3_MAXPATHLEN)
-		RETURN_STATUS(nfserr_nametoolong);
+	if (argp->tlen == 0) {
+		resp->status = nfserr_inval;
+		goto out;
+	}
+	if (argp->tlen > NFS3_MAXPATHLEN) {
+		resp->status = nfserr_nametoolong;
+		goto out;
+	}
 
 	argp->tname = svc_fill_symlink_pathname(rqstp, &argp->first,
 						page_address(rqstp->rq_arg.pages[0]),
 						argp->tlen);
-	if (IS_ERR(argp->tname))
-		RETURN_STATUS(nfserrno(PTR_ERR(argp->tname)));
+	if (IS_ERR(argp->tname)) {
+		resp->status = nfserrno(PTR_ERR(argp->tname));
+		goto out;
+	}
 
 	dprintk("nfsd: SYMLINK(3)  %s %.*s -> %.*s\n",
 				SVCFH_fmt(&argp->ffh),
@@ -297,10 +290,11 @@ nfsd3_proc_symlink(struct svc_rqst *rqstp)
 
 	fh_copy(&resp->dirfh, &argp->ffh);
 	fh_init(&resp->fh, NFS3_FHSIZE);
-	nfserr = nfsd_symlink(rqstp, &resp->dirfh, argp->fname, argp->flen,
-						   argp->tname, &resp->fh);
+	resp->status = nfsd_symlink(rqstp, &resp->dirfh, argp->fname,
+				    argp->flen, argp->tname, &resp->fh);
 	kfree(argp->tname);
-	RETURN_STATUS(nfserr);
+out:
+	return resp->status;
 }
 
 /*
@@ -311,7 +305,6 @@ nfsd3_proc_mknod(struct svc_rqst *rqstp)
 {
 	struct nfsd3_mknodargs *argp = rqstp->rq_argp;
 	struct nfsd3_diropres  *resp = rqstp->rq_resp;
-	__be32	nfserr;
 	int type;
 	dev_t	rdev = 0;
 
@@ -323,22 +316,28 @@ nfsd3_proc_mknod(struct svc_rqst *rqstp)
 	fh_copy(&resp->dirfh, &argp->fh);
 	fh_init(&resp->fh, NFS3_FHSIZE);
 
-	if (argp->ftype == 0 || argp->ftype >= NF3BAD)
-		RETURN_STATUS(nfserr_inval);
+	if (argp->ftype == 0 || argp->ftype >= NF3BAD) {
+		resp->status = nfserr_inval;
+		goto out;
+	}
 	if (argp->ftype == NF3CHR || argp->ftype == NF3BLK) {
 		rdev = MKDEV(argp->major, argp->minor);
 		if (MAJOR(rdev) != argp->major ||
-		    MINOR(rdev) != argp->minor)
-			RETURN_STATUS(nfserr_inval);
-	} else
-		if (argp->ftype != NF3SOCK && argp->ftype != NF3FIFO)
-			RETURN_STATUS(nfserr_inval);
+		    MINOR(rdev) != argp->minor) {
+			resp->status = nfserr_inval;
+			goto out;
+		}
+	} else if (argp->ftype != NF3SOCK && argp->ftype != NF3FIFO) {
+		resp->status = nfserr_inval;
+		goto out;
+	}
 
 	type = nfs3_ftypes[argp->ftype];
-	nfserr = nfsd_create(rqstp, &resp->dirfh, argp->name, argp->len,
-				    &argp->attrs, type, rdev, &resp->fh);
+	resp->status = nfsd_create(rqstp, &resp->dirfh, argp->name, argp->len,
+				   &argp->attrs, type, rdev, &resp->fh);
 	fh_unlock(&resp->dirfh);
-	RETURN_STATUS(nfserr);
+out:
+	return resp->status;
 }
 
 /*
@@ -349,7 +348,6 @@ nfsd3_proc_remove(struct svc_rqst *rqstp)
 {
 	struct nfsd3_diropargs *argp = rqstp->rq_argp;
 	struct nfsd3_attrstat *resp = rqstp->rq_resp;
-	__be32	nfserr;
 
 	dprintk("nfsd: REMOVE(3)   %s %.*s\n",
 				SVCFH_fmt(&argp->fh),
@@ -358,9 +356,10 @@ nfsd3_proc_remove(struct svc_rqst *rqstp)
 
 	/* Unlink. -S_IFDIR means file must not be a directory */
 	fh_copy(&resp->fh, &argp->fh);
-	nfserr = nfsd_unlink(rqstp, &resp->fh, -S_IFDIR, argp->name, argp->len);
+	resp->status = nfsd_unlink(rqstp, &resp->fh, -S_IFDIR,
+				   argp->name, argp->len);
 	fh_unlock(&resp->fh);
-	RETURN_STATUS(nfserr);
+	return resp->status;
 }
 
 /*
@@ -371,7 +370,6 @@ nfsd3_proc_rmdir(struct svc_rqst *rqstp)
 {
 	struct nfsd3_diropargs *argp = rqstp->rq_argp;
 	struct nfsd3_attrstat *resp = rqstp->rq_resp;
-	__be32	nfserr;
 
 	dprintk("nfsd: RMDIR(3)    %s %.*s\n",
 				SVCFH_fmt(&argp->fh),
@@ -379,9 +377,10 @@ nfsd3_proc_rmdir(struct svc_rqst *rqstp)
 				argp->name);
 
 	fh_copy(&resp->fh, &argp->fh);
-	nfserr = nfsd_unlink(rqstp, &resp->fh, S_IFDIR, argp->name, argp->len);
+	resp->status = nfsd_unlink(rqstp, &resp->fh, S_IFDIR,
+				   argp->name, argp->len);
 	fh_unlock(&resp->fh);
-	RETURN_STATUS(nfserr);
+	return resp->status;
 }
 
 static __be32
@@ -389,7 +388,6 @@ nfsd3_proc_rename(struct svc_rqst *rqstp)
 {
 	struct nfsd3_renameargs *argp = rqstp->rq_argp;
 	struct nfsd3_renameres *resp = rqstp->rq_resp;
-	__be32	nfserr;
 
 	dprintk("nfsd: RENAME(3)   %s %.*s ->\n",
 				SVCFH_fmt(&argp->ffh),
@@ -402,9 +400,9 @@ nfsd3_proc_rename(struct svc_rqst *rqstp)
 
 	fh_copy(&resp->ffh, &argp->ffh);
 	fh_copy(&resp->tfh, &argp->tfh);
-	nfserr = nfsd_rename(rqstp, &resp->ffh, argp->fname, argp->flen,
-				    &resp->tfh, argp->tname, argp->tlen);
-	RETURN_STATUS(nfserr);
+	resp->status = nfsd_rename(rqstp, &resp->ffh, argp->fname, argp->flen,
+				   &resp->tfh, argp->tname, argp->tlen);
+	return resp->status;
 }
 
 static __be32
@@ -412,7 +410,6 @@ nfsd3_proc_link(struct svc_rqst *rqstp)
 {
 	struct nfsd3_linkargs *argp = rqstp->rq_argp;
 	struct nfsd3_linkres  *resp = rqstp->rq_resp;
-	__be32	nfserr;
 
 	dprintk("nfsd: LINK(3)     %s ->\n",
 				SVCFH_fmt(&argp->ffh));
@@ -423,9 +420,9 @@ nfsd3_proc_link(struct svc_rqst *rqstp)
 
 	fh_copy(&resp->fh,  &argp->ffh);
 	fh_copy(&resp->tfh, &argp->tfh);
-	nfserr = nfsd_link(rqstp, &resp->tfh, argp->tname, argp->tlen,
-				  &resp->fh);
-	RETURN_STATUS(nfserr);
+	resp->status = nfsd_link(rqstp, &resp->tfh, argp->tname, argp->tlen,
+				 &resp->fh);
+	return resp->status;
 }
 
 /*
@@ -436,7 +433,6 @@ nfsd3_proc_readdir(struct svc_rqst *rqstp)
 {
 	struct nfsd3_readdirargs *argp = rqstp->rq_argp;
 	struct nfsd3_readdirres  *resp = rqstp->rq_resp;
-	__be32		nfserr;
 	int		count = 0;
 	struct page	**p;
 	caddr_t		page_addr = NULL;
@@ -456,8 +452,8 @@ nfsd3_proc_readdir(struct svc_rqst *rqstp)
 	resp->common.err = nfs_ok;
 	resp->buffer = argp->buffer;
 	resp->rqstp = rqstp;
-	nfserr = nfsd_readdir(rqstp, &resp->fh, (loff_t*) &argp->cookie, 
-					&resp->common, nfs3svc_encode_entry);
+	resp->status = nfsd_readdir(rqstp, &resp->fh, (loff_t *)&argp->cookie,
+				    &resp->common, nfs3svc_encode_entry);
 	memcpy(resp->verf, argp->verf, 8);
 	count = 0;
 	for (p = rqstp->rq_respages + 1; p < rqstp->rq_next_page; p++) {
@@ -485,7 +481,7 @@ nfsd3_proc_readdir(struct svc_rqst *rqstp)
 		resp->offset = NULL;
 	}
 
-	RETURN_STATUS(nfserr);
+	return resp->status;
 }
 
 /*
@@ -497,7 +493,6 @@ nfsd3_proc_readdirplus(struct svc_rqst *rqstp)
 {
 	struct nfsd3_readdirargs *argp = rqstp->rq_argp;
 	struct nfsd3_readdirres  *resp = rqstp->rq_resp;
-	__be32	nfserr;
 	int	count = 0;
 	loff_t	offset;
 	struct page **p;
@@ -520,17 +515,17 @@ nfsd3_proc_readdirplus(struct svc_rqst *rqstp)
 	resp->rqstp = rqstp;
 	offset = argp->cookie;
 
-	nfserr = fh_verify(rqstp, &resp->fh, S_IFDIR, NFSD_MAY_NOP);
-	if (nfserr)
-		RETURN_STATUS(nfserr);
+	resp->status = fh_verify(rqstp, &resp->fh, S_IFDIR, NFSD_MAY_NOP);
+	if (resp->status != nfs_ok)
+		goto out;
 
-	if (resp->fh.fh_export->ex_flags & NFSEXP_NOREADDIRPLUS)
-		RETURN_STATUS(nfserr_notsupp);
+	if (resp->fh.fh_export->ex_flags & NFSEXP_NOREADDIRPLUS) {
+		resp->status = nfserr_notsupp;
+		goto out;
+	}
 
-	nfserr = nfsd_readdir(rqstp, &resp->fh,
-				     &offset,
-				     &resp->common,
-				     nfs3svc_encode_entry_plus);
+	resp->status = nfsd_readdir(rqstp, &resp->fh, &offset,
+				    &resp->common, nfs3svc_encode_entry_plus);
 	memcpy(resp->verf, argp->verf, 8);
 	for (p = rqstp->rq_respages + 1; p < rqstp->rq_next_page; p++) {
 		page_addr = page_address(*p);
@@ -555,7 +550,8 @@ nfsd3_proc_readdirplus(struct svc_rqst *rqstp)
 		resp->offset = NULL;
 	}
 
-	RETURN_STATUS(nfserr);
+out:
+	return resp->status;
 }
 
 /*
@@ -566,14 +562,13 @@ nfsd3_proc_fsstat(struct svc_rqst *rqstp)
 {
 	struct nfsd_fhandle *argp = rqstp->rq_argp;
 	struct nfsd3_fsstatres *resp = rqstp->rq_resp;
-	__be32	nfserr;
 
 	dprintk("nfsd: FSSTAT(3)   %s\n",
 				SVCFH_fmt(&argp->fh));
 
-	nfserr = nfsd_statfs(rqstp, &argp->fh, &resp->stats, 0);
+	resp->status = nfsd_statfs(rqstp, &argp->fh, &resp->stats, 0);
 	fh_put(&argp->fh);
-	RETURN_STATUS(nfserr);
+	return resp->status;
 }
 
 /*
@@ -584,7 +579,6 @@ nfsd3_proc_fsinfo(struct svc_rqst *rqstp)
 {
 	struct nfsd_fhandle *argp = rqstp->rq_argp;
 	struct nfsd3_fsinfores *resp = rqstp->rq_resp;
-	__be32	nfserr;
 	u32	max_blocksize = svc_max_payload(rqstp);
 
 	dprintk("nfsd: FSINFO(3)   %s\n",
@@ -600,13 +594,13 @@ nfsd3_proc_fsinfo(struct svc_rqst *rqstp)
 	resp->f_maxfilesize = ~(u32) 0;
 	resp->f_properties = NFS3_FSF_DEFAULT;
 
-	nfserr = fh_verify(rqstp, &argp->fh, 0,
-			NFSD_MAY_NOP | NFSD_MAY_BYPASS_GSS_ON_ROOT);
+	resp->status = fh_verify(rqstp, &argp->fh, 0,
+				 NFSD_MAY_NOP | NFSD_MAY_BYPASS_GSS_ON_ROOT);
 
 	/* Check special features of the file system. May request
 	 * different read/write sizes for file systems known to have
 	 * problems with large blocks */
-	if (nfserr == 0) {
+	if (resp->status == nfs_ok) {
 		struct super_block *sb = argp->fh.fh_dentry->d_sb;
 
 		/* Note that we don't care for remote fs's here */
@@ -617,7 +611,7 @@ nfsd3_proc_fsinfo(struct svc_rqst *rqstp)
 	}
 
 	fh_put(&argp->fh);
-	RETURN_STATUS(nfserr);
+	return resp->status;
 }
 
 /*
@@ -628,7 +622,6 @@ nfsd3_proc_pathconf(struct svc_rqst *rqstp)
 {
 	struct nfsd_fhandle *argp = rqstp->rq_argp;
 	struct nfsd3_pathconfres *resp = rqstp->rq_resp;
-	__be32	nfserr;
 
 	dprintk("nfsd: PATHCONF(3) %s\n",
 				SVCFH_fmt(&argp->fh));
@@ -641,9 +634,9 @@ nfsd3_proc_pathconf(struct svc_rqst *rqstp)
 	resp->p_case_insensitive = 0;
 	resp->p_case_preserving = 1;
 
-	nfserr = fh_verify(rqstp, &argp->fh, 0, NFSD_MAY_NOP);
+	resp->status = fh_verify(rqstp, &argp->fh, 0, NFSD_MAY_NOP);
 
-	if (nfserr == 0) {
+	if (resp->status == nfs_ok) {
 		struct super_block *sb = argp->fh.fh_dentry->d_sb;
 
 		/* Note that we don't care for remote fs's here */
@@ -660,10 +653,9 @@ nfsd3_proc_pathconf(struct svc_rqst *rqstp)
 	}
 
 	fh_put(&argp->fh);
-	RETURN_STATUS(nfserr);
+	return resp->status;
 }
 
-
 /*
  * Commit a file (range) to stable storage.
  */
@@ -672,21 +664,22 @@ nfsd3_proc_commit(struct svc_rqst *rqstp)
 {
 	struct nfsd3_commitargs *argp = rqstp->rq_argp;
 	struct nfsd3_commitres *resp = rqstp->rq_resp;
-	__be32	nfserr;
 
 	dprintk("nfsd: COMMIT(3)   %s %u@%Lu\n",
 				SVCFH_fmt(&argp->fh),
 				argp->count,
 				(unsigned long long) argp->offset);
 
-	if (argp->offset > NFS_OFFSET_MAX)
-		RETURN_STATUS(nfserr_inval);
+	if (argp->offset > NFS_OFFSET_MAX) {
+		resp->status = nfserr_inval;
+		goto out;
+	}
 
 	fh_copy(&resp->fh, &argp->fh);
-	nfserr = nfsd_commit(rqstp, &resp->fh, argp->offset, argp->count,
-			resp->verf);
-
-	RETURN_STATUS(nfserr);
+	resp->status = nfsd_commit(rqstp, &resp->fh, argp->offset,
+				   argp->count, resp->verf);
+out:
+	return resp->status;
 }
 
 



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

* [PATCH v3 14/15] NFSD: Map nfserr_wrongsec outside of nfsd_dispatch
  2020-10-01 22:58 [PATCH v3 00/15] nfsd_dispatch() clean up Chuck Lever
                   ` (12 preceding siblings ...)
  2020-10-01 22:59 ` [PATCH v3 13/15] NFSD: Remove the RETURN_STATUS() macro Chuck Lever
@ 2020-10-01 23:00 ` Chuck Lever
  2020-10-01 23:00 ` [PATCH v3 15/15] NFSD: Hoist status code encoding into XDR encoder functions Chuck Lever
  2020-10-02 17:39 ` [PATCH v3 00/15] nfsd_dispatch() clean up J. Bruce Fields
  15 siblings, 0 replies; 19+ messages in thread
From: Chuck Lever @ 2020-10-01 23:00 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

Refactor: Handle this NFS version-specific mapping in the only
place where nfserr_wrongsec is generated.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/export.c |    2 +-
 fs/nfsd/nfssvc.c |    2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index cb777fe82988..21e404e7cb68 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -1002,7 +1002,7 @@ __be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp)
 	if (nfsd4_spo_must_allow(rqstp))
 		return 0;
 
-	return nfserr_wrongsec;
+	return rqstp->rq_vers < 4 ? nfserr_acces : nfserr_wrongsec;
 }
 
 /*
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index 4aa8db879ca2..beb3875241cb 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -964,8 +964,6 @@ static __be32 map_new_errors(u32 vers, __be32 nfserr)
 {
 	if (nfserr == nfserr_jukebox && vers == 2)
 		return nfserr_dropit;
-	if (nfserr == nfserr_wrongsec && vers < 4)
-		return nfserr_acces;
 	return nfserr;
 }
 



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

* [PATCH v3 15/15] NFSD: Hoist status code encoding into XDR encoder functions
  2020-10-01 22:58 [PATCH v3 00/15] nfsd_dispatch() clean up Chuck Lever
                   ` (13 preceding siblings ...)
  2020-10-01 23:00 ` [PATCH v3 14/15] NFSD: Map nfserr_wrongsec outside of nfsd_dispatch Chuck Lever
@ 2020-10-01 23:00 ` Chuck Lever
  2020-10-02 17:39 ` [PATCH v3 00/15] nfsd_dispatch() clean up J. Bruce Fields
  15 siblings, 0 replies; 19+ messages in thread
From: Chuck Lever @ 2020-10-01 23:00 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

The original intent was presumably to reduce code duplication. The
trade-off was:

- No support for an NFSD proc function returning a non-success
  RPC accept_stat value.
- No support for void NFS replies to non-NULL procedures.
- Everyone pays for the deduplication with a few extra conditional
  branches in a hot path.

In addition, nfsd_dispatch() leaves *statp uninitialized in the
success path, unlike svc_generic_dispatch().

Address all of these problems by moving the logic for encoding
the NFS status code into the NFS XDR encoders themselves. Then
update the NFS .pc_func methods to return an RPC accept_stat
value.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfs2acl.c  |   19 ++++++---
 fs/nfsd/nfs3acl.c  |    9 ++--
 fs/nfsd/nfs3proc.c |   44 ++++++++++-----------
 fs/nfsd/nfs3xdr.c  |   19 +++++++--
 fs/nfsd/nfs4proc.c |    5 +-
 fs/nfsd/nfs4xdr.c  |    5 +-
 fs/nfsd/nfsproc.c  |  111 ++++++++++++++++++++++++++--------------------------
 fs/nfsd/nfssvc.c   |   21 ++--------
 fs/nfsd/nfsxdr.c   |   23 +++++++++--
 fs/nfsd/xdr.h      |    5 ++
 10 files changed, 143 insertions(+), 118 deletions(-)

diff --git a/fs/nfsd/nfs2acl.c b/fs/nfsd/nfs2acl.c
index 6f46afdb0616..6a900f770dd2 100644
--- a/fs/nfsd/nfs2acl.c
+++ b/fs/nfsd/nfs2acl.c
@@ -21,7 +21,7 @@
 static __be32
 nfsacld_proc_null(struct svc_rqst *rqstp)
 {
-	return nfs_ok;
+	return rpc_success;
 }
 
 /*
@@ -79,7 +79,7 @@ static __be32 nfsacld_proc_getacl(struct svc_rqst *rqstp)
 
 	/* resp->acl_{access,default} are released in nfssvc_release_getacl. */
 out:
-	return resp->status;
+	return rpc_success;
 
 fail:
 	posix_acl_release(resp->acl_access);
@@ -131,7 +131,7 @@ static __be32 nfsacld_proc_setacl(struct svc_rqst *rqstp)
 	   nfssvc_decode_setaclargs. */
 	posix_acl_release(argp->acl_access);
 	posix_acl_release(argp->acl_default);
-	return resp->status;
+	return rpc_success;
 
 out_drop_lock:
 	fh_unlock(fh);
@@ -157,7 +157,7 @@ static __be32 nfsacld_proc_getattr(struct svc_rqst *rqstp)
 		goto out;
 	resp->status = fh_getattr(&resp->fh, &resp->stat);
 out:
-	return resp->status;
+	return rpc_success;
 }
 
 /*
@@ -179,7 +179,7 @@ static __be32 nfsacld_proc_access(struct svc_rqst *rqstp)
 		goto out;
 	resp->status = fh_getattr(&resp->fh, &resp->stat);
 out:
-	return resp->status;
+	return rpc_success;
 }
 
 /*
@@ -275,6 +275,7 @@ static int nfsaclsvc_encode_getaclres(struct svc_rqst *rqstp, __be32 *p)
 	int n;
 	int w;
 
+	*p++ = resp->status;
 	if (resp->status != nfs_ok)
 		return xdr_ressize_check(rqstp, p);
 
@@ -317,10 +318,12 @@ static int nfsaclsvc_encode_attrstatres(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd_attrstat *resp = rqstp->rq_resp;
 
+	*p++ = resp->status;
 	if (resp->status != nfs_ok)
-		return xdr_ressize_check(rqstp, p);
+		goto out;
 
 	p = nfs2svc_encode_fattr(rqstp, p, &resp->fh, &resp->stat);
+out:
 	return xdr_ressize_check(rqstp, p);
 }
 
@@ -329,11 +332,13 @@ static int nfsaclsvc_encode_accessres(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd3_accessres *resp = rqstp->rq_resp;
 
+	*p++ = resp->status;
 	if (resp->status != nfs_ok)
-		return xdr_ressize_check(rqstp, p);
+		goto out;
 
 	p = nfs2svc_encode_fattr(rqstp, p, &resp->fh, &resp->stat);
 	*p++ = htonl(resp->access);
+out:
 	return xdr_ressize_check(rqstp, p);
 }
 
diff --git a/fs/nfsd/nfs3acl.c b/fs/nfsd/nfs3acl.c
index 3fee24dee98c..34a394e50e1d 100644
--- a/fs/nfsd/nfs3acl.c
+++ b/fs/nfsd/nfs3acl.c
@@ -19,7 +19,7 @@
 static __be32
 nfsd3_proc_null(struct svc_rqst *rqstp)
 {
-	return nfs_ok;
+	return rpc_success;
 }
 
 /*
@@ -71,7 +71,7 @@ static __be32 nfsd3_proc_getacl(struct svc_rqst *rqstp)
 
 	/* resp->acl_{access,default} are released in nfs3svc_release_getacl. */
 out:
-	return resp->status;
+	return rpc_success;
 
 fail:
 	posix_acl_release(resp->acl_access);
@@ -118,7 +118,7 @@ static __be32 nfsd3_proc_setacl(struct svc_rqst *rqstp)
 	   nfs3svc_decode_setaclargs. */
 	posix_acl_release(argp->acl_access);
 	posix_acl_release(argp->acl_default);
-	return resp->status;
+	return rpc_success;
 }
 
 /*
@@ -173,6 +173,7 @@ static int nfs3svc_encode_getaclres(struct svc_rqst *rqstp, __be32 *p)
 	struct nfsd3_getaclres *resp = rqstp->rq_resp;
 	struct dentry *dentry = resp->fh.fh_dentry;
 
+	*p++ = resp->status;
 	p = nfs3svc_encode_post_op_attr(rqstp, p, &resp->fh);
 	if (resp->status == 0 && dentry && d_really_is_positive(dentry)) {
 		struct inode *inode = d_inode(dentry);
@@ -217,8 +218,8 @@ static int nfs3svc_encode_setaclres(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd3_attrstat *resp = rqstp->rq_resp;
 
+	*p++ = resp->status;
 	p = nfs3svc_encode_post_op_attr(rqstp, p, &resp->fh);
-
 	return xdr_ressize_check(rqstp, p);
 }
 
diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
index 1d2c149e5ff4..14468613d150 100644
--- a/fs/nfsd/nfs3proc.c
+++ b/fs/nfsd/nfs3proc.c
@@ -32,7 +32,7 @@ static int	nfs3_ftypes[] = {
 static __be32
 nfsd3_proc_null(struct svc_rqst *rqstp)
 {
-	return nfs_ok;
+	return rpc_success;
 }
 
 /*
@@ -55,7 +55,7 @@ nfsd3_proc_getattr(struct svc_rqst *rqstp)
 
 	resp->status = fh_getattr(&resp->fh, &resp->stat);
 out:
-	return resp->status;
+	return rpc_success;
 }
 
 /*
@@ -73,7 +73,7 @@ nfsd3_proc_setattr(struct svc_rqst *rqstp)
 	fh_copy(&resp->fh, &argp->fh);
 	resp->status = nfsd_setattr(rqstp, &resp->fh, &argp->attrs,
 				    argp->check_guard, argp->guardtime);
-	return resp->status;
+	return rpc_success;
 }
 
 /*
@@ -96,7 +96,7 @@ nfsd3_proc_lookup(struct svc_rqst *rqstp)
 	resp->status = nfsd_lookup(rqstp, &resp->dirfh,
 				   argp->name, argp->len,
 				   &resp->fh);
-	return resp->status;
+	return rpc_success;
 }
 
 /*
@@ -115,7 +115,7 @@ nfsd3_proc_access(struct svc_rqst *rqstp)
 	fh_copy(&resp->fh, &argp->fh);
 	resp->access = argp->access;
 	resp->status = nfsd_access(rqstp, &resp->fh, &resp->access, NULL);
-	return resp->status;
+	return rpc_success;
 }
 
 /*
@@ -133,7 +133,7 @@ nfsd3_proc_readlink(struct svc_rqst *rqstp)
 	fh_copy(&resp->fh, &argp->fh);
 	resp->len = NFS3_MAXPATHLEN;
 	resp->status = nfsd_readlink(rqstp, &resp->fh, argp->buffer, &resp->len);
-	return resp->status;
+	return rpc_success;
 }
 
 /*
@@ -163,7 +163,7 @@ nfsd3_proc_read(struct svc_rqst *rqstp)
 	resp->status = nfsd_read(rqstp, &resp->fh, argp->offset,
 				 rqstp->rq_vec, argp->vlen, &resp->count,
 				 &resp->eof);
-	return resp->status;
+	return rpc_success;
 }
 
 /*
@@ -196,7 +196,7 @@ nfsd3_proc_write(struct svc_rqst *rqstp)
 				  resp->committed, resp->verf);
 	resp->count = cnt;
 out:
-	return resp->status;
+	return rpc_success;
 }
 
 /*
@@ -234,7 +234,7 @@ nfsd3_proc_create(struct svc_rqst *rqstp)
 	resp->status = do_nfsd_create(rqstp, dirfhp, argp->name, argp->len,
 				      attr, newfhp, argp->createmode,
 				      (u32 *)argp->verf, NULL, NULL);
-	return resp->status;
+	return rpc_success;
 }
 
 /*
@@ -257,7 +257,7 @@ nfsd3_proc_mkdir(struct svc_rqst *rqstp)
 	resp->status = nfsd_create(rqstp, &resp->dirfh, argp->name, argp->len,
 				   &argp->attrs, S_IFDIR, 0, &resp->fh);
 	fh_unlock(&resp->dirfh);
-	return resp->status;
+	return rpc_success;
 }
 
 static __be32
@@ -294,7 +294,7 @@ nfsd3_proc_symlink(struct svc_rqst *rqstp)
 				    argp->flen, argp->tname, &resp->fh);
 	kfree(argp->tname);
 out:
-	return resp->status;
+	return rpc_success;
 }
 
 /*
@@ -337,7 +337,7 @@ nfsd3_proc_mknod(struct svc_rqst *rqstp)
 				   &argp->attrs, type, rdev, &resp->fh);
 	fh_unlock(&resp->dirfh);
 out:
-	return resp->status;
+	return rpc_success;
 }
 
 /*
@@ -359,7 +359,7 @@ nfsd3_proc_remove(struct svc_rqst *rqstp)
 	resp->status = nfsd_unlink(rqstp, &resp->fh, -S_IFDIR,
 				   argp->name, argp->len);
 	fh_unlock(&resp->fh);
-	return resp->status;
+	return rpc_success;
 }
 
 /*
@@ -380,7 +380,7 @@ nfsd3_proc_rmdir(struct svc_rqst *rqstp)
 	resp->status = nfsd_unlink(rqstp, &resp->fh, S_IFDIR,
 				   argp->name, argp->len);
 	fh_unlock(&resp->fh);
-	return resp->status;
+	return rpc_success;
 }
 
 static __be32
@@ -402,7 +402,7 @@ nfsd3_proc_rename(struct svc_rqst *rqstp)
 	fh_copy(&resp->tfh, &argp->tfh);
 	resp->status = nfsd_rename(rqstp, &resp->ffh, argp->fname, argp->flen,
 				   &resp->tfh, argp->tname, argp->tlen);
-	return resp->status;
+	return rpc_success;
 }
 
 static __be32
@@ -422,7 +422,7 @@ nfsd3_proc_link(struct svc_rqst *rqstp)
 	fh_copy(&resp->tfh, &argp->tfh);
 	resp->status = nfsd_link(rqstp, &resp->tfh, argp->tname, argp->tlen,
 				 &resp->fh);
-	return resp->status;
+	return rpc_success;
 }
 
 /*
@@ -481,7 +481,7 @@ nfsd3_proc_readdir(struct svc_rqst *rqstp)
 		resp->offset = NULL;
 	}
 
-	return resp->status;
+	return rpc_success;
 }
 
 /*
@@ -551,7 +551,7 @@ nfsd3_proc_readdirplus(struct svc_rqst *rqstp)
 	}
 
 out:
-	return resp->status;
+	return rpc_success;
 }
 
 /*
@@ -568,7 +568,7 @@ nfsd3_proc_fsstat(struct svc_rqst *rqstp)
 
 	resp->status = nfsd_statfs(rqstp, &argp->fh, &resp->stats, 0);
 	fh_put(&argp->fh);
-	return resp->status;
+	return rpc_success;
 }
 
 /*
@@ -611,7 +611,7 @@ nfsd3_proc_fsinfo(struct svc_rqst *rqstp)
 	}
 
 	fh_put(&argp->fh);
-	return resp->status;
+	return rpc_success;
 }
 
 /*
@@ -653,7 +653,7 @@ nfsd3_proc_pathconf(struct svc_rqst *rqstp)
 	}
 
 	fh_put(&argp->fh);
-	return resp->status;
+	return rpc_success;
 }
 
 /*
@@ -679,7 +679,7 @@ nfsd3_proc_commit(struct svc_rqst *rqstp)
 	resp->status = nfsd_commit(rqstp, &resp->fh, argp->offset,
 				   argp->count, resp->verf);
 out:
-	return resp->status;
+	return rpc_success;
 }
 
 
diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
index e540fd1a29d8..9c23b6acf234 100644
--- a/fs/nfsd/nfs3xdr.c
+++ b/fs/nfsd/nfs3xdr.c
@@ -641,10 +641,7 @@ nfs3svc_decode_commitargs(struct svc_rqst *rqstp, __be32 *p)
 /*
  * XDR encode functions
  */
-/*
- * There must be an encoding function for void results so svc_process
- * will work properly.
- */
+
 int
 nfs3svc_encode_voidres(struct svc_rqst *rqstp, __be32 *p)
 {
@@ -657,6 +654,7 @@ nfs3svc_encode_attrstat(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd3_attrstat *resp = rqstp->rq_resp;
 
+	*p++ = resp->status;
 	if (resp->status == 0) {
 		lease_get_mtime(d_inode(resp->fh.fh_dentry),
 				&resp->stat.mtime);
@@ -671,6 +669,7 @@ nfs3svc_encode_wccstat(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd3_attrstat *resp = rqstp->rq_resp;
 
+	*p++ = resp->status;
 	p = encode_wcc_data(rqstp, p, &resp->fh);
 	return xdr_ressize_check(rqstp, p);
 }
@@ -681,6 +680,7 @@ nfs3svc_encode_diropres(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd3_diropres *resp = rqstp->rq_resp;
 
+	*p++ = resp->status;
 	if (resp->status == 0) {
 		p = encode_fh(p, &resp->fh);
 		p = encode_post_op_attr(rqstp, p, &resp->fh);
@@ -695,6 +695,7 @@ nfs3svc_encode_accessres(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd3_accessres *resp = rqstp->rq_resp;
 
+	*p++ = resp->status;
 	p = encode_post_op_attr(rqstp, p, &resp->fh);
 	if (resp->status == 0)
 		*p++ = htonl(resp->access);
@@ -707,6 +708,7 @@ nfs3svc_encode_readlinkres(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd3_readlinkres *resp = rqstp->rq_resp;
 
+	*p++ = resp->status;
 	p = encode_post_op_attr(rqstp, p, &resp->fh);
 	if (resp->status == 0) {
 		*p++ = htonl(resp->len);
@@ -729,6 +731,7 @@ nfs3svc_encode_readres(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd3_readres *resp = rqstp->rq_resp;
 
+	*p++ = resp->status;
 	p = encode_post_op_attr(rqstp, p, &resp->fh);
 	if (resp->status == 0) {
 		*p++ = htonl(resp->count);
@@ -754,6 +757,7 @@ nfs3svc_encode_writeres(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd3_writeres *resp = rqstp->rq_resp;
 
+	*p++ = resp->status;
 	p = encode_wcc_data(rqstp, p, &resp->fh);
 	if (resp->status == 0) {
 		*p++ = htonl(resp->count);
@@ -770,6 +774,7 @@ nfs3svc_encode_createres(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd3_diropres *resp = rqstp->rq_resp;
 
+	*p++ = resp->status;
 	if (resp->status == 0) {
 		*p++ = xdr_one;
 		p = encode_fh(p, &resp->fh);
@@ -785,6 +790,7 @@ nfs3svc_encode_renameres(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd3_renameres *resp = rqstp->rq_resp;
 
+	*p++ = resp->status;
 	p = encode_wcc_data(rqstp, p, &resp->ffh);
 	p = encode_wcc_data(rqstp, p, &resp->tfh);
 	return xdr_ressize_check(rqstp, p);
@@ -796,6 +802,7 @@ nfs3svc_encode_linkres(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd3_linkres *resp = rqstp->rq_resp;
 
+	*p++ = resp->status;
 	p = encode_post_op_attr(rqstp, p, &resp->fh);
 	p = encode_wcc_data(rqstp, p, &resp->tfh);
 	return xdr_ressize_check(rqstp, p);
@@ -807,6 +814,7 @@ nfs3svc_encode_readdirres(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd3_readdirres *resp = rqstp->rq_resp;
 
+	*p++ = resp->status;
 	p = encode_post_op_attr(rqstp, p, &resp->fh);
 
 	if (resp->status == 0) {
@@ -1059,6 +1067,7 @@ nfs3svc_encode_fsstatres(struct svc_rqst *rqstp, __be32 *p)
 	struct kstatfs	*s = &resp->stats;
 	u64		bs = s->f_bsize;
 
+	*p++ = resp->status;
 	*p++ = xdr_zero;	/* no post_op_attr */
 
 	if (resp->status == 0) {
@@ -1079,6 +1088,7 @@ nfs3svc_encode_fsinfores(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd3_fsinfores *resp = rqstp->rq_resp;
 
+	*p++ = resp->status;
 	*p++ = xdr_zero;	/* no post_op_attr */
 
 	if (resp->status == 0) {
@@ -1124,6 +1134,7 @@ nfs3svc_encode_commitres(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd3_commitres *resp = rqstp->rq_resp;
 
+	*p++ = resp->status;
 	p = encode_wcc_data(rqstp, p, &resp->fh);
 	/* Write verifier */
 	if (resp->status == 0) {
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index b99c050797db..efef9ebe652f 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -2165,7 +2165,7 @@ nfsd4_removexattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 static __be32
 nfsd4_proc_null(struct svc_rqst *rqstp)
 {
-	return nfs_ok;
+	return rpc_success;
 }
 
 static inline void nfsd4_increment_op_stats(u32 opnum)
@@ -2464,8 +2464,7 @@ nfsd4_proc_compound(struct svc_rqst *rqstp)
 out:
 	/* Reset deferral mechanism for RPC deferrals */
 	set_bit(RQ_USEDEFERRAL, &rqstp->rq_flags);
-	dprintk("nfsv4 compound returned %d\n", ntohl(status));
-	return status;
+	return rpc_success;
 }
 
 #define op_encode_hdr_size		(2)
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 758d8154a5b3..073f8be7555c 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -5189,15 +5189,14 @@ nfs4svc_decode_compoundargs(struct svc_rqst *rqstp, __be32 *p)
 int
 nfs4svc_encode_compoundres(struct svc_rqst *rqstp, __be32 *p)
 {
-	/*
-	 * All that remains is to write the tag and operation count...
-	 */
 	struct nfsd4_compoundres *resp = rqstp->rq_resp;
 	struct xdr_buf *buf = resp->xdr.buf;
 
 	WARN_ON_ONCE(buf->len != buf->head[0].iov_len + buf->page_len +
 				 buf->tail[0].iov_len);
 
+	*p = resp->cstate.status;
+
 	rqstp->rq_next_page = resp->xdr.page_ptr + 1;
 
 	p = resp->tagp;
diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c
index 526170319ecf..f2450c719032 100644
--- a/fs/nfsd/nfsproc.c
+++ b/fs/nfsd/nfsproc.c
@@ -16,7 +16,7 @@
 static __be32
 nfsd_proc_null(struct svc_rqst *rqstp)
 {
-	return nfs_ok;
+	return rpc_success;
 }
 
 /*
@@ -38,7 +38,7 @@ nfsd_proc_getattr(struct svc_rqst *rqstp)
 		goto out;
 	resp->status = fh_getattr(&resp->fh, &resp->stat);
 out:
-	return resp->status;
+	return rpc_success;
 }
 
 /*
@@ -106,14 +106,14 @@ nfsd_proc_setattr(struct svc_rqst *rqstp)
 
 	resp->status = fh_getattr(&resp->fh, &resp->stat);
 out:
-	return resp->status;
+	return rpc_success;
 }
 
 /* Obsolete, replaced by MNTPROC_MNT. */
 static __be32
 nfsd_proc_root(struct svc_rqst *rqstp)
 {
-	return nfs_ok;
+	return rpc_success;
 }
 
 /*
@@ -140,7 +140,7 @@ nfsd_proc_lookup(struct svc_rqst *rqstp)
 
 	resp->status = fh_getattr(&resp->fh, &resp->stat);
 out:
-	return resp->status;
+	return rpc_success;
 }
 
 /*
@@ -159,7 +159,7 @@ nfsd_proc_readlink(struct svc_rqst *rqstp)
 	resp->status = nfsd_readlink(rqstp, &argp->fh, argp->buffer, &resp->len);
 
 	fh_put(&argp->fh);
-	return resp->status;
+	return rpc_success;
 }
 
 /*
@@ -197,19 +197,18 @@ nfsd_proc_read(struct svc_rqst *rqstp)
 				 rqstp->rq_vec, argp->vlen,
 				 &resp->count,
 				 &eof);
-	if (resp->status != nfs_ok)
-		goto out;
-
-	resp->status = fh_getattr(&resp->fh, &resp->stat);
-out:
-	return resp->status;
+	if (resp->status == nfs_ok)
+		resp->status = fh_getattr(&resp->fh, &resp->stat);
+	else if (resp->status == nfserr_jukebox)
+		return rpc_drop_reply;
+	return rpc_success;
 }
 
 /* Reserved */
 static __be32
 nfsd_proc_writecache(struct svc_rqst *rqstp)
 {
-	return nfs_ok;
+	return rpc_success;
 }
 
 /*
@@ -238,12 +237,12 @@ nfsd_proc_write(struct svc_rqst *rqstp)
 	resp->status = nfsd_write(rqstp, fh_copy(&resp->fh, &argp->fh),
 				  argp->offset, rqstp->rq_vec, nvecs,
 				  &cnt, NFS_DATA_SYNC, NULL);
-	if (resp->status != nfs_ok)
-		goto out;
-
-	resp->status = fh_getattr(&resp->fh, &resp->stat);
+	if (resp->status == nfs_ok)
+		resp->status = fh_getattr(&resp->fh, &resp->stat);
+	else if (resp->status == nfserr_jukebox)
+		return rpc_drop_reply;
 out:
-	return resp->status;
+	return rpc_success;
 }
 
 /*
@@ -410,47 +409,48 @@ nfsd_proc_create(struct svc_rqst *rqstp)
 		goto out;
 	resp->status = fh_getattr(&resp->fh, &resp->stat);
 out:
-	return resp->status;
+	return rpc_success;
 }
 
 static __be32
 nfsd_proc_remove(struct svc_rqst *rqstp)
 {
 	struct nfsd_diropargs *argp = rqstp->rq_argp;
-	__be32	nfserr;
+	struct nfsd_stat *resp = rqstp->rq_resp;
 
 	dprintk("nfsd: REMOVE   %s %.*s\n", SVCFH_fmt(&argp->fh),
 		argp->len, argp->name);
 
 	/* Unlink. -SIFDIR means file must not be a directory */
-	nfserr = nfsd_unlink(rqstp, &argp->fh, -S_IFDIR, argp->name, argp->len);
+	resp->status = nfsd_unlink(rqstp, &argp->fh, -S_IFDIR,
+				   argp->name, argp->len);
 	fh_put(&argp->fh);
-	return nfserr;
+	return rpc_success;
 }
 
 static __be32
 nfsd_proc_rename(struct svc_rqst *rqstp)
 {
 	struct nfsd_renameargs *argp = rqstp->rq_argp;
-	__be32	nfserr;
+	struct nfsd_stat *resp = rqstp->rq_resp;
 
 	dprintk("nfsd: RENAME   %s %.*s -> \n",
 		SVCFH_fmt(&argp->ffh), argp->flen, argp->fname);
 	dprintk("nfsd:        ->  %s %.*s\n",
 		SVCFH_fmt(&argp->tfh), argp->tlen, argp->tname);
 
-	nfserr = nfsd_rename(rqstp, &argp->ffh, argp->fname, argp->flen,
-				    &argp->tfh, argp->tname, argp->tlen);
+	resp->status = nfsd_rename(rqstp, &argp->ffh, argp->fname, argp->flen,
+				   &argp->tfh, argp->tname, argp->tlen);
 	fh_put(&argp->ffh);
 	fh_put(&argp->tfh);
-	return nfserr;
+	return rpc_success;
 }
 
 static __be32
 nfsd_proc_link(struct svc_rqst *rqstp)
 {
 	struct nfsd_linkargs *argp = rqstp->rq_argp;
-	__be32	nfserr;
+	struct nfsd_stat *resp = rqstp->rq_resp;
 
 	dprintk("nfsd: LINK     %s ->\n",
 		SVCFH_fmt(&argp->ffh));
@@ -459,22 +459,22 @@ nfsd_proc_link(struct svc_rqst *rqstp)
 		argp->tlen,
 		argp->tname);
 
-	nfserr = nfsd_link(rqstp, &argp->tfh, argp->tname, argp->tlen,
-				  &argp->ffh);
+	resp->status = nfsd_link(rqstp, &argp->tfh, argp->tname, argp->tlen,
+				 &argp->ffh);
 	fh_put(&argp->ffh);
 	fh_put(&argp->tfh);
-	return nfserr;
+	return rpc_success;
 }
 
 static __be32
 nfsd_proc_symlink(struct svc_rqst *rqstp)
 {
 	struct nfsd_symlinkargs *argp = rqstp->rq_argp;
+	struct nfsd_stat *resp = rqstp->rq_resp;
 	struct svc_fh	newfh;
-	__be32		nfserr;
 
 	if (argp->tlen > NFS_MAXPATHLEN) {
-		nfserr = nfserr_nametoolong;
+		resp->status = nfserr_nametoolong;
 		goto out;
 	}
 
@@ -482,7 +482,7 @@ nfsd_proc_symlink(struct svc_rqst *rqstp)
 						page_address(rqstp->rq_arg.pages[0]),
 						argp->tlen);
 	if (IS_ERR(argp->tname)) {
-		nfserr = nfserrno(PTR_ERR(argp->tname));
+		resp->status = nfserrno(PTR_ERR(argp->tname));
 		goto out;
 	}
 
@@ -491,14 +491,14 @@ nfsd_proc_symlink(struct svc_rqst *rqstp)
 		argp->tlen, argp->tname);
 
 	fh_init(&newfh, NFS_FHSIZE);
-	nfserr = nfsd_symlink(rqstp, &argp->ffh, argp->fname, argp->flen,
-						 argp->tname, &newfh);
+	resp->status = nfsd_symlink(rqstp, &argp->ffh, argp->fname, argp->flen,
+				    argp->tname, &newfh);
 
 	kfree(argp->tname);
 	fh_put(&argp->ffh);
 	fh_put(&newfh);
 out:
-	return nfserr;
+	return rpc_success;
 }
 
 /*
@@ -528,7 +528,7 @@ nfsd_proc_mkdir(struct svc_rqst *rqstp)
 
 	resp->status = fh_getattr(&resp->fh, &resp->stat);
 out:
-	return resp->status;
+	return rpc_success;
 }
 
 /*
@@ -538,13 +538,14 @@ static __be32
 nfsd_proc_rmdir(struct svc_rqst *rqstp)
 {
 	struct nfsd_diropargs *argp = rqstp->rq_argp;
-	__be32	nfserr;
+	struct nfsd_stat *resp = rqstp->rq_resp;
 
 	dprintk("nfsd: RMDIR    %s %.*s\n", SVCFH_fmt(&argp->fh), argp->len, argp->name);
 
-	nfserr = nfsd_unlink(rqstp, &argp->fh, S_IFDIR, argp->name, argp->len);
+	resp->status = nfsd_unlink(rqstp, &argp->fh, S_IFDIR,
+				   argp->name, argp->len);
 	fh_put(&argp->fh);
-	return nfserr;
+	return rpc_success;
 }
 
 /*
@@ -584,7 +585,7 @@ nfsd_proc_readdir(struct svc_rqst *rqstp)
 		*resp->offset = htonl(offset);
 
 	fh_put(&argp->fh);
-	return resp->status;
+	return rpc_success;
 }
 
 /*
@@ -601,7 +602,7 @@ nfsd_proc_statfs(struct svc_rqst *rqstp)
 	resp->status = nfsd_statfs(rqstp, &argp->fh, &resp->stats,
 				   NFSD_MAY_BYPASS_GSS_ON_ROOT);
 	fh_put(&argp->fh);
-	return resp->status;
+	return rpc_success;
 }
 
 /*
@@ -622,7 +623,7 @@ static const struct svc_procedure nfsd_procedures2[18] = {
 		.pc_argsize = sizeof(struct nfsd_void),
 		.pc_ressize = sizeof(struct nfsd_void),
 		.pc_cachetype = RC_NOCACHE,
-		.pc_xdrressize = ST,
+		.pc_xdrressize = 0,
 	},
 	[NFSPROC_GETATTR] = {
 		.pc_func = nfsd_proc_getattr,
@@ -651,7 +652,7 @@ static const struct svc_procedure nfsd_procedures2[18] = {
 		.pc_argsize = sizeof(struct nfsd_void),
 		.pc_ressize = sizeof(struct nfsd_void),
 		.pc_cachetype = RC_NOCACHE,
-		.pc_xdrressize = ST,
+		.pc_xdrressize = 0,
 	},
 	[NFSPROC_LOOKUP] = {
 		.pc_func = nfsd_proc_lookup,
@@ -689,7 +690,7 @@ static const struct svc_procedure nfsd_procedures2[18] = {
 		.pc_argsize = sizeof(struct nfsd_void),
 		.pc_ressize = sizeof(struct nfsd_void),
 		.pc_cachetype = RC_NOCACHE,
-		.pc_xdrressize = ST,
+		.pc_xdrressize = 0,
 	},
 	[NFSPROC_WRITE] = {
 		.pc_func = nfsd_proc_write,
@@ -714,36 +715,36 @@ static const struct svc_procedure nfsd_procedures2[18] = {
 	[NFSPROC_REMOVE] = {
 		.pc_func = nfsd_proc_remove,
 		.pc_decode = nfssvc_decode_diropargs,
-		.pc_encode = nfssvc_encode_void,
+		.pc_encode = nfssvc_encode_stat,
 		.pc_argsize = sizeof(struct nfsd_diropargs),
-		.pc_ressize = sizeof(struct nfsd_void),
+		.pc_ressize = sizeof(struct nfsd_stat),
 		.pc_cachetype = RC_REPLSTAT,
 		.pc_xdrressize = ST,
 	},
 	[NFSPROC_RENAME] = {
 		.pc_func = nfsd_proc_rename,
 		.pc_decode = nfssvc_decode_renameargs,
-		.pc_encode = nfssvc_encode_void,
+		.pc_encode = nfssvc_encode_stat,
 		.pc_argsize = sizeof(struct nfsd_renameargs),
-		.pc_ressize = sizeof(struct nfsd_void),
+		.pc_ressize = sizeof(struct nfsd_stat),
 		.pc_cachetype = RC_REPLSTAT,
 		.pc_xdrressize = ST,
 	},
 	[NFSPROC_LINK] = {
 		.pc_func = nfsd_proc_link,
 		.pc_decode = nfssvc_decode_linkargs,
-		.pc_encode = nfssvc_encode_void,
+		.pc_encode = nfssvc_encode_stat,
 		.pc_argsize = sizeof(struct nfsd_linkargs),
-		.pc_ressize = sizeof(struct nfsd_void),
+		.pc_ressize = sizeof(struct nfsd_stat),
 		.pc_cachetype = RC_REPLSTAT,
 		.pc_xdrressize = ST,
 	},
 	[NFSPROC_SYMLINK] = {
 		.pc_func = nfsd_proc_symlink,
 		.pc_decode = nfssvc_decode_symlinkargs,
-		.pc_encode = nfssvc_encode_void,
+		.pc_encode = nfssvc_encode_stat,
 		.pc_argsize = sizeof(struct nfsd_symlinkargs),
-		.pc_ressize = sizeof(struct nfsd_void),
+		.pc_ressize = sizeof(struct nfsd_stat),
 		.pc_cachetype = RC_REPLSTAT,
 		.pc_xdrressize = ST,
 	},
@@ -760,9 +761,9 @@ static const struct svc_procedure nfsd_procedures2[18] = {
 	[NFSPROC_RMDIR] = {
 		.pc_func = nfsd_proc_rmdir,
 		.pc_decode = nfssvc_decode_diropargs,
-		.pc_encode = nfssvc_encode_void,
+		.pc_encode = nfssvc_encode_stat,
 		.pc_argsize = sizeof(struct nfsd_diropargs),
-		.pc_ressize = sizeof(struct nfsd_void),
+		.pc_ressize = sizeof(struct nfsd_stat),
 		.pc_cachetype = RC_REPLSTAT,
 		.pc_xdrressize = ST,
 	},
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index beb3875241cb..27b1ad136150 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -960,13 +960,6 @@ nfsd(void *vrqstp)
 	return 0;
 }
 
-static __be32 map_new_errors(u32 vers, __be32 nfserr)
-{
-	if (nfserr == nfserr_jukebox && vers == 2)
-		return nfserr_dropit;
-	return nfserr;
-}
-
 /*
  * A write procedure can have a large argument, and a read procedure can
  * have a large reply, but no NFSv2 or NFSv3 procedure has argument and
@@ -1014,7 +1007,7 @@ int nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)
 	const struct svc_procedure *proc = rqstp->rq_procinfo;
 	struct kvec *argv = &rqstp->rq_arg.head[0];
 	struct kvec *resv = &rqstp->rq_res.head[0];
-	__be32 nfserr, *nfserrp;
+	__be32 *p;
 
 	dprintk("nfsd_dispatch: vers %d proc %d\n",
 				rqstp->rq_vers, rqstp->rq_proc);
@@ -1043,18 +1036,14 @@ int nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)
 	 * Need to grab the location to store the status, as
 	 * NFSv4 does some encoding while processing
 	 */
-	nfserrp = resv->iov_base + resv->iov_len;
+	p = resv->iov_base + resv->iov_len;
 	resv->iov_len += sizeof(__be32);
 
-	nfserr = proc->pc_func(rqstp);
-	nfserr = map_new_errors(rqstp->rq_vers, nfserr);
-	if (nfserr == nfserr_dropit || test_bit(RQ_DROPME, &rqstp->rq_flags))
+	*statp = proc->pc_func(rqstp);
+	if (*statp == rpc_drop_reply || test_bit(RQ_DROPME, &rqstp->rq_flags))
 		goto out_update_drop;
 
-	if (rqstp->rq_proc != 0)
-		*nfserrp++ = nfserr;
-
-	if (!proc->pc_encode(rqstp, nfserrp))
+	if (!proc->pc_encode(rqstp, p))
 		goto out_encode_err;
 
 	nfsd_cache_update(rqstp, rqstp->rq_cachetype, statp + 1);
diff --git a/fs/nfsd/nfsxdr.c b/fs/nfsd/nfsxdr.c
index 952e71c95d4e..8a288c8fcd57 100644
--- a/fs/nfsd/nfsxdr.c
+++ b/fs/nfsd/nfsxdr.c
@@ -429,15 +429,25 @@ nfssvc_encode_void(struct svc_rqst *rqstp, __be32 *p)
 	return xdr_ressize_check(rqstp, p);
 }
 
+int
+nfssvc_encode_stat(struct svc_rqst *rqstp, __be32 *p)
+{
+	struct nfsd_stat *resp = rqstp->rq_resp;
+
+	*p++ = resp->status;
+	return xdr_ressize_check(rqstp, p);
+}
+
 int
 nfssvc_encode_attrstat(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd_attrstat *resp = rqstp->rq_resp;
 
+	*p++ = resp->status;
 	if (resp->status != nfs_ok)
-		return xdr_ressize_check(rqstp, p);
-
+		goto out;
 	p = encode_fattr(rqstp, p, &resp->fh, &resp->stat);
+out:
 	return xdr_ressize_check(rqstp, p);
 }
 
@@ -446,11 +456,12 @@ nfssvc_encode_diropres(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd_diropres *resp = rqstp->rq_resp;
 
+	*p++ = resp->status;
 	if (resp->status != nfs_ok)
-		return xdr_ressize_check(rqstp, p);
-
+		goto out;
 	p = encode_fh(p, &resp->fh);
 	p = encode_fattr(rqstp, p, &resp->fh, &resp->stat);
+out:
 	return xdr_ressize_check(rqstp, p);
 }
 
@@ -459,6 +470,7 @@ nfssvc_encode_readlinkres(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd_readlinkres *resp = rqstp->rq_resp;
 
+	*p++ = resp->status;
 	if (resp->status != nfs_ok)
 		return xdr_ressize_check(rqstp, p);
 
@@ -479,6 +491,7 @@ nfssvc_encode_readres(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd_readres *resp = rqstp->rq_resp;
 
+	*p++ = resp->status;
 	if (resp->status != nfs_ok)
 		return xdr_ressize_check(rqstp, p);
 
@@ -502,6 +515,7 @@ nfssvc_encode_readdirres(struct svc_rqst *rqstp, __be32 *p)
 {
 	struct nfsd_readdirres *resp = rqstp->rq_resp;
 
+	*p++ = resp->status;
 	if (resp->status != nfs_ok)
 		return xdr_ressize_check(rqstp, p);
 
@@ -520,6 +534,7 @@ nfssvc_encode_statfsres(struct svc_rqst *rqstp, __be32 *p)
 	struct nfsd_statfsres *resp = rqstp->rq_resp;
 	struct kstatfs	*stat = &resp->stats;
 
+	*p++ = resp->status;
 	if (resp->status != nfs_ok)
 		return xdr_ressize_check(rqstp, p);
 
diff --git a/fs/nfsd/xdr.h b/fs/nfsd/xdr.h
index 17221931815f..0ff336b0b25f 100644
--- a/fs/nfsd/xdr.h
+++ b/fs/nfsd/xdr.h
@@ -82,6 +82,10 @@ struct nfsd_readdirargs {
 	__be32 *		buffer;
 };
 
+struct nfsd_stat {
+	__be32			status;
+};
+
 struct nfsd_attrstat {
 	__be32			status;
 	struct svc_fh		fh;
@@ -153,6 +157,7 @@ int nfssvc_decode_linkargs(struct svc_rqst *, __be32 *);
 int nfssvc_decode_symlinkargs(struct svc_rqst *, __be32 *);
 int nfssvc_decode_readdirargs(struct svc_rqst *, __be32 *);
 int nfssvc_encode_void(struct svc_rqst *, __be32 *);
+int nfssvc_encode_stat(struct svc_rqst *, __be32 *);
 int nfssvc_encode_attrstat(struct svc_rqst *, __be32 *);
 int nfssvc_encode_diropres(struct svc_rqst *, __be32 *);
 int nfssvc_encode_readlinkres(struct svc_rqst *, __be32 *);



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

* Re: [PATCH v3 00/15] nfsd_dispatch() clean up
  2020-10-01 22:58 [PATCH v3 00/15] nfsd_dispatch() clean up Chuck Lever
                   ` (14 preceding siblings ...)
  2020-10-01 23:00 ` [PATCH v3 15/15] NFSD: Hoist status code encoding into XDR encoder functions Chuck Lever
@ 2020-10-02 17:39 ` J. Bruce Fields
  2020-10-02 17:42   ` J. Bruce Fields
  15 siblings, 1 reply; 19+ messages in thread
From: J. Bruce Fields @ 2020-10-02 17:39 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs

I'm seeing a pynfs4.0 GATT9 regression.  That's a test that attempts a
compound with 90 GETATTR ops each a request for all mandatory
attributes.  The test expects OK or RESOURCE but looks like its getting
a corrupted response?  (I haven't looked at the wire traffic yet.)  I
think it's one of the final patches changing how errors are returned.

--b.

On Thu, Oct 01, 2020 at 06:58:46PM -0400, Chuck Lever wrote:
> Hi Bruce-
> 
> Here's the latest version of the nfsd_dispatch clean up series,
> building on the "non-controversial" patches I posted last week.
> 
> The purpose of this series is three-fold:
> 
> o Prepare to add NFS procedure tracepoints
> o Prepare to eventually deprecate NFSv2
> o Minor optimizations of the dispatcher hot path
> 
> 
> Changes since v2:
> - Fixed crasher caused by invoking NFSv2 ROOT or WRITECACHE
> - Hoisted encoding of NFS status code into XDR Reply encoders
> - Numerous bug fixes, clean ups, and patch re-ordering
> 
> Changes since v1:
> - Pulled in latest version of rq_lease_breaker cleanup
> - Added patches to make NFSv2 error encoding similar to NFSv3
> - Clarified nfsd_dispatch's new documenting comment
> - Renamed a variable
> 
> ---
> 
> Chuck Lever (14):
>       NFSD: Add missing NFSv2 .pc_func methods
>       lockd: Replace PROC() macro with open code
>       NFSACL: Replace PROC() macro with open code
>       NFSD: Encoder and decoder functions are always present
>       NFSD: Clean up switch statement in nfsd_dispatch()
>       NFSD: Clean up stale comments in nfsd_dispatch()
>       NFSD: Clean up nfsd_dispatch() variables
>       NFSD: Refactor nfsd_dispatch() error paths
>       NFSD: Remove vestigial typedefs
>       NFSD: Fix .pc_release method for NFSv2
>       NFSD: Call NFSv2 encoders on error returns
>       NFSD: Remove the RETURN_STATUS() macro
>       NFSD: Map nfserr_wrongsec outside of nfsd_dispatch
>       NFSD: Hoist status code encoding into XDR encoder functions
> 
> J. Bruce Fields (1):
>       nfsd: rq_lease_breaker cleanup
> 
> 
>  fs/lockd/svc4proc.c         | 248 ++++++++++++++++++++++++-------
>  fs/lockd/svcproc.c          | 250 ++++++++++++++++++++++++-------
>  fs/nfsd/export.c            |   2 +-
>  fs/nfsd/nfs2acl.c           | 160 +++++++++++++-------
>  fs/nfsd/nfs3acl.c           |  88 ++++++-----
>  fs/nfsd/nfs3proc.c          | 238 +++++++++++++++---------------
>  fs/nfsd/nfs3xdr.c           |  25 +++-
>  fs/nfsd/nfs4proc.c          |   6 +-
>  fs/nfsd/nfs4xdr.c           |  11 +-
>  fs/nfsd/nfsproc.c           | 283 ++++++++++++++++++++----------------
>  fs/nfsd/nfssvc.c            | 121 ++++++++-------
>  fs/nfsd/nfsxdr.c            |  52 ++++++-
>  fs/nfsd/xdr.h               |  16 +-
>  fs/nfsd/xdr3.h              |   1 +
>  fs/nfsd/xdr4.h              |   1 +
>  include/uapi/linux/nfsacl.h |   2 +
>  16 files changed, 984 insertions(+), 520 deletions(-)
> 
> --
> Chuck Lever

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

* Re: [PATCH v3 00/15] nfsd_dispatch() clean up
  2020-10-02 17:39 ` [PATCH v3 00/15] nfsd_dispatch() clean up J. Bruce Fields
@ 2020-10-02 17:42   ` J. Bruce Fields
  2020-10-02 17:44     ` Chuck Lever
  0 siblings, 1 reply; 19+ messages in thread
From: J. Bruce Fields @ 2020-10-02 17:42 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs

On Fri, Oct 02, 2020 at 01:39:08PM -0400, J. Bruce Fields wrote:
> I'm seeing a pynfs4.0 GATT9 regression.  That's a test that attempts a
> compound with 90 GETATTR ops each a request for all mandatory
> attributes.  The test expects OK or RESOURCE but looks like its getting
> a corrupted response?  (I haven't looked at the wire traffic yet.)  I
> think it's one of the final patches changing how errors are returned.

Also some other tests that send compounds with lots of ops:

GATT9    st_getattr.testLotsofGetattrsFile                        : FAILURE
           nfs4lib.InvalidCompoundRes: Invalid COMPOUND result:
           Truncated response list.
COMP6    st_compound.testLongCompound                             : FAILURE
           COMPOUND with len=150 argarry got Invalid COMPOUND
           result: Truncated response list., expected
           NFS4ERR_RESOURCE
COMP4    st_compound.testInvalidMinor                             : FAILURE
           nfs4lib.InvalidCompoundRes: Invalid COMPOUND result:
           Truncated response list.

Bisect lands on the last patch ("Hoist status code...").

--b.

> 
> --b.
> 
> On Thu, Oct 01, 2020 at 06:58:46PM -0400, Chuck Lever wrote:
> > Hi Bruce-
> > 
> > Here's the latest version of the nfsd_dispatch clean up series,
> > building on the "non-controversial" patches I posted last week.
> > 
> > The purpose of this series is three-fold:
> > 
> > o Prepare to add NFS procedure tracepoints
> > o Prepare to eventually deprecate NFSv2
> > o Minor optimizations of the dispatcher hot path
> > 
> > 
> > Changes since v2:
> > - Fixed crasher caused by invoking NFSv2 ROOT or WRITECACHE
> > - Hoisted encoding of NFS status code into XDR Reply encoders
> > - Numerous bug fixes, clean ups, and patch re-ordering
> > 
> > Changes since v1:
> > - Pulled in latest version of rq_lease_breaker cleanup
> > - Added patches to make NFSv2 error encoding similar to NFSv3
> > - Clarified nfsd_dispatch's new documenting comment
> > - Renamed a variable
> > 
> > ---
> > 
> > Chuck Lever (14):
> >       NFSD: Add missing NFSv2 .pc_func methods
> >       lockd: Replace PROC() macro with open code
> >       NFSACL: Replace PROC() macro with open code
> >       NFSD: Encoder and decoder functions are always present
> >       NFSD: Clean up switch statement in nfsd_dispatch()
> >       NFSD: Clean up stale comments in nfsd_dispatch()
> >       NFSD: Clean up nfsd_dispatch() variables
> >       NFSD: Refactor nfsd_dispatch() error paths
> >       NFSD: Remove vestigial typedefs
> >       NFSD: Fix .pc_release method for NFSv2
> >       NFSD: Call NFSv2 encoders on error returns
> >       NFSD: Remove the RETURN_STATUS() macro
> >       NFSD: Map nfserr_wrongsec outside of nfsd_dispatch
> >       NFSD: Hoist status code encoding into XDR encoder functions
> > 
> > J. Bruce Fields (1):
> >       nfsd: rq_lease_breaker cleanup
> > 
> > 
> >  fs/lockd/svc4proc.c         | 248 ++++++++++++++++++++++++-------
> >  fs/lockd/svcproc.c          | 250 ++++++++++++++++++++++++-------
> >  fs/nfsd/export.c            |   2 +-
> >  fs/nfsd/nfs2acl.c           | 160 +++++++++++++-------
> >  fs/nfsd/nfs3acl.c           |  88 ++++++-----
> >  fs/nfsd/nfs3proc.c          | 238 +++++++++++++++---------------
> >  fs/nfsd/nfs3xdr.c           |  25 +++-
> >  fs/nfsd/nfs4proc.c          |   6 +-
> >  fs/nfsd/nfs4xdr.c           |  11 +-
> >  fs/nfsd/nfsproc.c           | 283 ++++++++++++++++++++----------------
> >  fs/nfsd/nfssvc.c            | 121 ++++++++-------
> >  fs/nfsd/nfsxdr.c            |  52 ++++++-
> >  fs/nfsd/xdr.h               |  16 +-
> >  fs/nfsd/xdr3.h              |   1 +
> >  fs/nfsd/xdr4.h              |   1 +
> >  include/uapi/linux/nfsacl.h |   2 +
> >  16 files changed, 984 insertions(+), 520 deletions(-)
> > 
> > --
> > Chuck Lever

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

* Re: [PATCH v3 00/15] nfsd_dispatch() clean up
  2020-10-02 17:42   ` J. Bruce Fields
@ 2020-10-02 17:44     ` Chuck Lever
  0 siblings, 0 replies; 19+ messages in thread
From: Chuck Lever @ 2020-10-02 17:44 UTC (permalink / raw)
  To: Bruce Fields; +Cc: Linux NFS Mailing List



> On Oct 2, 2020, at 1:42 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> 
> On Fri, Oct 02, 2020 at 01:39:08PM -0400, J. Bruce Fields wrote:
>> I'm seeing a pynfs4.0 GATT9 regression.  That's a test that attempts a
>> compound with 90 GETATTR ops each a request for all mandatory
>> attributes.  The test expects OK or RESOURCE but looks like its getting
>> a corrupted response?  (I haven't looked at the wire traffic yet.)  I
>> think it's one of the final patches changing how errors are returned.
> 
> Also some other tests that send compounds with lots of ops:
> 
> GATT9    st_getattr.testLotsofGetattrsFile                        : FAILURE
>           nfs4lib.InvalidCompoundRes: Invalid COMPOUND result:
>           Truncated response list.
> COMP6    st_compound.testLongCompound                             : FAILURE
>           COMPOUND with len=150 argarry got Invalid COMPOUND
>           result: Truncated response list., expected
>           NFS4ERR_RESOURCE
> COMP4    st_compound.testInvalidMinor                             : FAILURE
>           nfs4lib.InvalidCompoundRes: Invalid COMPOUND result:
>           Truncated response list.
> 
> Bisect lands on the last patch ("Hoist status code...").

Excellent, thanks for the test results. I'll take a look.


--
Chuck Lever




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

end of thread, other threads:[~2020-10-02 17:44 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-01 22:58 [PATCH v3 00/15] nfsd_dispatch() clean up Chuck Lever
2020-10-01 22:58 ` [PATCH v3 01/15] nfsd: rq_lease_breaker cleanup Chuck Lever
2020-10-01 22:58 ` [PATCH v3 02/15] NFSD: Add missing NFSv2 .pc_func methods Chuck Lever
2020-10-01 22:59 ` [PATCH v3 03/15] lockd: Replace PROC() macro with open code Chuck Lever
2020-10-01 22:59 ` [PATCH v3 04/15] NFSACL: " Chuck Lever
2020-10-01 22:59 ` [PATCH v3 05/15] NFSD: Encoder and decoder functions are always present Chuck Lever
2020-10-01 22:59 ` [PATCH v3 06/15] NFSD: Clean up switch statement in nfsd_dispatch() Chuck Lever
2020-10-01 22:59 ` [PATCH v3 07/15] NFSD: Clean up stale comments " Chuck Lever
2020-10-01 22:59 ` [PATCH v3 08/15] NFSD: Clean up nfsd_dispatch() variables Chuck Lever
2020-10-01 22:59 ` [PATCH v3 09/15] NFSD: Refactor nfsd_dispatch() error paths Chuck Lever
2020-10-01 22:59 ` [PATCH v3 10/15] NFSD: Remove vestigial typedefs Chuck Lever
2020-10-01 22:59 ` [PATCH v3 11/15] NFSD: Fix .pc_release method for NFSv2 Chuck Lever
2020-10-01 22:59 ` [PATCH v3 12/15] NFSD: Call NFSv2 encoders on error returns Chuck Lever
2020-10-01 22:59 ` [PATCH v3 13/15] NFSD: Remove the RETURN_STATUS() macro Chuck Lever
2020-10-01 23:00 ` [PATCH v3 14/15] NFSD: Map nfserr_wrongsec outside of nfsd_dispatch Chuck Lever
2020-10-01 23:00 ` [PATCH v3 15/15] NFSD: Hoist status code encoding into XDR encoder functions Chuck Lever
2020-10-02 17:39 ` [PATCH v3 00/15] nfsd_dispatch() clean up J. Bruce Fields
2020-10-02 17:42   ` J. Bruce Fields
2020-10-02 17:44     ` Chuck Lever

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).