From 55f5f356af2ef884413bd656f100055741ae871b Mon Sep 17 00:00:00 2001 From: Jeffrey Altman Date: Tue, 15 Nov 2011 18:23:46 -0500 Subject: [PATCH] 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 Reviewed-by: Jeffrey Altman Tested-by: Jeffrey Altman --- src/WINNT/afsd/cm.h | 1 + src/WINNT/afsd/cm_buf.c | 17 +++++++---- src/WINNT/afsd/cm_memmap.h | 2 +- src/WINNT/afsd/cm_scache.c | 4 +++ src/WINNT/afsd/cm_scache.h | 3 +- src/WINNT/afsrdr/user/RDRFunction.c | 56 ++----------------------------------- 6 files changed, 22 insertions(+), 61 deletions(-) diff --git a/src/WINNT/afsd/cm.h b/src/WINNT/afsd/cm.h index 3a86f77..c7513b5 100644 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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); } -- 1.9.4