Rx: Do not hold call lock across memcpy in rx_ReadProc/rx_WriteProc

1.4.x does not hold the call lock across memcpy operations in
rx_ReadProc, rx_ReadProc32, rx_WriteProc, rx_WriteProc32.  The
claim is that the call curpos, curlen, and nLeft fields which
refer to the current packet being processed will not be touched
by any other thread.  Therefore it is safe to drop the call lock
to permit another thread to add packets to the call while the memcpy
is performed in parallel.

This patchset continues to hold the call lock longer than the
original implementation but does drop it for the length of time
it takes to copy data from the packet buffer to the application
buffer.

Change-Id: I87dacdf541ba50db80ab55e500b38edab5126dc2
Reviewed-on: http://gerrit.openafs.org/2817
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementia.org>
Tested-by: Derrick Brashear <shadow@dementia.org>
(cherry picked from commit e197c19cedaafa0d2bb59212fb171083f2140041)
Reviewed-on: http://gerrit.openafs.org/2822

diff --git a/src/rx/rx_rdwr.c b/src/rx/rx_rdwr.c
index dc1b9e7a76f74d9b73b6ec5c07be13e32b6a11be..05097e5f0ee4347f93d3885a9f8a8ea7ef15716a 100644 (file)
--- a/src/rx/rx_rdwr.c
+++ b/src/rx/rx_rdwr.c
@@ -318,6 +318,7 @@ rx_ReadProc(struct rx_call *call, char *buf, int nbytes)
     int tcurlen;
     int tnLeft;
     char *tcurpos;
+    int locked = 1;
     SPLVAR;
 
     /* Free any packets from the last call to ReadvProc/WritevProc */
@@ -338,13 +339,21 @@ rx_ReadProc(struct rx_call *call, char *buf, int nbytes)
     tnLeft = call->nLeft;
     if (!call->error && tcurlen > nbytes && tnLeft > nbytes) {
        tcurpos = call->curpos;
-       memcpy(buf, tcurpos, nbytes);
+        MUTEX_EXIT(&call->lock);
+        USERPRI;
+        locked = 0;
+
+        memcpy(buf, tcurpos, nbytes);
+
        call->curpos = tcurpos + nbytes;
        call->curlen = tcurlen - nbytes;
        call->nLeft = tnLeft - nbytes;
 
         if (!call->nLeft && call->currentPacket != NULL) {
             /* out of packet.  Get another one. */
+            NETPRI;
+            MUTEX_ENTER(&call->lock);
+            locked = 1;
             rxi_FreePacket(call->currentPacket);
             call->currentPacket = (struct rx_packet *)0;
         }
@@ -352,8 +361,10 @@ rx_ReadProc(struct rx_call *call, char *buf, int nbytes)
     } else
         bytes = rxi_ReadProc(call, buf, nbytes);
 
-    MUTEX_EXIT(&call->lock);
-    USERPRI;
+    if (locked) {
+        MUTEX_EXIT(&call->lock);
+        USERPRI;
+    }
     return bytes;
 }
 
@@ -365,6 +376,7 @@ rx_ReadProc32(struct rx_call *call, afs_int32 * value)
     int tcurlen;
     int tnLeft;
     char *tcurpos;
+    int locked = 1;
     SPLVAR;
 
     /* Free any packets from the last call to ReadvProc/WritevProc */
@@ -386,12 +398,20 @@ rx_ReadProc32(struct rx_call *call, afs_int32 * value)
     if (!call->error && tcurlen >= sizeof(afs_int32)
        && tnLeft >= sizeof(afs_int32)) {
        tcurpos = call->curpos;
-       memcpy((char *)value, tcurpos, sizeof(afs_int32));
-       call->curpos = tcurpos + sizeof(afs_int32);
+        MUTEX_EXIT(&call->lock);
+        USERPRI;
+        locked = 0;
+
+        memcpy((char *)value, tcurpos, sizeof(afs_int32));
+
+        call->curpos = tcurpos + sizeof(afs_int32);
        call->curlen = (u_short)(tcurlen - sizeof(afs_int32));
        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);
+            locked = 1;
             rxi_FreePacket(call->currentPacket);
             call->currentPacket = (struct rx_packet *)0;
         }
@@ -399,8 +419,10 @@ rx_ReadProc32(struct rx_call *call, afs_int32 * value)
     } else
         bytes = rxi_ReadProc(call, (char *)value, sizeof(afs_int32));
 
-    MUTEX_EXIT(&call->lock);
-    USERPRI;
+    if (locked) {
+        MUTEX_EXIT(&call->lock);
+        USERPRI;
+    }
     return bytes;
 }
 
@@ -877,6 +899,7 @@ rx_WriteProc(struct rx_call *call, char *buf, int nbytes)
     int tcurlen;
     int tnFree;
     char *tcurpos;
+    int locked = 1;
     SPLVAR;
 
     /* Free any packets from the last call to ReadvProc/WritevProc */
@@ -897,6 +920,11 @@ rx_WriteProc(struct rx_call *call, char *buf, int nbytes)
     tnFree = (int)call->nFree;
     if (!call->error && tcurlen >= nbytes && tnFree >= nbytes) {
        tcurpos = call->curpos;
+
+        MUTEX_EXIT(&call->lock);
+        USERPRI;
+        locked = 0;
+
        memcpy(tcurpos, buf, nbytes);
        call->curpos = tcurpos + nbytes;
        call->curlen = (u_short)(tcurlen - nbytes);
@@ -905,8 +933,10 @@ rx_WriteProc(struct rx_call *call, char *buf, int nbytes)
     } else
         bytes = rxi_WriteProc(call, buf, nbytes);
 
-    MUTEX_EXIT(&call->lock);
-    USERPRI;
+    if (locked) {
+        MUTEX_EXIT(&call->lock);
+        USERPRI;
+    }
     return bytes;
 }
 
@@ -918,6 +948,7 @@ rx_WriteProc32(struct rx_call *call, afs_int32 * value)
     int tcurlen;
     int tnFree;
     char *tcurpos;
+    int locked = 1;
     SPLVAR;
 
     /* Free any packets from the last call to ReadvProc/WritevProc */
@@ -939,6 +970,11 @@ rx_WriteProc32(struct rx_call *call, afs_int32 * value)
     if (!call->error && tcurlen >= sizeof(afs_int32)
        && tnFree >= sizeof(afs_int32)) {
        tcurpos = call->curpos;
+
+        MUTEX_EXIT(&call->lock);
+        USERPRI;
+        locked = 0;
+
        if (!((size_t)tcurpos & (sizeof(afs_int32) - 1))) {
            *((afs_int32 *) (tcurpos)) = *value;
        } else {
@@ -951,8 +987,10 @@ rx_WriteProc32(struct rx_call *call, afs_int32 * value)
     } else
         bytes = rxi_WriteProc(call, (char *)value, sizeof(afs_int32));
 
-    MUTEX_EXIT(&call->lock);
-    USERPRI;
+    if (locked) {
+        MUTEX_EXIT(&call->lock);
+        USERPRI;
+    }
     return bytes;
 }