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 <shadow@dementia.org>
Reviewed-by: Jeffrey Altman <jaltman@openafs.org>
Tested-by: Jeffrey Altman <jaltman@openafs.org>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
(cherry picked from commit 0b9c9e9973e8d32cdfe1fc884fb2c310cedc0404)

Change-Id: I47525eb4cf0bb6d049094c7f98f8cc79be9ef51e
Reviewed-on: http://gerrit.openafs.org/4939
Reviewed-by: Derrick Brashear <shadow@dementia.org>
Tested-by: Derrick Brashear <shadow@dementia.org>

diff --git a/src/rx/rx.c b/src/rx/rx.c
index 9612892c05793b80ab0bd1b3e1e7ffd8d870159e..fc7e3ce71653020a1e9a94d006186959101b3ce9 100644 (file)
--- 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 7f5b1403f671aadce349dd0cd62be7be5234756f..f744b9674400c259f99b7862e7ab6b6e51479229 100644 (file)
--- 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 e3066d2d1c3f447d1f6bb5733d93f713b76ce504..56ea3bccd8032d63ca16b656cdfb824366334da7 100644 (file)
--- 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);