* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.