rx: Don't wait for TQ busy when entering recovery
authorSimon Wilkinson <sxw@your-file-system.com>
Sat, 18 Jun 2011 12:01:35 +0000 (13:01 +0100)
committerDerrick Brashear <shadow@dementia.org>
Wed, 22 Jun 2011 01:59:13 +0000 (18:59 -0700)
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>

src/rx/rx.c
src/rx/rx.h
src/rx/rx_rdwr.c

index 6e50c22..1bad02d 100644 (file)
@@ -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
index df44665..f45fbd0 100644 (file)
@@ -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 */
index adaf545..019a4f7 100644 (file)
@@ -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);