diff mbox series

[05/27] SUNRPC: Avoid holding locks across the XDR encoding of the RPC message

Message ID 20180903152936.24325-6-trond.myklebust@hammerspace.com (mailing list archive)
State New, archived
Headers show
Series Convert RPC client transmission to a queued model | expand

Commit Message

Trond Myklebust Sept. 3, 2018, 3:29 p.m. UTC
Currently, we grab the socket bit lock before we allow the message
to be XDR encoded. That significantly slows down the transmission
rate, since we serialise on a potentially blocking operation.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 net/sunrpc/clnt.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Chuck Lever Sept. 3, 2018, 5:11 p.m. UTC | #1
> On Sep 3, 2018, at 11:29 AM, Trond Myklebust <trondmy@gmail.com> wrote:
> 
> Currently, we grab the socket bit lock before we allow the message
> to be XDR encoded. That significantly slows down the transmission
> rate, since we serialise on a potentially blocking operation.

Which operation blocks, and how often?


> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> ---
> net/sunrpc/clnt.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index e5ac35e803ad..66ec61347716 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -1949,9 +1949,6 @@ call_transmit(struct rpc_task *task)
> 	task->tk_action = call_status;
> 	if (task->tk_status < 0)
> 		return;
> -	if (!xprt_prepare_transmit(task))
> -		return;
> -	task->tk_action = call_transmit_status;
> 	/* Encode here so that rpcsec_gss can use correct sequence number. */
> 	if (rpc_task_need_encode(task)) {
> 		rpc_xdr_encode(task);
> @@ -1965,8 +1962,11 @@ call_transmit(struct rpc_task *task)
> 			return;
> 		}
> 	}
> +	if (!xprt_prepare_transmit(task))
> +		return;
> +	task->tk_action = call_transmit_status;
> 	xprt_transmit(task);
> -	if (task->tk_status < 0)
> +	if (task->tk_status < 0) {

The added curly bracket seems incorrect.


> 		return;
> 	if (is_retrans)
> 		task->tk_client->cl_stats->rpcretrans++;
> -- 
> 2.17.1
> 

--
Chuck Lever
chucklever@gmail.com
Trond Myklebust Sept. 3, 2018, 5:40 p.m. UTC | #2
On Mon, 2018-09-03 at 13:11 -0400, Chuck Lever wrote:
> > On Sep 3, 2018, at 11:29 AM, Trond Myklebust <trondmy@gmail.com>
> > wrote:
> > 
> > Currently, we grab the socket bit lock before we allow the message
> > to be XDR encoded. That significantly slows down the transmission
> > rate, since we serialise on a potentially blocking operation.
> 
> Which operation blocks, and how often?

RPCSEC_GSS allocates memory when doing privacy encoding, for instance. 

> > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> > ---
> > net/sunrpc/clnt.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> > index e5ac35e803ad..66ec61347716 100644
> > --- a/net/sunrpc/clnt.c
> > +++ b/net/sunrpc/clnt.c
> > @@ -1949,9 +1949,6 @@ call_transmit(struct rpc_task *task)
> > 	task->tk_action = call_status;
> > 	if (task->tk_status < 0)
> > 		return;
> > -	if (!xprt_prepare_transmit(task))
> > -		return;
> > -	task->tk_action = call_transmit_status;
> > 	/* Encode here so that rpcsec_gss can use correct sequence
> > number. */
> > 	if (rpc_task_need_encode(task)) {
> > 		rpc_xdr_encode(task);
> > @@ -1965,8 +1962,11 @@ call_transmit(struct rpc_task *task)
> > 			return;
> > 		}
> > 	}
> > +	if (!xprt_prepare_transmit(task))
> > +		return;
> > +	task->tk_action = call_transmit_status;
> > 	xprt_transmit(task);
> > -	if (task->tk_status < 0)
> > +	if (task->tk_status < 0) {
> 
> The added curly bracket seems incorrect.

Oops. Yes, that's a rebase brain-fart... Thanks for spotting it!
> 
> 
> > 		return;
> > 	if (is_retrans)
> > 		task->tk_client->cl_stats->rpcretrans++;
> > -- 
> > 2.17.1
> > 
> 
> --
> Chuck Lever
> chucklever@gmail.com
> 
> 
>
diff mbox series

Patch

diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index e5ac35e803ad..66ec61347716 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -1949,9 +1949,6 @@  call_transmit(struct rpc_task *task)
 	task->tk_action = call_status;
 	if (task->tk_status < 0)
 		return;
-	if (!xprt_prepare_transmit(task))
-		return;
-	task->tk_action = call_transmit_status;
 	/* Encode here so that rpcsec_gss can use correct sequence number. */
 	if (rpc_task_need_encode(task)) {
 		rpc_xdr_encode(task);
@@ -1965,8 +1962,11 @@  call_transmit(struct rpc_task *task)
 			return;
 		}
 	}
+	if (!xprt_prepare_transmit(task))
+		return;
+	task->tk_action = call_transmit_status;
 	xprt_transmit(task);
-	if (task->tk_status < 0)
+	if (task->tk_status < 0) {
 		return;
 	if (is_retrans)
 		task->tk_client->cl_stats->rpcretrans++;