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 3a86f77ee18db1116dae344846fb05347cdf922f..c7513b56a26ff2f0d912d3f2ed0aa5aeed4cb729 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 b45263ae26b4278d9b1b72b4cbee6f5782276954..5068d2f99d2ec19ef60fedde5b8e4cb2c2ed54c2 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 5f8b4ce9d017ef2c82c3527f4803e10439a455ae..76316e80fb21aa9894cacbaac3fd24b4f8700387 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 9410c7fcd7df62cd21db5e34d851594332599978..98ba61ca2dcc1ea3bebc30c5488d929f9b83c579 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 2064f01927aaeab858a264d669a70a4c733fafa5..122f3237676e3924254b3c1e896a6a676e7017dc 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 0c6154853b725c6b6e2125da756a7d74faf3c110..1423111a6a20575f5d2ca7c8ae832f995788ab96 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);
             }