From 7fffb6b479454560503ba3166151b501381f5a6d Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Wed, 22 Jun 2011 17:27:16 -0400 Subject: [PATCH] SUNRPC: Fix a potential race in between xprt_complete_rqst and xprt_transmit In xprt_transmit, if the test for list_empty(&req->rq_list) is to remain lockless, we need to test for whether or not req->rq_reply_bytes_recvd is set (i.e. we already have a reply) after that test. The reason is that xprt_complete_rqst orders the list deletion and the setting of the req->rq_reply_bytes_recvd. By doing the test of req->rq_reply_bytes_recvd under the spinlock, we avoid an extra smp_rmb(). Also ensure that we turn off autodisconnect whether or not the RPC request expects a reply. Signed-off-by: Trond Myklebust --- net/sunrpc/xprt.c | 34 +++++++++++++++++++++------------- 1 files changed, 21 insertions(+), 13 deletions(-) diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c index ce5eb68..10e1f21 100644 --- a/net/sunrpc/xprt.c +++ b/net/sunrpc/xprt.c @@ -878,23 +878,31 @@ void xprt_transmit(struct rpc_task *task) dprintk("RPC: %5u xprt_transmit(%u)\n", task->tk_pid, req->rq_slen); - if (!req->rq_reply_bytes_recvd) { - if (list_empty(&req->rq_list) && rpc_reply_expected(task)) { - /* - * Add to the list only if we're expecting a reply - */ + if (list_empty(&req->rq_list)) { + /* + * Add to the list only if we're expecting a reply + */ + if (rpc_reply_expected(task)) { spin_lock_bh(&xprt->transport_lock); - /* Update the softirq receive buffer */ - memcpy(&req->rq_private_buf, &req->rq_rcv_buf, - sizeof(req->rq_private_buf)); - /* Add request to the receive list */ - list_add_tail(&req->rq_list, &xprt->recv); + /* Don't put back on the list if we have a reply + * We do this test under the spin lock to avoid + * an extra smp_rmb() betweent the tests of + * req->rq_list and req->rq_reply_bytes_recvd + */ + if (req->rq_reply_bytes_recvd == 0) { + /* Update the softirq receive buffer */ + memcpy(&req->rq_private_buf, &req->rq_rcv_buf, + sizeof(req->rq_private_buf)); + /* Add request to the receive list */ + list_add_tail(&req->rq_list, &xprt->recv); + } spin_unlock_bh(&xprt->transport_lock); xprt_reset_majortimeo(req); - /* Turn off autodisconnect */ - del_singleshot_timer_sync(&xprt->timer); } - } else if (!req->rq_bytes_sent) + /* Turn off autodisconnect */ + del_singleshot_timer_sync(&xprt->timer); + } + if (req->rq_reply_bytes_recvd != 0 && req->rq_bytes_sent == 0) return; req->rq_connect_cookie = xprt->connect_cookie; -- 1.7.5.4