Windows: create scache->redirMx to reduce contention

Relying on the cm_scache_t.rw lock to protect the cm_scache_t.redirQueue*
results in a large amount of contention between processing extent
requests and releases from the afs redirector and the threads attempting
to read from or write data to the file server.   There is no reason why
the same lock must be used.  Allocate a dedicated mutex to protect the
queue.

By placing the new mutex after the buf_globalLock in the locking
hierarchy it permits the lock acquisition logic for extent processing
to be simplified further reducing cm_scache_t.rw lock transitions.

Change-Id: Id2ded86c1f3757a2f1071c8cf39f2fbc6bcfcfaa
Reviewed-on: http://gerrit.openafs.org/6053
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@secure-endpoints.com>
Tested-by: Jeffrey Altman <jaltman@secure-endpoints.com>

diff --git a/src/WINNT/afsd/cm.h b/src/WINNT/afsd/cm.h
index 3a86f77..c7513b5 100644 (file)
--- a/src/WINNT/afsd/cm.h
+++ b/src/WINNT/afsd/cm.h
@@ -67,6 +67,7 @@
 #define LOCK_HIERARCHY_BUFFER                  540
 #define LOCK_HIERARCHY_SCACHE                  550
 #define LOCK_HIERARCHY_BUF_GLOBAL              560
+#define LOCK_HIERARCHY_SCACHE_REDIRMX          565
 #define LOCK_HIERARCHY_VOLUME                  570
 #define LOCK_HIERARCHY_USER                    580
 #define LOCK_HIERARCHY_SCACHE_GLOBAL           590

diff --git a/src/WINNT/afsd/cm_buf.c b/src/WINNT/afsd/cm_buf.c
index b45263a..5068d2f 100644 (file)
--- a/src/WINNT/afsd/cm_buf.c
+++ b/src/WINNT/afsd/cm_buf.c
@@ -2571,8 +2571,8 @@ void
 buf_InsertToRedirQueue(cm_scache_t *scp, cm_buf_t *bufp)
 {
     lock_AssertWrite(&buf_globalLock);
-    if (scp)
-        lock_AssertWrite(&scp->rw);
+
+    lock_ObtainMutex(&scp->redirMx);
 
     if (bufp->qFlags & CM_BUF_QINLRU) {
         _InterlockedAnd(&bufp->qFlags, ~CM_BUF_QINLRU);
@@ -2594,14 +2594,16 @@ buf_InsertToRedirQueue(cm_scache_t *scp, cm_buf_t *bufp)
         scp->redirLastAccess = bufp->redirLastAccess;
         InterlockedIncrement(&scp->redirBufCount);
     }
+
+    lock_ReleaseMutex(&scp->redirMx);
 }
 
 void
 buf_RemoveFromRedirQueue(cm_scache_t *scp, cm_buf_t *bufp)
 {
     lock_AssertWrite(&buf_globalLock);
-    if (scp)
-        lock_AssertWrite(&scp->rw);
+
+    lock_ObtainMutex(&scp->redirMx);
 
     _InterlockedAnd(&bufp->qFlags, ~CM_BUF_QREDIR);
     osi_QRemoveHT( (osi_queue_t **) &cm_data.buf_redirListp,
@@ -2614,6 +2616,8 @@ buf_RemoveFromRedirQueue(cm_scache_t *scp, cm_buf_t *bufp)
                        &bufp->redirq);
         InterlockedDecrement(&scp->redirBufCount);
     }
+
+    lock_ReleaseMutex(&scp->redirMx);
 }
 
 void
@@ -2623,8 +2627,7 @@ buf_MoveToHeadOfRedirQueue(cm_scache_t *scp, cm_buf_t *bufp)
     osi_assertx(bufp->qFlags & CM_BUF_QREDIR,
                  "buf_MoveToHeadOfRedirQueue buffer not held by redirector");
 
-    if (scp)
-        lock_AssertWrite(&scp->rw);
+    lock_ObtainMutex(&scp->redirMx);
 
     osi_QRemoveHT( (osi_queue_t **) &cm_data.buf_redirListp,
                    (osi_queue_t **) &cm_data.buf_redirListEndp,
@@ -2642,4 +2645,6 @@ buf_MoveToHeadOfRedirQueue(cm_scache_t *scp, cm_buf_t *bufp)
                    &bufp->redirq);
         scp->redirLastAccess = bufp->redirLastAccess;
     }
+
+    lock_ReleaseMutex(&scp->redirMx);
 }

diff --git a/src/WINNT/afsd/cm_memmap.h b/src/WINNT/afsd/cm_memmap.h
index 5f8b4ce..76316e8 100644 (file)
--- a/src/WINNT/afsd/cm_memmap.h
+++ b/src/WINNT/afsd/cm_memmap.h
@@ -10,7 +10,7 @@
 #ifndef CM_MEMMAP_H
 #define CM_MEMMAP_H 1
 
-#define CM_CONFIG_DATA_VERSION  16
+#define CM_CONFIG_DATA_VERSION  17
 #define CM_CONFIG_DATA_MAGIC            ('A' | 'F'<<8 | 'S'<<16 | CM_CONFIG_DATA_VERSION<<24)
 
 typedef struct cm_config_data {

diff --git a/src/WINNT/afsd/cm_scache.c b/src/WINNT/afsd/cm_scache.c
index 9410c7f..98ba61c 100644 (file)
--- a/src/WINNT/afsd/cm_scache.c
+++ b/src/WINNT/afsd/cm_scache.c
@@ -329,6 +329,7 @@ cm_GetNewSCache(afs_uint32 locked)
 #ifdef USE_BPLUS
     lock_InitializeRWLock(&scp->dirlock, "cm_scache_t dirlock", LOCK_HIERARCHY_SCACHE_DIRLOCK);
 #endif
+    lock_InitializeMutex(&scp->redirMx, "cm_scache_t redirMx", LOCK_HIERARCHY_SCACHE_REDIRMX);
     scp->serverLock = -1;
 
     /* and put it in the LRU queue */
@@ -390,6 +391,7 @@ void cm_fakeSCacheInit(int newFile)
     lock_InitializeRWLock(&cm_data.fakeSCache.rw, "cm_scache_t rw", LOCK_HIERARCHY_SCACHE);
     lock_InitializeRWLock(&cm_data.fakeSCache.bufCreateLock, "cm_scache_t bufCreateLock", LOCK_HIERARCHY_SCACHE_BUFCREATE);
     lock_InitializeRWLock(&cm_data.fakeSCache.dirlock, "cm_scache_t dirlock", LOCK_HIERARCHY_SCACHE_DIRLOCK);
+    lock_InitializeMutex(&cm_data.fakeSCache.redirMx, "cm_scache_t redirMx", LOCK_HIERARCHY_SCACHE_REDIRMX);
 }
 
 long
@@ -572,6 +574,7 @@ cm_ShutdownSCache(void)
 #endif
         lock_FinalizeRWLock(&scp->rw);
         lock_FinalizeRWLock(&scp->bufCreateLock);
+        lock_FinalizeMutex(&scp->redirMx);
     }
     lock_ReleaseWrite(&cm_scacheLock);
 
@@ -625,6 +628,7 @@ void cm_InitSCache(int newFile, long maxSCaches)
                 scp->redirBufCount = 0;
                 scp->redirQueueT = NULL;
                 scp->redirQueueH = NULL;
+                lock_InitializeMutex(&scp->redirMx, "cm_scache_t redirMx", LOCK_HIERARCHY_SCACHE_REDIRMX);
             }
         }
         cm_allFileLocks = NULL;

diff --git a/src/WINNT/afsd/cm_scache.h b/src/WINNT/afsd/cm_scache.h
index 2064f01..122f323 100644 (file)
--- a/src/WINNT/afsd/cm_scache.h
+++ b/src/WINNT/afsd/cm_scache.h
@@ -228,13 +228,14 @@ typedef struct cm_scache {
                                        cm_scacheLock. */
     osi_queue_t * waitQueueT;       /* locked by cm_scacheLock */
 
-    /* redirector state - protected by scp->rw */
+    /* redirector state - protected by scp->redirMx */
     osi_queue_t * redirQueueH;      /* LRU queue of buffers for this
                                        file that are assigned to the
                                        afsredir kernel module. */
     osi_queue_t * redirQueueT;
     afs_uint32    redirBufCount;    /* Number of buffers held by the redirector */
     time_t        redirLastAccess;  /* last time redir accessed the vnode */
+    osi_mutex_t   redirMx;
 
     afs_uint32 activeRPCs;              /* atomic */
 } cm_scache_t;

diff --git a/src/WINNT/afsrdr/user/RDRFunction.c b/src/WINNT/afsrdr/user/RDRFunction.c
index 0c61548..1423111 100644 (file)
--- a/src/WINNT/afsrdr/user/RDRFunction.c
+++ b/src/WINNT/afsrdr/user/RDRFunction.c
@@ -2959,13 +2959,8 @@ RDR_BkgFetch(cm_scache_t *scp, afs_uint32 p1, afs_uint32 p2, afs_uint32 p3, afs_
 
         code = cm_GetBuffer(scp, bufp, NULL, userp, reqp);
         if (code == 0) {
-            if (bufp->flags & CM_BUF_DIRTY) {
-                if (rwheld) {
-                    lock_ReleaseWrite(&scp->rw);
-                    rwheld = 0;
-                }
-                cm_BufWrite(scp, &bufp->offset, cm_chunkSize, 0, userp, reqp);
-            }
+            if (bufp->flags & CM_BUF_DIRTY)
+                cm_BufWrite(scp, &bufp->offset, cm_chunkSize, CM_BUF_WRITE_SCP_LOCKED, userp, reqp);
 
             if (!(bufp->qFlags & CM_BUF_QREDIR)) {
 #ifdef VALIDATE_CHECK_SUM
@@ -2974,18 +2969,12 @@ RDR_BkgFetch(cm_scache_t *scp, afs_uint32 p1, afs_uint32 p2, afs_uint32 p3, afs_
                 char dbgstr[1024];
 #endif
 #endif
-                if (!rwheld) {
-                    lock_ObtainWrite(&scp->rw);
-                    rwheld = 1;
-                }
                 lock_ObtainWrite(&buf_globalLock);
                 if (!(bufp->flags & CM_BUF_DIRTY) &&
                     bufp->cmFlags == 0 &&
                     !(bufp->qFlags & CM_BUF_QREDIR)) {
                     buf_InsertToRedirQueue(scp, bufp);
                     lock_ReleaseWrite(&buf_globalLock);
-                    lock_ReleaseWrite(&scp->rw);
-                    rwheld = 0;
 
 #ifdef VALIDATE_CHECK_SUM
                     buf_ComputeCheckSum(bufp);
@@ -3030,12 +3019,6 @@ RDR_BkgFetch(cm_scache_t *scp, afs_uint32 p1, afs_uint32 p2, afs_uint32 p3, afs_
                 osi_Log4(afsd_logp, "RDR_BkgFetch Extent2FS Already held by Redirector bufp 0x%p foffset 0x%p coffset 0x%p len 0x%x",
                           bufp, bufp->offset.QuadPart, bufp->datap - RDR_extentBaseAddress, cm_data.blockSize);
             }
-
-            if (rwheld) {
-                lock_ReleaseWrite(&scp->rw);
-                rwheld = 0;
-            }
-
         } else {
             /*
              * depending on what the error from cm_GetBuffer is
@@ -3268,12 +3251,10 @@ RDR_RequestFileExtentsAsync( IN cm_user_t *userp,
                     char dbgstr[1024];
 #endif
 #endif
-                    lock_ObtainWrite(&scp->rw);
                     lock_ObtainWrite(&buf_globalLock);
                     if (!(bufp->qFlags & CM_BUF_QREDIR)) {
                         buf_InsertToRedirQueue(scp, bufp);
                         lock_ReleaseWrite(&buf_globalLock);
-                        lock_ReleaseWrite(&scp->rw);
 
 #ifdef VALIDATE_CHECK_SUM
                         buf_ComputeCheckSum(bufp);
@@ -3314,11 +3295,9 @@ RDR_RequestFileExtentsAsync( IN cm_user_t *userp,
                          * However, we know the buffer is in recent use so move the buffer to the
                          * front of the queue
                          */
-                        lock_ObtainWrite(&scp->rw);
                         lock_ObtainWrite(&buf_globalLock);
                         buf_MoveToHeadOfRedirQueue(scp, bufp);
                         lock_ReleaseWrite(&buf_globalLock);
-                        lock_ReleaseWrite(&scp->rw);
 
                         osi_Log4(afsd_logp, "RDR_RequestFileExtentsAsync Extent2FS Already held by Redirector bufp 0x%p foffset 0x%p coffset 0x%p len 0x%x",
                                  bufp, ByteOffset.QuadPart, bufp->datap - RDR_extentBaseAddress, cm_data.blockSize);
@@ -3486,13 +3465,9 @@ RDR_ReleaseFileExtents( IN cm_user_t *userp,
                           pExtent->CacheOffset.LowPart);
 
                 /* Move the buffer to the front of the queue */
-                if (scp)
-                    lock_ObtainWrite(&scp->rw);
                 lock_ObtainWrite(&buf_globalLock);
                 buf_MoveToHeadOfRedirQueue(scp, bufp);
                 lock_ReleaseWrite(&buf_globalLock);
-                if (scp)
-                    lock_ReleaseWrite(&scp->rw);
                 buf_Release(bufp);
                 continue;
             }
@@ -3519,19 +3494,12 @@ RDR_ReleaseFileExtents( IN cm_user_t *userp,
                              (pExtent->Flags & AFS_EXTENT_FLAG_RELEASE) )
                         {
                             if (bufp->qFlags & CM_BUF_QREDIR) {
-                                if (scp)
-                                    lock_ObtainWrite(&scp->rw);
                                 lock_ObtainWrite(&buf_globalLock);
                                 if (bufp->qFlags & CM_BUF_QREDIR) {
                                     buf_RemoveFromRedirQueue(scp, bufp);
-                                    lock_ReleaseWrite(&scp->rw);
                                     buf_ReleaseLocked(bufp, TRUE);
-                                } else {
-                                    if (scp)
-                                        lock_ReleaseWrite(&scp->rw);
                                 }
-                                if (scp)
-                                    lock_ReleaseWrite(&buf_globalLock);
+                                lock_ReleaseWrite(&buf_globalLock);
                             }
 #ifdef ODS_DEBUG
                             snprintf( dbgstr, 1024,
@@ -3986,13 +3954,9 @@ RDR_ProcessReleaseFileExtentsResult( IN AFSReleaseFileExtentsResultCB *ReleaseFi
                               pExtent->CacheOffset.LowPart);
 
                     /* Move the buffer to the front of the queue */
-                    if (scp)
-                        lock_ObtainWrite(&scp->rw);
                     lock_ObtainWrite(&buf_globalLock);
                     buf_MoveToHeadOfRedirQueue(scp, bufp);
                     lock_ReleaseWrite(&buf_globalLock);
-                    if (scp)
-                        lock_ReleaseWrite(&scp->rw);
                     buf_Release(bufp);
                     continue;
                 }
@@ -4028,17 +3992,10 @@ RDR_ProcessReleaseFileExtentsResult( IN AFSReleaseFileExtentsResultCB *ReleaseFi
                                  (pExtent->Flags & AFS_EXTENT_FLAG_RELEASE) )
                             {
                                 if (bufp->qFlags & CM_BUF_QREDIR) {
-                                    if (scp)
-                                        lock_ObtainWrite(&scp->rw);
                                     lock_ObtainWrite(&buf_globalLock);
                                     if (bufp->qFlags & CM_BUF_QREDIR) {
                                         buf_RemoveFromRedirQueue(scp, bufp);
-                                        if (scp)
-                                            lock_ReleaseWrite(&scp->rw);
                                         buf_ReleaseLocked(bufp, TRUE);
-                                    } else {
-                                        if (scp)
-                                            lock_ReleaseWrite(&scp->rw);
                                     }
                                     lock_ReleaseWrite(&buf_globalLock);
                                 }
@@ -4384,17 +4341,10 @@ RDR_ReleaseFailedSetFileExtents( IN cm_user_t *userp,
 
             lock_ObtainMutex(&bufp->mx);
             if (bufp->qFlags & CM_BUF_QREDIR) {
-                if (scp)
-                    lock_ObtainWrite(&scp->rw);
                 lock_ObtainWrite(&buf_globalLock);
                 if (bufp->qFlags & CM_BUF_QREDIR) {
                     buf_RemoveFromRedirQueue(scp, bufp);
-                    if (scp)
-                        lock_ReleaseWrite(&scp->rw);
                     buf_ReleaseLocked(bufp, TRUE);
-                } else {
-                    if (scp)
-                        lock_ReleaseWrite(&scp->rw);
                 }
                 lock_ReleaseWrite(&buf_globalLock);
             }