If an RPC request has to be resent due to a timeout, it turns out

that call_encode() may cause rq_rcv_buf to be reset despite the fact
that a reply might be delivered at any moment by a softirq.

This typically results in 'NFS: server cheating in read reply'
error messages.

Solve by adding rq_private_buf, which is updated atomically from
rq_rcv_buf.

diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h
index b360e54f894a54e4767d87070cdc5eea3acdaa7a..2687ba7b9f139da97afd6f3a00b6a649c042afa8 100644 (file)
--- a/include/linux/sunrpc/xprt.h
+++ b/include/linux/sunrpc/xprt.h
@@ -98,6 +98,10 @@ struct rpc_rqst {
 
        struct list_head        rq_list;
 
+       struct xdr_buf          rq_private_buf;         /* The receive buffer
+                                                        * used in the softirq.
+                                                        */
+
        /*
         * For authentication (e.g. auth_des)
         */

diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index d616a9e8fd0ecc7158f50a65468c427518ad4eaf..83d8d6e2c77595db86b1902f03acbd1ac6a114b9 100644 (file)
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -797,6 +797,10 @@ call_decode(struct rpc_task *task)
                return;
        }
 
+       /* Check that the softirq receive buffer is valid */
+       WARN_ON(memcmp(&req->rq_rcv_buf, &req->rq_private_buf,
+                               sizeof(req->rq_rcv_buf)) != 0);
+
        /* Verify the RPC header */
        if (!(p = call_verify(task))) {
                if (task->tk_action == NULL)

diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index 2fc38572410cf519d8d0e1b443a1a68428224b57..e572a821d7937361740de4672cf437f08d15fa3c 100644 (file)
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -726,11 +726,11 @@ udp_data_ready(struct sock *sk, int len)
 
        dprintk("RPC: %4d received reply\n", task->tk_pid);
 
-       if ((copied = rovr->rq_rlen) > repsize)
+       if ((copied = rovr->rq_private_buf.len) > repsize)
                copied = repsize;
 
        /* Suck it into the iovec, verify checksum if not done by hw. */
-       if (csum_partial_copy_to_xdr(&rovr->rq_rcv_buf, skb))
+       if (csum_partial_copy_to_xdr(&rovr->rq_private_buf, skb))
                goto out_unlock;
 
        /* Something worked... */
@@ -853,7 +853,7 @@ tcp_read_request(struct rpc_xprt *xprt, skb_reader_t *desc)
                return;
        }
 
-       rcvbuf = &req->rq_rcv_buf;
+       rcvbuf = &req->rq_private_buf;
        len = desc->count;
        if (len > xprt->tcp_reclen - xprt->tcp_offset) {
                skb_reader_t my_desc;
@@ -871,7 +871,7 @@ tcp_read_request(struct rpc_xprt *xprt, skb_reader_t *desc)
        xprt->tcp_copied += len;
        xprt->tcp_offset += len;
 
-       if (xprt->tcp_copied == req->rq_rlen)
+       if (xprt->tcp_copied == req->rq_private_buf.len)
                xprt->tcp_flags &= ~XPRT_COPY_DATA;
        else if (xprt->tcp_offset == xprt->tcp_reclen) {
                if (xprt->tcp_flags & XPRT_LAST_FRAG)
@@ -1130,11 +1130,6 @@ xprt_prepare_transmit(struct rpc_task *task)
                err = -ENOTCONN;
                goto out_unlock;
        }
-
-       if (list_empty(&req->rq_list)) {
-               list_add_tail(&req->rq_list, &xprt->recv);
-               req->rq_received = 0;
-       }
 out_unlock:
        spin_unlock_bh(&xprt->sock_lock);
        return err;
@@ -1159,6 +1154,20 @@ xprt_transmit(struct rpc_task *task)
                *marker = htonl(0x80000000|(req->rq_slen-sizeof(*marker)));
        }
 
+       smp_rmb();
+       if (!req->rq_received) {
+               if (list_empty(&req->rq_list)) {
+                       spin_lock_bh(&xprt->sock_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);
+                       spin_unlock_bh(&xprt->sock_lock);
+               }
+       } else if (!req->rq_bytes_sent)
+               return;
+
        /* Continue transmitting the packet/record. We must be careful
         * to cope with writespace callbacks arriving _after_ we have
         * called xprt_sendmsg().