From: Simon Wilkinson Date: Sat, 18 Jun 2011 12:01:35 +0000 (+0100) Subject: rx: Don't wait for TQ busy when entering recovery X-Git-Tag: upstream/1.6.0.pre7^2~31 X-Git-Url: https://git.michaelhowe.org/gitweb/?a=commitdiff_plain;h=c4e359b69a9f5e8a21cc06f7338595404d290cd2;p=packages%2Fo%2Fopenafs.git rx: Don't wait for TQ busy when entering recovery Two different threads can cause a call to enter recovery. The event thread will move a call into recovery as a result of a timeout, or the listener thread will move it there following a fast retransmit. In both of these cases, recovery looks different. In the case of a timeout, we enter slow start, starting as if we were begininning transmission for the first time. Following fast retransmit, we enter fast recovery, with different starting parameters than those coming from slow start. As a reslt, the current behaviour, where either call sitting in FAST_RECOVERY_WAIT causes the other to simply return is inappropriate. Further investigation indiciates that FAST_RECOVER_WAIT is actually uncessary. There is no harm caused to a thread which is currently blocked on the network in the middle of a transmit, in adjusting the window size underneath it. As both of these states collapse the window, that thread will simply cease sending earlier. So, simplify the code, and remove the potential race between event and listener by removing the FAST_RECOVER_WAIT state. Reviewed-on: http://gerrit.openafs.org/4867 Reviewed-by: Derrick Brashear Reviewed-by: Jeffrey Altman Tested-by: Jeffrey Altman Tested-by: BuildBot (cherry picked from commit 0b9c9e9973e8d32cdfe1fc884fb2c310cedc0404) Change-Id: I47525eb4cf0bb6d049094c7f98f8cc79be9ef51e Reviewed-on: http://gerrit.openafs.org/4939 Reviewed-by: Derrick Brashear Tested-by: Derrick Brashear --- diff --git a/src/rx/rx.c b/src/rx/rx.c index 9612892c0..fc7e3ce71 100644 --- a/src/rx/rx.c +++ b/src/rx/rx.c @@ -4665,17 +4665,6 @@ rxi_ReceiveAckPacket(struct rx_call *call, struct rx_packet *np, call->nCwindAcks = 0; } else if (nNacked && call->nNacks >= (u_short) rx_nackThreshold) { /* Three negative acks in a row trigger congestion recovery */ -#ifdef AFS_GLOBAL_RXLOCK_KERNEL - MUTEX_EXIT(&peer->peer_lock); - if (call->flags & RX_CALL_FAST_RECOVER_WAIT) { - /* someone else is waiting to start recovery */ - return np; - } - call->flags |= RX_CALL_FAST_RECOVER_WAIT; - rxi_WaitforTQBusy(call); - MUTEX_ENTER(&peer->peer_lock); -#endif /* AFS_GLOBAL_RXLOCK_KERNEL */ - call->flags &= ~RX_CALL_FAST_RECOVER_WAIT; call->flags |= RX_CALL_FAST_RECOVER; call->ssthresh = MAX(4, MIN((int)call->cwind, (int)call->twind)) >> 1; call->cwind = @@ -5849,6 +5838,7 @@ rxi_SendXmitList(struct rx_call *call, struct rx_packet **list, int len, int istack) { int i; + int recovery; struct xmitlist working; struct xmitlist last; @@ -5860,6 +5850,8 @@ rxi_SendXmitList(struct rx_call *call, struct rx_packet **list, int len, working.len = 0; working.resending = 0; + recovery = call->flags & RX_CALL_FAST_RECOVER; + for (i = 0; i < len; i++) { /* Does the current packet force us to flush the current list? */ if (working.len > 0 @@ -5873,7 +5865,8 @@ rxi_SendXmitList(struct rx_call *call, struct rx_packet **list, int len, rxi_SendList(call, &last, istack, 1); /* If the call enters an error state stop sending, or if * we entered congestion recovery mode, stop sending */ - if (call->error || (call->flags & RX_CALL_FAST_RECOVER_WAIT)) + if (call->error + || (!recovery && (call->flags & RX_CALL_FAST_RECOVER))) return; } last = working; @@ -5900,7 +5893,7 @@ rxi_SendXmitList(struct rx_call *call, struct rx_packet **list, int len, /* If the call enters an error state stop sending, or if * we entered congestion recovery mode, stop sending */ if (call->error - || (call->flags & RX_CALL_FAST_RECOVER_WAIT)) + || (!recovery && (call->flags & RX_CALL_FAST_RECOVER))) return; } last = working; @@ -5933,7 +5926,8 @@ rxi_SendXmitList(struct rx_call *call, struct rx_packet **list, int len, rxi_SendList(call, &last, istack, morePackets); /* If the call enters an error state stop sending, or if * we entered congestion recovery mode, stop sending */ - if (call->error || (call->flags & RX_CALL_FAST_RECOVER_WAIT)) + if (call->error + || (!recovery && (call->flags & RX_CALL_FAST_RECOVER))) return; } if (morePackets) { @@ -5978,18 +5972,6 @@ rxi_Resend(struct rxevent *event, void *arg0, void *arg1, int istack) goto out; } -#ifdef AFS_GLOBAL_RXLOCK_KERNEL - if (call->flags & RX_CALL_FAST_RECOVER_WAIT) { - /* Someone else is waiting to start recovery */ - goto out; - } - call->flags |= RX_CALL_FAST_RECOVER_WAIT; - rxi_WaitforTQBusy(call); - call->flags &= ~RX_CALL_FAST_RECOVER_WAIT; - if (call->error) - goto out; -#endif - /* We're in loss recovery */ call->flags |= RX_CALL_FAST_RECOVER; @@ -6089,13 +6071,6 @@ rxi_Start(struct rx_call *call, int istack) nXmitPackets = 0; maxXmitPackets = MIN(call->twind, call->cwind); for (queue_Scan(&call->tq, p, nxp, rx_packet)) { - if (call->flags & RX_CALL_FAST_RECOVER_WAIT) { - /* We shouldn't be sending packets if a thread is waiting - * to initiate congestion recovery */ - dpf(("call %d waiting to initiate fast recovery\n", - *(call->callNumber))); - break; - } if ((nXmitPackets) && (call->flags & RX_CALL_FAST_RECOVER)) { /* Only send one packet during fast recovery */ @@ -6158,15 +6133,6 @@ rxi_Start(struct rx_call *call, int istack) } #ifdef AFS_GLOBAL_RXLOCK_KERNEL - /* - * TQ references no longer protected by this flag; they must remain - * protected by the global lock. - */ - if (call->flags & RX_CALL_FAST_RECOVER_WAIT) { - call->flags &= ~RX_CALL_TQ_BUSY; - rxi_WakeUpTransmitQueue(call); - return; - } if (call->error) { /* We went into the error state while sending packets. Now is * the time to reset the call. This will also inform the using diff --git a/src/rx/rx.h b/src/rx/rx.h index 7f5b1403f..f744b9674 100644 --- a/src/rx/rx.h +++ b/src/rx/rx.h @@ -634,7 +634,7 @@ struct rx_call { #define RX_CALL_TQ_SOME_ACKED 512 /* rxi_Start needs to discard ack'd packets. */ #define RX_CALL_TQ_WAIT 1024 /* Reader is waiting for TQ_BUSY to be reset */ #define RX_CALL_FAST_RECOVER 2048 /* call is doing congestion recovery */ -#define RX_CALL_FAST_RECOVER_WAIT 4096 /* thread is waiting to start recovery */ +/* 4096 was RX_CALL_FAST_RECOVER_WAIT */ #define RX_CALL_SLOW_START_OK 8192 /* receiver acks every other packet */ #define RX_CALL_IOVEC_WAIT 16384 /* waiting thread is using an iovec */ #define RX_CALL_HAVE_LAST 32768 /* Last packet has been received */ diff --git a/src/rx/rx_rdwr.c b/src/rx/rx_rdwr.c index e3066d2d1..56ea3bccd 100644 --- a/src/rx/rx_rdwr.c +++ b/src/rx/rx_rdwr.c @@ -762,10 +762,10 @@ rxi_WriteProc(struct rx_call *call, char *buf, call->tqc++; #endif /* RXDEBUG_PACKET */ cp = (struct rx_packet *)0; - if (! - (call-> - flags & (RX_CALL_FAST_RECOVER | - RX_CALL_FAST_RECOVER_WAIT))) { + /* If the call is in recovery, let it exhaust its current + * retransmit queue before forcing it to send new packets + */ + if (!(call->flags & (RX_CALL_FAST_RECOVER))) { rxi_Start(call, 0); } } else if (cp) { @@ -1276,7 +1276,10 @@ rxi_WritevProc(struct rx_call *call, struct iovec *iov, int nio, int nbytes) queue_SpliceAppend(&call->tq, &tmpq); - if (!(call->flags & (RX_CALL_FAST_RECOVER | RX_CALL_FAST_RECOVER_WAIT))) { + /* If the call is in recovery, let it exhaust its current retransmit + * queue before forcing it to send new packets + */ + if (!(call->flags & RX_CALL_FAST_RECOVER)) { rxi_Start(call, 0); } @@ -1403,9 +1406,11 @@ rxi_FlushWrite(struct rx_call *call) #ifdef RXDEBUG_PACKET call->tqc++; #endif /* RXDEBUG_PACKET */ - if (! - (call-> - flags & (RX_CALL_FAST_RECOVER | RX_CALL_FAST_RECOVER_WAIT))) { + + /* If the call is in recovery, let it exhaust its current retransmit + * queue before forcing it to send new packets + */ + if (!(call->flags & RX_CALL_FAST_RECOVER)) { rxi_Start(call, 0); } MUTEX_EXIT(&call->lock);