* [PATCH 0/9] nfsd_dispatch() clean up
@ 2020-09-25 16:59 Chuck Lever
2020-09-25 16:59 ` [PATCH 1/9] nfsd: rq_lease_breaker cleanup Chuck Lever
` (8 more replies)
0 siblings, 9 replies; 11+ messages in thread
From: Chuck Lever @ 2020-09-25 16:59 UTC (permalink / raw)
To: bfields; +Cc: linux-nfs
Hi Bruce, these don't seem too controversial, and could be merged
in v5.10 if you agree they are ready.
---
Chuck Lever (8):
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: Set *statp in success path
J. Bruce Fields (1):
nfsd: rq_lease_breaker cleanup
fs/lockd/svc4proc.c | 242 +++++++++++++++++++++++++++--------
fs/lockd/svcproc.c | 244 ++++++++++++++++++++++++++++--------
fs/nfsd/nfs2acl.c | 78 ++++++++----
fs/nfsd/nfs3acl.c | 50 +++++---
fs/nfsd/nfs3proc.c | 1 +
fs/nfsd/nfs3xdr.c | 6 +
fs/nfsd/nfs4proc.c | 1 +
fs/nfsd/nfs4xdr.c | 6 +
fs/nfsd/nfssvc.c | 105 +++++++++-------
fs/nfsd/xdr3.h | 1 +
fs/nfsd/xdr4.h | 1 +
include/uapi/linux/nfsacl.h | 2 +
12 files changed, 548 insertions(+), 189 deletions(-)
--
Chuck Lever
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/9] nfsd: rq_lease_breaker cleanup
2020-09-25 16:59 [PATCH 0/9] nfsd_dispatch() clean up Chuck Lever
@ 2020-09-25 16:59 ` Chuck Lever
2020-09-25 17:30 ` Chuck Lever
2020-09-25 17:00 ` [PATCH 2/9] lockd: Replace PROC() macro with open code Chuck Lever
` (7 subsequent siblings)
8 siblings, 1 reply; 11+ messages in thread
From: Chuck Lever @ 2020-09-25 16:59 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>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
fs/nfsd/nfs4state.c | 3 +++
fs/nfsd/nfssvc.c | 1 -
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index c09a2a4281ec..d9325dea0b74 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4596,6 +4596,9 @@ static bool nfsd_breaker_owns_lease(struct file_lock *fl)
if (!i_am_nfsd())
return NULL;
+ /* Note rq_prog == NFS_ACL_PROGRAM is also possible: */
+ if (rqst->rq_prog != NFS_PROGRAM || rqst->rq_vers < 4)
+ return NULL;
rqst = kthread_data(current);
if (!rqst->rq_lease_breaker)
return NULL;
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] 11+ messages in thread
* [PATCH 2/9] lockd: Replace PROC() macro with open code
2020-09-25 16:59 [PATCH 0/9] nfsd_dispatch() clean up Chuck Lever
2020-09-25 16:59 ` [PATCH 1/9] nfsd: rq_lease_breaker cleanup Chuck Lever
@ 2020-09-25 17:00 ` Chuck Lever
2020-09-25 17:00 ` [PATCH 3/9] NFSACL: " Chuck Lever
` (6 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Chuck Lever @ 2020-09-25 17:00 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 | 242 ++++++++++++++++++++++++++++++++++++++++-----------
fs/lockd/svcproc.c | 244 ++++++++++++++++++++++++++++++++++++++++-----------
2 files changed, 386 insertions(+), 100 deletions(-)
diff --git a/fs/lockd/svc4proc.c b/fs/lockd/svc4proc.c
index e4d3f783e06a..9913b823a5e7 100644
--- a/fs/lockd/svc4proc.c
+++ b/fs/lockd/svc4proc.c
@@ -491,60 +491,204 @@ nlm4svc_proc_granted_res(struct svc_rqst *rqstp)
* 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] = { /* unused procedure */
+ .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 = 0,
+ },
+ [18] = { /* unused procedure */
+ .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 = 0,
+ },
+ [19] = { /* unused procedure */
+ .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 = 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..dbcfa67a36b1 100644
--- a/fs/lockd/svcproc.c
+++ b/fs/lockd/svcproc.c
@@ -533,62 +533,204 @@ nlmsvc_proc_granted_res(struct svc_rqst *rqstp)
* 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] = { /* unused procedure */
+ .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,
+ },
+ [18] = { /* unused procedure */
+ .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,
+ },
+ [19] = { /* unused procedure */
+ .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_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] 11+ messages in thread
* [PATCH 3/9] NFSACL: Replace PROC() macro with open code
2020-09-25 16:59 [PATCH 0/9] nfsd_dispatch() clean up Chuck Lever
2020-09-25 16:59 ` [PATCH 1/9] nfsd: rq_lease_breaker cleanup Chuck Lever
2020-09-25 17:00 ` [PATCH 2/9] lockd: Replace PROC() macro with open code Chuck Lever
@ 2020-09-25 17:00 ` Chuck Lever
2020-09-25 17:00 ` [PATCH 4/9] NFSD: Encoder and decoder functions are always present Chuck Lever
` (5 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Chuck Lever @ 2020-09-25 17:00 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] 11+ messages in thread
* [PATCH 4/9] NFSD: Encoder and decoder functions are always present
2020-09-25 16:59 [PATCH 0/9] nfsd_dispatch() clean up Chuck Lever
` (2 preceding siblings ...)
2020-09-25 17:00 ` [PATCH 3/9] NFSACL: " Chuck Lever
@ 2020-09-25 17:00 ` Chuck Lever
2020-09-25 17:00 ` [PATCH 5/9] NFSD: Clean up switch statement in nfsd_dispatch() Chuck Lever
` (4 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Chuck Lever @ 2020-09-25 17:00 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 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 259d5ad0e3f4..4449d9858bdc 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] 11+ messages in thread
* [PATCH 5/9] NFSD: Clean up switch statement in nfsd_dispatch()
2020-09-25 16:59 [PATCH 0/9] nfsd_dispatch() clean up Chuck Lever
` (3 preceding siblings ...)
2020-09-25 17:00 ` [PATCH 4/9] NFSD: Encoder and decoder functions are always present Chuck Lever
@ 2020-09-25 17:00 ` Chuck Lever
2020-09-25 17:00 ` [PATCH 6/9] NFSD: Clean up stale comments " Chuck Lever
` (3 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Chuck Lever @ 2020-09-25 17:00 UTC (permalink / raw)
To: bfields; +Cc: linux-nfs
Reorder the arms so the compiler places checks for the most frequent
cases 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] 11+ messages in thread
* [PATCH 6/9] NFSD: Clean up stale comments in nfsd_dispatch()
2020-09-25 16:59 [PATCH 0/9] nfsd_dispatch() clean up Chuck Lever
` (4 preceding siblings ...)
2020-09-25 17:00 ` [PATCH 5/9] NFSD: Clean up switch statement in nfsd_dispatch() Chuck Lever
@ 2020-09-25 17:00 ` Chuck Lever
2020-09-25 17:00 ` [PATCH 7/9] NFSD: Clean up nfsd_dispatch() variables Chuck Lever
` (2 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Chuck Lever @ 2020-09-25 17:00 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 | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index 3cdefb2294ce..b2581bcbd81c 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -1000,8 +1000,16 @@ 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: OUT: RPC accept_stat value
+ *
+ * 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;
@@ -1016,19 +1024,18 @@ nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)
*statp = rpc_garbage_args;
return 1;
}
+
/*
* 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;
- /* 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 +1045,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 +1064,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 +1076,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] 11+ messages in thread
* [PATCH 7/9] NFSD: Clean up nfsd_dispatch() variables
2020-09-25 16:59 [PATCH 0/9] nfsd_dispatch() clean up Chuck Lever
` (5 preceding siblings ...)
2020-09-25 17:00 ` [PATCH 6/9] NFSD: Clean up stale comments " Chuck Lever
@ 2020-09-25 17:00 ` Chuck Lever
2020-09-25 17:00 ` [PATCH 8/9] NFSD: Refactor nfsd_dispatch() error paths Chuck Lever
2020-09-25 17:00 ` [PATCH 9/9] NFSD: Set *statp in success path Chuck Lever
8 siblings, 0 replies; 11+ messages in thread
From: Chuck Lever @ 2020-09-25 17:00 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 b2581bcbd81c..2eb20cbf590f 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -1011,13 +1011,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);
@@ -1030,7 +1030,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;
@@ -1049,9 +1049,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] 11+ messages in thread
* [PATCH 8/9] NFSD: Refactor nfsd_dispatch() error paths
2020-09-25 16:59 [PATCH 0/9] nfsd_dispatch() clean up Chuck Lever
` (6 preceding siblings ...)
2020-09-25 17:00 ` [PATCH 7/9] NFSD: Clean up nfsd_dispatch() variables Chuck Lever
@ 2020-09-25 17:00 ` Chuck Lever
2020-09-25 17:00 ` [PATCH 9/9] NFSD: Set *statp in success path Chuck Lever
8 siblings, 0 replies; 11+ messages in thread
From: Chuck Lever @ 2020-09-25 17:00 UTC (permalink / raw)
To: bfields; +Cc: linux-nfs
nfsd_dispatch() is a hot path. Ensure the compiler takes the
processing of infrequent errors out of line.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
fs/nfsd/nfssvc.c | 59 +++++++++++++++++++++++++++++++-----------------------
1 file changed, 34 insertions(+), 25 deletions(-)
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index 2eb20cbf590f..d389b276aa5e 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -1019,30 +1019,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;
}
/*
@@ -1054,11 +1048,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;
@@ -1066,16 +1057,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] 11+ messages in thread
* [PATCH 9/9] NFSD: Set *statp in success path
2020-09-25 16:59 [PATCH 0/9] nfsd_dispatch() clean up Chuck Lever
` (7 preceding siblings ...)
2020-09-25 17:00 ` [PATCH 8/9] NFSD: Refactor nfsd_dispatch() error paths Chuck Lever
@ 2020-09-25 17:00 ` Chuck Lever
8 siblings, 0 replies; 11+ messages in thread
From: Chuck Lever @ 2020-09-25 17:00 UTC (permalink / raw)
To: bfields; +Cc: linux-nfs
*statp is supposed to be set in every path through nfsd_dispatch().
The success case appears to leave *statp uninitialized.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
fs/nfsd/nfssvc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index d389b276aa5e..2117cc70b493 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -1063,6 +1063,7 @@ int nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)
nfsd_cache_update(rqstp, rqstp->rq_cachetype, statp + 1);
out_cached_reply:
+ *statp = rpc_success;
return 1;
out_too_large:
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/9] nfsd: rq_lease_breaker cleanup
2020-09-25 16:59 ` [PATCH 1/9] nfsd: rq_lease_breaker cleanup Chuck Lever
@ 2020-09-25 17:30 ` Chuck Lever
0 siblings, 0 replies; 11+ messages in thread
From: Chuck Lever @ 2020-09-25 17:30 UTC (permalink / raw)
To: Bruce Fields; +Cc: Linux NFS Mailing List
> On Sep 25, 2020, at 12:59 PM, Chuck Lever <chuck.lever@oracle.com> wrote:
>
> 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>
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> fs/nfsd/nfs4state.c | 3 +++
> fs/nfsd/nfssvc.c | 1 -
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index c09a2a4281ec..d9325dea0b74 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -4596,6 +4596,9 @@ static bool nfsd_breaker_owns_lease(struct file_lock *fl)
>
> if (!i_am_nfsd())
> return NULL;
> + /* Note rq_prog == NFS_ACL_PROGRAM is also possible: */
> + if (rqst->rq_prog != NFS_PROGRAM || rqst->rq_vers < 4)
> + return NULL;
> rqst = kthread_data(current);
I had to apply this one by hand, and as usual, got it wrong.
> if (!rqst->rq_lease_breaker)
> return NULL;
> 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)
>
>
--
Chuck Lever
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-09-25 17:30 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-25 16:59 [PATCH 0/9] nfsd_dispatch() clean up Chuck Lever
2020-09-25 16:59 ` [PATCH 1/9] nfsd: rq_lease_breaker cleanup Chuck Lever
2020-09-25 17:30 ` Chuck Lever
2020-09-25 17:00 ` [PATCH 2/9] lockd: Replace PROC() macro with open code Chuck Lever
2020-09-25 17:00 ` [PATCH 3/9] NFSACL: " Chuck Lever
2020-09-25 17:00 ` [PATCH 4/9] NFSD: Encoder and decoder functions are always present Chuck Lever
2020-09-25 17:00 ` [PATCH 5/9] NFSD: Clean up switch statement in nfsd_dispatch() Chuck Lever
2020-09-25 17:00 ` [PATCH 6/9] NFSD: Clean up stale comments " Chuck Lever
2020-09-25 17:00 ` [PATCH 7/9] NFSD: Clean up nfsd_dispatch() variables Chuck Lever
2020-09-25 17:00 ` [PATCH 8/9] NFSD: Refactor nfsd_dispatch() error paths Chuck Lever
2020-09-25 17:00 ` [PATCH 9/9] NFSD: Set *statp in success path 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).