From: Jeffrey Altman <jaltman@your-file-system.com>
Date: Mon, 20 Sep 2010 12:11:20 +0000 (-0700)
Subject: Rx: Do not hold call lock across memcpy in rx_ReadProc/rx_WriteProc
X-Git-Tag: openafs-devel-1_7_1~1532
X-Git-Url: https://git.openafs.org/?p=openafs.git;a=commitdiff_plain;h=e197c19cedaafa0d2bb59212fb171083f2140041

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>
---

diff --git a/src/rx/rx_rdwr.c b/src/rx/rx_rdwr.c
index dc1b9e7..05097e5 100644
--- 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;
 }