Rx: Do not drop call lock in rx_WriteProc* and rx_ReadProc*

rx_WriteProc and rx_ReadProc has special fast logic that
handles the most frequent case.  This code was called
without obtaining the call lock. However, each of these functions
must obtain the call lock for the queue_IsNotEmpty() test and
must re-obtain the call lock if the rxi_XXX variant is required.
Dropping the lock and re-obtaining it is more expensive than
holding it across the memcpy.  Therefore, we shouldn't drop the
lock until we are done.

Change-Id: Icca679567994e91bbbf3afec72b863d986f86582
Reviewed-on: http://gerrit.openafs.org/1108
Reviewed-by: Derrick Brashear <shadow@dementia.org>
Tested-by: Jeffrey Altman <jaltman@openafs.org>
Reviewed-by: Simon Wilkinson <sxw@inf.ed.ac.uk>
Tested-by: Simon Wilkinson <sxw@inf.ed.ac.uk>
Reviewed-by: Jeffrey Altman <jaltman@openafs.org>

diff --git a/src/rx/rx_rdwr.c b/src/rx/rx_rdwr.c
index 876d7be..9e25501 100644 (file)
--- a/src/rx/rx_rdwr.c
+++ b/src/rx/rx_rdwr.c
@@ -322,14 +322,9 @@ rx_ReadProc(struct rx_call *call, char *buf, int nbytes)
 #endif /* RXDEBUG_PACKET */
             rxi_FreePackets(0, &call->iovq);
     }
-    MUTEX_EXIT(&call->lock);
-    USERPRI;
 
     /*
      * Most common case, all of the data is in the current iovec.
-     * We do not need the lock because this is the only thread that
-     * updates the curlen, curpos, nLeft fields.
-     *
      * We are relying on nLeft being zero unless the call is in receive mode.
      */
     tcurlen = call->curlen;
@@ -343,19 +338,13 @@ rx_ReadProc(struct rx_call *call, char *buf, int nbytes)
 
         if (!call->nLeft && call->currentPacket != NULL) {
             /* out of packet.  Get another one. */
-            NETPRI;
-            MUTEX_ENTER(&call->lock);
             rxi_FreePacket(call->currentPacket);
             call->currentPacket = (struct rx_packet *)0;
-            MUTEX_EXIT(&call->lock);
-            USERPRI;
         }
-       return nbytes;
-    }
+       bytes = nbytes;
+    } else
+        bytes = rxi_ReadProc(call, buf, nbytes);
 
-    NETPRI;
-    MUTEX_ENTER(&call->lock);
-    bytes = rxi_ReadProc(call, buf, nbytes);
     MUTEX_EXIT(&call->lock);
     USERPRI;
     return bytes;
@@ -380,14 +369,9 @@ rx_ReadProc32(struct rx_call *call, afs_int32 * value)
 #endif /* RXDEBUG_PACKET */
             rxi_FreePackets(0, &call->iovq);
     }
-    MUTEX_EXIT(&call->lock);
-    USERPRI;
 
     /*
      * Most common case, all of the data is in the current iovec.
-     * We do not need the lock because this is the only thread that
-     * updates the curlen, curpos, nLeft fields.
-     *
      * We are relying on nLeft being zero unless the call is in receive mode.
      */
     tcurlen = call->curlen;
@@ -401,19 +385,13 @@ rx_ReadProc32(struct rx_call *call, afs_int32 * value)
        call->nLeft = (u_short)(tnLeft - sizeof(afs_int32));
         if (!call->nLeft && call->currentPacket != NULL) {
             /* out of packet.  Get another one. */
-            NETPRI;
-            MUTEX_ENTER(&call->lock);
             rxi_FreePacket(call->currentPacket);
             call->currentPacket = (struct rx_packet *)0;
-            MUTEX_EXIT(&call->lock);
-            USERPRI;
         }
-       return sizeof(afs_int32);
-    }
+       bytes = sizeof(afs_int32);
+    } else
+        bytes = rxi_ReadProc(call, (char *)value, sizeof(afs_int32));
 
-    NETPRI;
-    MUTEX_ENTER(&call->lock);
-    bytes = rxi_ReadProc(call, (char *)value, sizeof(afs_int32));
     MUTEX_EXIT(&call->lock);
     USERPRI;
     return bytes;
@@ -885,14 +863,9 @@ rx_WriteProc(struct rx_call *call, char *buf, int nbytes)
 #endif /* RXDEBUG_PACKET */
             rxi_FreePackets(0, &call->iovq);
     }
-    MUTEX_EXIT(&call->lock);
-    USERPRI;
 
     /*
      * Most common case: all of the data fits in the current iovec.
-     * We do not need the lock because this is the only thread that
-     * updates the curlen, curpos, nFree fields.
-     *
      * We are relying on nFree being zero unless the call is in send mode.
      */
     tcurlen = (int)call->curlen;
@@ -903,12 +876,10 @@ rx_WriteProc(struct rx_call *call, char *buf, int nbytes)
        call->curpos = tcurpos + nbytes;
        call->curlen = (u_short)(tcurlen - nbytes);
        call->nFree = (u_short)(tnFree - nbytes);
-       return nbytes;
-    }
+       bytes = nbytes;
+    } else
+        bytes = rxi_WriteProc(call, buf, nbytes);
 
-    NETPRI;
-    MUTEX_ENTER(&call->lock);
-    bytes = rxi_WriteProc(call, buf, nbytes);
     MUTEX_EXIT(&call->lock);
     USERPRI;
     return bytes;
@@ -933,14 +904,9 @@ rx_WriteProc32(struct rx_call *call, afs_int32 * value)
 #endif /* RXDEBUG_PACKET */
             rxi_FreePackets(0, &call->iovq);
     }
-    MUTEX_EXIT(&call->lock);
-    USERPRI;
 
     /*
      * Most common case: all of the data fits in the current iovec.
-     * We do not need the lock because this is the only thread that
-     * updates the curlen, curpos, nFree fields.
-     *
      * We are relying on nFree being zero unless the call is in send mode.
      */
     tcurlen = call->curlen;
@@ -956,12 +922,10 @@ rx_WriteProc32(struct rx_call *call, afs_int32 * value)
        call->curpos = tcurpos + sizeof(afs_int32);
        call->curlen = (u_short)(tcurlen - sizeof(afs_int32));
        call->nFree = (u_short)(tnFree - sizeof(afs_int32));
-       return sizeof(afs_int32);
-    }
+       bytes = sizeof(afs_int32);
+    } else
+        bytes = rxi_WriteProc(call, (char *)value, sizeof(afs_int32));
 
-    NETPRI;
-    MUTEX_ENTER(&call->lock);
-    bytes = rxi_WriteProc(call, (char *)value, sizeof(afs_int32));
     MUTEX_EXIT(&call->lock);
     USERPRI;
     return bytes;