Trond Myklebust wrote:
On Mon, 2008-05-19 at 13:50 +1000, Peter Leckie wrote:
Don't call __xprt_get_cong() if this is a retransmit.
This prevents __xprt_get_cong() from recursively
incrementing the congestion avoidance window for
retransmitted data.
Signed-off-by: Peter Leckie <pleckie@xxxxxxxxxxxxxxxxx>
Reviewed-by: Greg Banks <gnb@xxxxxxxxxxxxxxxxx>
X-Sgi-Pv: 971446
<http://bugworks/query.cgi/971446>---
Index: linux-2.6.25.3/net/sunrpc/xprt.c
===================================================================
--- linux-2.6.25.3.orig/net/sunrpc/xprt.c
+++ linux-2.6.25.3/net/sunrpc/xprt.c
@@ -224,7 +224,8 @@ int xprt_reserve_xprt_cong(struct rpc_ta
return 1;
goto out_sleep;
}
- if (__xprt_get_cong(xprt, task)) {
+ /*If this is a retransmit don't increment cong*/
+ if ((req && req->rq_ntrans) ||__xprt_get_cong(xprt, task)) {
xprt->snd_task = task;
if (req) {
req->rq_bytes_sent = 0;
Why would we not want to increment the congestion avoidance window on
retransmitted data? On timeout, xprt_adjust_cwnd will call
__xprt_put_cong() prior to the retransmission, so I can't see how this
is a 'recursive increment'.
Trond
That's a good point you raise there I was looking to closely at the tcp
equivalent, the correct fix for this issue would be to implement a timer
function for NFS/RDMA pretty much identical to xs_udp_timer(), as follows:
Implement xprt_rdma_timer() to be called when an RPC times out.
This is needed to decrement the cong after an rpc times out preventing
the congestion aviodance from tripping under retransmitts.
Signed-off-by: Peter Leckie <pleckie@xxxxxxxxxxxxxxxxx>
Reviewed-by: Greg Banks <gnb@xxxxxxxxxxxxxxxxx>
---
Index: linux-2.6.25.3/net/sunrpc/xprtrdma/transport.c
===================================================================
--- linux-2.6.25.3.orig/net/sunrpc/xprtrdma/transport.c
+++ linux-2.6.25.3/net/sunrpc/xprtrdma/transport.c
@@ -450,6 +450,18 @@ out1:
}
/*
+ * xprt_rdma_timer - called when a retransmit timeout occurs on a RDMA
transport
+ * @task: task that timed out
+ *
+ * Adjust the congestion window after a retransmit timeout has occurred.
+ */
+static void
+xprt_rdma_timer(struct rpc_task *task)
+{
+ xprt_adjust_cwnd(task, -ETIMEDOUT);
+}
+
+/*
* Close a connection, during shutdown or timeout/reconnect
*/
static void
@@ -755,7 +767,8 @@ static struct rpc_xprt_ops xprt_rdma_pro
.send_request = xprt_rdma_send_request,
.close = xprt_rdma_close,
.destroy = xprt_rdma_destroy,
- .print_stats = xprt_rdma_print_stats
+ .print_stats = xprt_rdma_print_stats,
+ .timer = xprt_rdma_timer
};
static struct xprt_class xprt_rdma = {
Thanks,
Pete
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
