server tcp performance patches
Subsystems (3)
We previously attempted to turn on autotuning of nfsd's tcp receive buffers, enabling better performance on large bandwidth-delay-product networks, but ran into some regressions on local gigabit networks.
At the time Trond proposed modifying the server to receive partial rpc calls as they arrive instead of waiting for the entire request to arrive, reasoning that that would a) free up receive buffer space sooner, causing the server to advertise a larger window soon, and b) avoid theoretical deadlocks possible if the receive buffer ever fell below the minimum required to hold a request.
That seemed to solve the observed regression, but we didn't completely understand why, so I put off applying the patches.
I still don't completely understand the cause of the original regression, but it seems like a reasonable thing to do anyway, and solves an immediate problem, so I've updated Trond's original patch (and split it up slightly)--the main changes required were just to adapt to the 4.1 backchannel reply receive code.
I'm considering queueing this up for 2.6.40.
--b.
PATCH 1/7 SUNRPC: requeue tcp socket less frequently
From: Trond Myklebust <[email protected]>
Don't requeue the socket in some cases where we know it's unnecessary.
Signed-off-by Trond Myklebust <[email protected]>
Signed-off-by J. Bruce Fields <[email protected]>
--- net/sunrpc/svcsock.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)
net/sunrpc/svcsock.c +4 -1
@@ -959,7 +959,6 @@ static int svc_tcp_recv_record(struct svc_sock *svsk, struct svc_rqst *rqstp) goto err_again;/* record not complete */ } len = svsk->sk_reclen; -set_bit (XPT_DATA , &svsk->sk_xprt.xpt_flags);return len; error:
@@ -1109,6 +1108,10 @@ out:/* Reset TCP read info */ svsk->sk_reclen =0 ; svsk->sk_tcplen =0 ; +/* If we have more data, signal svc_xprt_enqueue() to try again */ +if (svc_recv_available (svsk) >sizeof (rpc_fraghdr)) +set_bit (XPT_DATA , &svsk->sk_xprt.xpt_flags); +svc_xprt_copy_addrs (rqstp, &svsk->sk_xprt);if (serv->sv_stats)
Signature · 1 line
1.7.1
PATCH 2/7 SUNRPC: svc_tcp_recvfrom cleanup
In reply to J. Bruce FieldsFrom: Trond Myklebust <[email protected]>
Minor cleanup in preparation for later patches.
Signed-off-by Trond Myklebust <[email protected]>
Signed-off-by J. Bruce Fields <[email protected]>
--- net/sunrpc/svcsock.c | 34 +++++++++++++++------------------- 1 files changed, 15 insertions(+), 19 deletions(-)
net/sunrpc/svcsock.c +15 -19
@@ -887,6 +887,7 @@ failed:static int svc_tcp_recv_record (struct svc_sock *svsk,struct svc_rqst *rqstp) {struct svc_serv *serv = svsk->sk_xprt.xpt_server; +unsigned int want;int len;if (test_and_clear_bit (XPT_CHNGBUF , &svsk->sk_xprt.xpt_flags))
@@ -909,9 +910,9 @@ static int svc_tcp_recv_record(struct svc_sock *svsk, struct svc_rqst *rqstp)clear_bit (XPT_DATA , &svsk->sk_xprt.xpt_flags);if (svsk->sk_tcplen <sizeof (rpc_fraghdr)) { -int want =sizeof (rpc_fraghdr) - svsk->sk_tcplen;struct kvec iov; + want =sizeof (rpc_fraghdr) - svsk->sk_tcplen; iov.iov_base = ((char *) &svsk->sk_reclen) + svsk->sk_tcplen; iov.iov_len = want;if ((len =svc_recvfrom (rqstp, &iov,1 , want)) <0 )
@@ -1034,8 +1035,9 @@ static int svc_tcp_recvfrom(struct svc_rqst *rqstp)struct svc_serv *serv = svsk->sk_xprt.xpt_server;int len;struct kvec *vec; -int pnum, vlen;struct rpc_rqst *req =NULL ; +unsigned int vlen; +int pnum; dprintk("svc: tcp_recv %p data %d conn %d close %d\n", svsk, test_bit(XPT_DATA, &svsk->sk_xprt.xpt_flags),
@@ -1066,7 +1068,7 @@ static int svc_tcp_recvfrom(struct svc_rqst *rqstp) } pnum =1 ; -while (vlen < len) { +while (vlen < svsk->sk_reclen -8 ) { vec[pnum].iov_base = (req) ?page_address (req->rq_private_buf.pages[pnum -1 ]) :page_address (rqstp->rq_pages[pnum]);
@@ -1077,29 +1079,23 @@ static int svc_tcp_recvfrom(struct svc_rqst *rqstp) rqstp->rq_respages = &rqstp->rq_pages[pnum];/* Now receive data */ - len =svc_recvfrom (rqstp, vec, pnum, len); + len =svc_recvfrom (rqstp, vec, pnum, svsk->sk_reclen -8 );if (len <0 ) goto err_again; -/* -* Account for the 8 bytes we read earlier -*/ - len +=8 ; -if (req) { -xprt_complete_rqst (req->rq_task, len); - len =0 ; +xprt_complete_rqst (req->rq_task, svsk->sk_reclen); + rqstp->rq_arg.len =0 ; goto out; } -dprintk ("svc: TCP complete record (%d bytes)\n" , len); - rqstp->rq_arg.len = len; +dprintk ("svc: TCP complete record (%d bytes)\n" , svsk->sk_reclen); + rqstp->rq_arg.len = svsk->sk_reclen; rqstp->rq_arg.page_base =0 ; -if (len <= rqstp->rq_arg.head[0 ].iov_len) { - rqstp->rq_arg.head[0 ].iov_len = len; +if (rqstp->rq_arg.len <= rqstp->rq_arg.head[0 ].iov_len) { + rqstp->rq_arg.head[0 ].iov_len = rqstp->rq_arg.len; rqstp->rq_arg.page_len =0 ; - }else { - rqstp->rq_arg.page_len = len - rqstp->rq_arg.head[0 ].iov_len; - } + }else + rqstp->rq_arg.page_len = rqstp->rq_arg.len - rqstp->rq_arg.head[0 ].iov_len; rqstp->rq_xprt_ctxt =NULL ; rqstp->rq_prot =IPPROTO_TCP ;
@@ -1117,7 +1113,7 @@ out:if (serv->sv_stats) serv->sv_stats->nettcpcnt++; -return len; +return rqstp->rq_arg.len; err_again:if (len == -EAGAIN ) {
Signature · 1 line
1.7.1
PATCH 3/7 svcrpc: note network-order types in svc_process_calldir
In reply to J. Bruce FieldsSigned-off-by J. Bruce Fields <[email protected]>
--- net/sunrpc/svcsock.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
net/sunrpc/svcsock.c +3 -3
@@ -976,9 +976,9 @@ static int svc_process_calldir(struct svc_sock *svsk, struct svc_rqst *rqstp, struct rpc_rqst **reqpp, struct kvec *vec) { struct rpc_rqst *req = NULL; -u32 *p; -u32 xid; -u32 calldir; +__be32 *p; +__be32 xid; +__be32 calldir;int len; len =svc_recvfrom (rqstp, vec,1 ,8 );
Signature · 1 line
1.7.1
PATCH 4/7 svcrpc: close connection if client sends short packet
In reply to J. Bruce FieldsIf the client sents a record too short to contain even the beginning of the rpc header, then just close the connection.
The current code drops the record data and continues. I don't see the point. It's a hopeless situation and simpler just to cut off the connection completely.
Signed-off-by J. Bruce Fields <[email protected]>
--- net/sunrpc/svcsock.c | 21 +++++++-------------- 1 files changed, 7 insertions(+), 14 deletions(-)
net/sunrpc/svcsock.c +7 -14
@@ -949,6 +949,9 @@ static int svc_tcp_recv_record(struct svc_sock *svsk, struct svc_rqst *rqstp) } } +if (svsk->sk_reclen <8 ) + goto err_delete;/* client is nuts. */ +/* Check whether enough data is available */ len =svc_recv_available (svsk);if (len <0 )
@@ -1052,20 +1055,10 @@ static int svc_tcp_recvfrom(struct svc_rqst *rqstp) vec[0 ] = rqstp->rq_arg.head[0 ]; vlen =PAGE_SIZE ; -/* -* We have enough data for the whole tcp record. Let's try and read the -* first 8 bytes to get the xid and the call direction. We can use this -* to figure out if this is a call or a reply to a callback. If -* sk_reclen is < 8 (xid and calldir), then this is a malformed packet. -* In that case, don't bother with the calldir and just read the data. -* It will be rejected in svc_process. -*/ -if (len >=8 ) { - len =svc_process_calldir (svsk, rqstp, &req, vec); -if (len <0 ) - goto err_again; - vlen -=8 ; - } + len =svc_process_calldir (svsk, rqstp, &req, vec); +if (len <0 ) + goto err_again; + vlen -=8 ; pnum =1 ;while (vlen < svsk->sk_reclen -8 ) {
Signature · 1 line
1.7.1
PATCH 5/7 svcrpc: copy cb reply instead of pages
In reply to J. Bruce FieldsFrom: Trond Myklebust <[email protected]>
It's much simpler just to copy the cb reply data than to play tricks with pages. Callback replies will typically be very small (at least until we implement cb_getattr, in which case files with very long ACLs could pose a problem), so there's no loss in efficiency.
Signed-off-by J. Bruce Fields <[email protected]>
--- net/sunrpc/svcsock.c | 122 +++++++++++++++++++++++--------------------------- 1 files changed, 56 insertions(+), 66 deletions(-)
net/sunrpc/svcsock.c +56 -66
@@ -975,57 +975,58 @@ static int svc_tcp_recv_record(struct svc_sock *svsk, struct svc_rqst *rqstp)return -EAGAIN ; } -static int svc_process_calldir (struct svc_sock *svsk,struct svc_rqst *rqstp, -struct rpc_rqst **reqpp,struct kvec *vec) +static int receive_cb_reply (struct svc_sock *svsk,struct svc_rqst *rqstp) { +struct rpc_xprt *bc_xprt = svsk->sk_xprt.xpt_bc_xprt;struct rpc_rqst *req =NULL ; -__be32 *p; +struct kvec *src, *dst; +__be32 *p = (__be32 *)rqstp->rq_arg.head[0 ].iov_base;__be32 xid;__be32 calldir; -int len; - - len =svc_recvfrom (rqstp, vec,1 ,8 ); -if (len <0 ) - goto error; - p = (u32 *)rqstp->rq_arg.head[0 ].iov_base; xid = *p++; calldir = *p; -if (calldir ==0 ) { -/* REQUEST is the most common case */ - vec[0 ] = rqstp->rq_arg.head[0 ]; - }else { -/* REPLY */ -struct rpc_xprt *bc_xprt = svsk->sk_xprt.xpt_bc_xprt; - -if (bc_xprt) - req =xprt_lookup_rqst (bc_xprt, xid); - -if (!req) { -printk (KERN_NOTICE -"%s: Got unrecognized reply: " -"calldir 0x%x xpt_bc_xprt %p xid %08x\n" , - __func__,ntohl (calldir), - bc_xprt, xid); - vec[0 ] = rqstp->rq_arg.head[0 ]; - goto out; - } +if (bc_xprt) + req =xprt_lookup_rqst (bc_xprt, xid); -memcpy (&req->rq_private_buf, &req->rq_rcv_buf, -sizeof (struct xdr_buf )); -/* copy the xid and call direction */ -memcpy (req->rq_private_buf.head[0 ].iov_base, - rqstp->rq_arg.head[0 ].iov_base,8 ); - vec[0 ] = req->rq_private_buf.head[0 ]; +if (!req) { +printk (KERN_NOTICE +"%s: Got unrecognized reply: " +"calldir 0x%x xpt_bc_xprt %p xid %08x\n" , + __func__,ntohl (calldir), + bc_xprt, xid); +return -EAGAIN ; } - out: - vec[0 ].iov_base +=8 ; - vec[0 ].iov_len -=8 ; - len = svsk->sk_reclen -8 ; - error: - *reqpp = req; -return len; + +memcpy (&req->rq_private_buf, &req->rq_rcv_buf,sizeof (struct xdr_buf )); +/* +* XXX!: cheating for now! Only copying HEAD. +* But we know this is good enough for now (in fact, for any +* callback reply in the forseeable future). +*/ + dst = &req->rq_private_buf.head[0 ]; + src = &rqstp->rq_arg.head[0 ]; +if (dst->iov_len < src->iov_len) +return -EAGAIN ;/* whatever; just giving up. */ +memcpy (dst->iov_base, src->iov_base, src->iov_len); +xprt_complete_rqst (req->rq_task, svsk->sk_reclen); + rqstp->rq_arg.len =0 ; +return 0 ; +} + +static int copy_pages_to_kvecs (struct kvec *vec,struct page **pages,int len) +{ +int i =0 ; +int t =0 ; + +while (t < len) { + vec[i].iov_base =page_address (pages[i]); + vec[i].iov_len =PAGE_SIZE ; + i++; + t +=PAGE_SIZE ; + } +return i; }/*
@@ -1038,8 +1039,8 @@ static int svc_tcp_recvfrom(struct svc_rqst *rqstp)struct svc_serv *serv = svsk->sk_xprt.xpt_server;int len;struct kvec *vec; -struct rpc_rqst *req =NULL ; -unsigned int vlen; +__be32 *p; +__be32 calldir;int pnum; dprintk("svc: tcp_recv %p data %d conn %d close %d\n",
@@ -1052,35 +1053,17 @@ static int svc_tcp_recvfrom(struct svc_rqst *rqstp) goto error; vec = rqstp->rq_vec; - vec[0 ] = rqstp->rq_arg.head[0 ]; - vlen =PAGE_SIZE ; - len =svc_process_calldir (svsk, rqstp, &req, vec); -if (len <0 ) - goto err_again; - vlen -=8 ; - - pnum =1 ; -while (vlen < svsk->sk_reclen -8 ) { - vec[pnum].iov_base = (req) ? -page_address (req->rq_private_buf.pages[pnum -1 ]) : -page_address (rqstp->rq_pages[pnum]); - vec[pnum].iov_len =PAGE_SIZE ; - pnum++; - vlen +=PAGE_SIZE ; - } + pnum =copy_pages_to_kvecs (&vec[0 ], &rqstp->rq_pages[0 ], + svsk->sk_reclen); + rqstp->rq_respages = &rqstp->rq_pages[pnum];/* Now receive data */ - len =svc_recvfrom (rqstp, vec, pnum, svsk->sk_reclen -8 ); + len =svc_recvfrom (rqstp, vec, pnum, svsk->sk_reclen);if (len <0 ) goto err_again; -if (req) { -xprt_complete_rqst (req->rq_task, svsk->sk_reclen); - rqstp->rq_arg.len =0 ; - goto out; - }dprintk ("svc: TCP complete record (%d bytes)\n" , svsk->sk_reclen); rqstp->rq_arg.len = svsk->sk_reclen; rqstp->rq_arg.page_base =0 ;
@@ -1093,7 +1076,14 @@ static int svc_tcp_recvfrom(struct svc_rqst *rqstp) rqstp->rq_xprt_ctxt =NULL ; rqstp->rq_prot =IPPROTO_TCP ; -out: + p = (__be32 *)rqstp->rq_arg.head[0 ].iov_base; + calldir = p[1 ]; +if (calldir) { + len =receive_cb_reply (svsk, rqstp); +if (len <0 ) + goto err_again; + } +/* Reset TCP read info */ svsk->sk_reclen =0 ; svsk->sk_tcplen =0 ;
Signature · 1 line
1.7.1
PATCH 6/7 SUNRPC: Don't wait for full record to receive tcp data
In reply to J. Bruce FieldsEnsure that we immediately read and buffer data from the incoming TCP stream so that we grow the receive window quickly, and don't deadlock on large READ or WRITE requests.
Also do some minor exit cleanup.
Signed-off-by Trond Myklebust <[email protected]>
Signed-off-by J. Bruce Fields <[email protected]>
--- include/linux/sunrpc/svcsock.h | 1 + net/sunrpc/svcsock.c | 144 +++++++++++++++++++++++++++++++--------- 2 files changed, 113 insertions(+), 32 deletions(-)
include/linux/sunrpc/svcsock.h +1 -0
@@ -28,6 +28,7 @@ struct svc_sock {/* private TCP part */ u32 sk_reclen;/* length of record */ u32 sk_tcplen;/* current read length */ +struct page * sk_pages[RPCSVC_MAXPAGES ];/* received data */ };/*
net/sunrpc/svcsock.c +112 -32
227 lines not on this page · raw message
Signature · 1 line
1.7.1
PATCH 7/7 svcrpc: take advantage of tcp autotuning
In reply to J. Bruce FieldsFrom: Olga Kornievskaia <[email protected]>
Allow the NFSv4 server to make use of TCP autotuning behaviour, which was previously disabled by setting the sk_userlocks variable.
Set the receive buffers to be big enough to receive the whole RPC request, and set this for the listening socket, not the accept socket.
Remove the code that readjusts the receive/send buffer sizes for the accepted socket. Previously this code was used to influence the TCP window management behaviour, which is no longer needed when autotuning is enabled.
This can improve IO bandwidth on networks with high bandwidth-delay products, where a large tcp window is required. It also simplifies performance tuning, since getting adequate tcp buffers previously required increasing the number of nfsd threads.
Signed-off-by Olga Kornievskaia <[email protected]>
Cc Jim Rees <[email protected]>
Signed-off-by J. Bruce Fields <[email protected]>
--- net/sunrpc/svcsock.c | 35 +++++++---------------------------- 1 files changed, 7 insertions(+), 28 deletions(-)
net/sunrpc/svcsock.c +7 -28
@@ -436,7 +436,6 @@ static void svc_sock_setbufsize(struct socket *sock, unsigned int snd,lock_sock (sock->sk); sock->sk->sk_sndbuf = snd *2 ; sock->sk->sk_rcvbuf = rcv *2 ; - sock->sk->sk_userlocks |=SOCK_SNDBUF_LOCK |SOCK_RCVBUF_LOCK ; sock->sk->sk_write_space (sock->sk);release_sock (sock->sk);#endif
@@ -967,23 +966,6 @@ static int svc_tcp_recv_record(struct svc_sock *svsk, struct svc_rqst *rqstp)unsigned int want;int len; -if (test_and_clear_bit (XPT_CHNGBUF , &svsk->sk_xprt.xpt_flags)) -/* sndbuf needs to have room for one request -* per thread, otherwise we can stall even when the -* network isn't a bottleneck. -* -* We count all threads rather than threads in a -* particular pool, which provides an upper bound -* on the number of threads which will access the socket. -* -* rcvbuf just needs to be able to hold a few requests. -* Normally they will be removed from the queue -* as soon a a complete request arrives. -*/ -svc_sock_setbufsize (svsk->sk_sock, - (serv->sv_nrthreads+3 ) * serv->sv_max_mesg, -3 * serv->sv_max_mesg); -clear_bit (XPT_DATA , &svsk->sk_xprt.xpt_flags);if (svsk->sk_tcplen <sizeof (rpc_fraghdr)) {
@@ -1361,15 +1343,6 @@ static void svc_tcp_init(struct svc_sock *svsk, struct svc_serv *serv)tcp_sk (sk)->nonagle |=TCP_NAGLE_OFF ; -/* initialise setting must have enough space to -* receive and respond to one request. -* svc_tcp_recvfrom will re-adjust if necessary -*/ -svc_sock_setbufsize (svsk->sk_sock, -3 * svsk->sk_xprt.xpt_server->sv_max_mesg, -3 * svsk->sk_xprt.xpt_server->sv_max_mesg); - -set_bit (XPT_CHNGBUF , &svsk->sk_xprt.xpt_flags);set_bit (XPT_DATA , &svsk->sk_xprt.xpt_flags);if (sk->sk_state !=TCP_ESTABLISHED )set_bit (XPT_CLOSE , &svsk->sk_xprt.xpt_flags);
@@ -1433,8 +1406,14 @@ static struct svc_sock *svc_setup_socket(struct svc_serv *serv,/* Initialize the socket */ if (sock->type ==SOCK_DGRAM )svc_udp_init (svsk, serv); -else +else { +/* initialise setting must have enough space to +* receive and respond to one request. +*/ +svc_sock_setbufsize (svsk->sk_sock,4 * serv->sv_max_mesg, +4 * serv->sv_max_mesg);svc_tcp_init (svsk, serv); + }dprintk ("svc: svc_setup_socket created %p (inet %p)\n" , svsk, svsk->sk_sk);
Signature · 1 line
1.7.1