rx: Add rxi_FlushWriteLocked

Currently, a couple of places in Rx do this:

    MUTEX_EXIT(&call->lock);
    rxi_FlushWrite(call);
    MUTEX_ENTER(&call->lock);

This is a little silly, because if rxi_FlushWrite has anything to do,
it just acquires/drops call->lock again.

This seems like a very minor performance penalty, but in the right
situation it can become more noticeable. Specifically, when an Rx call
on the server ends successfully, rx_EndCall will rxi_FlushWrite (to
send out the last Rx packet to the client) before marking the call as
finished. If the client receives the last Rx packet and starts a new
Rx call on the same channel before the server locks the call again,
the client can receive a BUSY packet (because it looks like the
previous call on the server hasn't finished yet). Nothing breaks, but
this means the client waits 3 seconds to retry.

This situation can probably happen with various rates of success in
almost any situation, but I can see it consistently happen with 'vos
move' when running 'vos' on the same machine as the source fileserver.
It is most noticeable when moving a large number of small volumes
(since you must wait an extra 3+ seconds per volume, where nothing is
happening).

To avoid this, create a new variant of rxi_FlushWrite, called
rxi_FlushWriteLocked. This just assumes the call lock is already held
by the caller, and avoids one extra lock/unlock pair. This is not the
only place where we unlock/lock the call during the rx_EndCall
situation described above, but it seems to be easiest to solve, and
it's enough (for me) to avoid the 3-second delay in the 'vos move'
scenario. Ideally, Rx should be able to atomically 'end' a call while
sending out this last packet, but for now, this commit is easy to do.

Note that rxi_FlushWrite previously didn't do much of note before
locking the call. It did call rxi_FreePackets without the call lock,
but calling that with the call lock is also fine; other callers do
that.

Change-Id: I8f71e86f6c1f6019abea21c205d2b26b7da0d808
Reviewed-on: https://gerrit.openafs.org/12421
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

diff --git a/src/rx/rx.c b/src/rx/rx.c
index e4ac431..be9949b 100644 (file)
--- a/src/rx/rx.c
+++ b/src/rx/rx.c
@@ -2371,9 +2371,7 @@ rx_EndCall(struct rx_call *call, afs_int32 rc)
            MUTEX_ENTER(&call->lock);
        }
        if (call->app.mode == RX_MODE_SENDING) {
-            MUTEX_EXIT(&call->lock);
-           rxi_FlushWrite(call);
-            MUTEX_ENTER(&call->lock);
+           rxi_FlushWriteLocked(call);
        }
        rxi_calltrace(RX_CALL_END, call);
        /* Call goes to hold state until reply packets are acknowledged */

diff --git a/src/rx/rx_prototypes.h b/src/rx/rx_prototypes.h
index 68beade..8af5308 100644 (file)
--- a/src/rx/rx_prototypes.h
+++ b/src/rx/rx_prototypes.h
@@ -481,6 +481,7 @@ extern int rxi_WritevProc(struct rx_call *call, struct iovec *iov, int nio,
 extern int rx_WritevProc(struct rx_call *call, struct iovec *iov, int nio,
                         int nbytes);
 extern void rxi_FlushWrite(struct rx_call *call);
+extern void rxi_FlushWriteLocked(struct rx_call *call);
 extern void rx_FlushWrite(struct rx_call *call);
 
 

diff --git a/src/rx/rx_rdwr.c b/src/rx/rx_rdwr.c
index 4b33a14..4124658 100644 (file)
--- a/src/rx/rx_rdwr.c
+++ b/src/rx/rx_rdwr.c
@@ -189,9 +189,7 @@ rxi_ReadProc(struct rx_call *call, char *buf,
                        return 0;
                    }
                    if (call->app.mode == RX_MODE_SENDING) {
-                        MUTEX_EXIT(&call->lock);
-                       rxi_FlushWrite(call);
-                        MUTEX_ENTER(&call->lock);
+                       rxi_FlushWriteLocked(call);
                        continue;
                    }
                }
@@ -1223,12 +1221,13 @@ rx_WritevProc(struct rx_call *call, struct iovec *iov, int nio, int nbytes)
 }
 
 /* Flush any buffered data to the stream, switch to read mode
- * (clients) or to EOF mode (servers)
+ * (clients) or to EOF mode (servers). If 'locked' is nonzero, call->lock must
+ * be already held.
  *
  * LOCKS HELD: called at netpri.
  */
-void
-rxi_FlushWrite(struct rx_call *call)
+static void
+FlushWrite(struct rx_call *call, int locked)
 {
     struct rx_packet *cp = NULL;
 
@@ -1259,7 +1258,10 @@ rxi_FlushWrite(struct rx_call *call)
        }
 #endif
 
-        MUTEX_ENTER(&call->lock);
+        if (!locked) {
+            MUTEX_ENTER(&call->lock);
+        }
+
         if (call->error)
             call->app.mode = RX_MODE_ERROR;
 
@@ -1307,10 +1309,24 @@ rxi_FlushWrite(struct rx_call *call)
        if (!(call->flags & RX_CALL_FAST_RECOVER)) {
            rxi_Start(call, 0);
        }
-        MUTEX_EXIT(&call->lock);
+        if (!locked) {
+            MUTEX_EXIT(&call->lock);
+        }
     }
 }
 
+void
+rxi_FlushWrite(struct rx_call *call)
+{
+    FlushWrite(call, 0);
+}
+
+void
+rxi_FlushWriteLocked(struct rx_call *call)
+{
+    FlushWrite(call, 1);
+}
+
 /* Flush any buffered data to the stream, switch to read mode
  * (clients) or to EOF mode (servers) */
 void
@@ -1318,6 +1334,6 @@ rx_FlushWrite(struct rx_call *call)
 {
     SPLVAR;
     NETPRI;
-    rxi_FlushWrite(call);
+    FlushWrite(call, 0);
     USERPRI;
 }