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.

Change-Id: Ic2e7606136ca04c869685345b63101c346ce702b
Reviewed-on: http://gerrit.openafs.org/4867
Reviewed-by: Derrick Brashear <shadow@dementia.org>
Reviewed-by: Jeffrey Altman <jaltman@openafs.org>
Tested-by: Jeffrey Altman <jaltman@openafs.org>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

diff --git a/src/rx/rx.c b/src/rx/rx.c
index 6e50c223df6f1acae274f74042c39406aac363c9..1bad02d0a74b39bb32b6fef12bc3f5e2302cfaa0 100644 (file)
--- a/src/rx/rx.c
+++ b/src/rx/rx.c
@@ -4641,17 +4641,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 =
@@ -5818,6 +5807,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;
 
@@ -5829,6 +5819,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
@@ -5842,7 +5834,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;
@@ -5869,7 +5862,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;
@@ -5902,7 +5895,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) {
@@ -5947,18 +5941,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;
 
@@ -6054,13 +6036,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 */
@@ -6123,15 +6098,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 df446652fa54d9f034e8ac27c58309fda26afefe..f45fbd078aab8571e840160e05ebc301d05652b8 100644 (file)
--- a/src/rx/rx.h
+++ b/src/rx/rx.h
@@ -628,7 +628,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 adaf545b8f942ad59f41eaa81eac7e95a93190db..019a4f7b3a7f189a937ab885611ea010a5558737 100644 (file)
--- a/src/rx/rx_rdwr.c
+++ b/src/rx/rx_rdwr.c
@@ -733,10 +733,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) {
@@ -1247,7 +1247,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);
     }
 
@@ -1374,9 +1377,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);