Hi Bruce. I forgot that these two go together. Sorry about that.
"SUNRPC: Simplify the SVC dispatch code path" is unchanged from
its initial posting a few minutes ago.
---
Chuck Lever (2):
SUNRPC: Simplify the SVC dispatch code path
SUNRPC: De-duplicate .pc_release() call sites
include/linux/sunrpc/svc.h | 5 +--
net/sunrpc/svc.c | 69 ++++----------------------------------
2 files changed, 8 insertions(+), 66 deletions(-)
--
Chuck Lever
Micro-optimization: The last user of the generic SVC dispatch code
path has been removed, so svc_process_common() can be simplified.
This declutters the hot path so that the by-far most common case
(a dispatch function exists) is made the /only/ path.
Signed-off-by: Chuck Lever <[email protected]>
---
include/linux/sunrpc/svc.h | 5 +---
net/sunrpc/svc.c | 51 ++------------------------------------------
2 files changed, 3 insertions(+), 53 deletions(-)
diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index 6263410c948a..4205a6ef4770 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -443,10 +443,7 @@ struct svc_version {
/* Need xprt with congestion control */
bool vs_need_cong_ctrl;
- /* Override dispatch function (e.g. when caching replies).
- * A return value of 0 means drop the request.
- * vs_dispatch == NULL means use default dispatcher.
- */
+ /* Dispatch function */
int (*vs_dispatch)(struct svc_rqst *, __be32 *);
};
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 08ca797bb8a4..e0dd6e6a4602 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1186,45 +1186,6 @@ void svc_printk(struct svc_rqst *rqstp, const char *fmt, ...)
static __printf(2,3) void svc_printk(struct svc_rqst *rqstp, const char *fmt, ...) {}
#endif
-static int
-svc_generic_dispatch(struct svc_rqst *rqstp, __be32 *statp)
-{
- struct kvec *argv = &rqstp->rq_arg.head[0];
- struct kvec *resv = &rqstp->rq_res.head[0];
- const struct svc_procedure *procp = rqstp->rq_procinfo;
-
- /*
- * Decode arguments
- * XXX: why do we ignore the return value?
- */
- if (procp->pc_decode &&
- !procp->pc_decode(rqstp, argv->iov_base)) {
- *statp = rpc_garbage_args;
- return 1;
- }
-
- *statp = procp->pc_func(rqstp);
-
- if (*statp == rpc_drop_reply ||
- test_bit(RQ_DROPME, &rqstp->rq_flags))
- return 0;
-
- if (rqstp->rq_auth_stat != rpc_auth_ok)
- return 1;
-
- if (*statp != rpc_success)
- return 1;
-
- /* Encode reply */
- if (procp->pc_encode &&
- !procp->pc_encode(rqstp, resv->iov_base + resv->iov_len)) {
- dprintk("svc: failed to encode reply\n");
- /* serv->sv_stats->rpcsystemerr++; */
- *statp = rpc_system_err;
- }
- return 1;
-}
-
__be32
svc_generic_init_request(struct svc_rqst *rqstp,
const struct svc_program *progp,
@@ -1392,16 +1353,8 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
svc_reserve_auth(rqstp, procp->pc_xdrressize<<2);
/* Call the function that processes the request. */
- if (!process.dispatch) {
- if (!svc_generic_dispatch(rqstp, statp))
- goto release_dropit;
- if (*statp == rpc_garbage_args)
- goto err_garbage;
- } else {
- dprintk("svc: calling dispatcher\n");
- if (!process.dispatch(rqstp, statp))
- goto release_dropit; /* Release reply info */
- }
+ if (!process.dispatch(rqstp, statp))
+ goto release_dropit;
if (rqstp->rq_auth_stat != rpc_auth_ok)
goto err_release_bad_auth;
There was some spaghetti in svc_process_common() that had evolved
over time such that there was still one case that needed a call
to .pc_release() but never made it. That issue was removed in
the previous patch.
As additional insurance against missing this important callout,
ensure that the .pc_release() method is always called, no matter
what the reply_stat is.
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/svc.c | 22 +++++++---------------
1 file changed, 7 insertions(+), 15 deletions(-)
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index e0dd6e6a4602..4292278a9552 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1252,7 +1252,7 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
__be32 *statp;
u32 prog, vers;
__be32 rpc_stat;
- int auth_res;
+ int auth_res, rc;
__be32 *reply_statp;
rpc_stat = rpc_success;
@@ -1353,20 +1353,18 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
svc_reserve_auth(rqstp, procp->pc_xdrressize<<2);
/* Call the function that processes the request. */
- if (!process.dispatch(rqstp, statp))
- goto release_dropit;
-
+ rc = process.dispatch(rqstp, statp);
+ if (procp->pc_release)
+ procp->pc_release(rqstp);
+ if (!rc)
+ goto dropit;
if (rqstp->rq_auth_stat != rpc_auth_ok)
- goto err_release_bad_auth;
+ goto err_bad_auth;
/* Check RPC status result */
if (*statp != rpc_success)
resv->iov_len = ((void*)statp) - resv->iov_base + 4;
- /* Release reply info */
- if (procp->pc_release)
- procp->pc_release(rqstp);
-
if (procp->pc_encode == NULL)
goto dropit;
@@ -1375,9 +1373,6 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
goto close_xprt;
return 1; /* Caller can now send it */
-release_dropit:
- if (procp->pc_release)
- procp->pc_release(rqstp);
dropit:
svc_authorise(rqstp); /* doesn't hurt to call this twice */
dprintk("svc: svc_process dropit\n");
@@ -1404,9 +1399,6 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
svc_putnl(resv, 2);
goto sendit;
-err_release_bad_auth:
- if (procp->pc_release)
- procp->pc_release(rqstp);
err_bad_auth:
dprintk("svc: authentication failed (%d)\n",
be32_to_cpu(rqstp->rq_auth_stat));
On Thu, Oct 07, 2021 at 04:17:17PM -0400, Chuck Lever wrote:
> Hi Bruce. I forgot that these two go together. Sorry about that.
>
> "SUNRPC: Simplify the SVC dispatch code path" is unchanged from
> its initial posting a few minutes ago.
Looks good to me, thanks; applied.--b.
>
> ---
>
> Chuck Lever (2):
> SUNRPC: Simplify the SVC dispatch code path
> SUNRPC: De-duplicate .pc_release() call sites
>
>
> include/linux/sunrpc/svc.h | 5 +--
> net/sunrpc/svc.c | 69 ++++----------------------------------
> 2 files changed, 8 insertions(+), 66 deletions(-)
>
> --
> Chuck Lever