From 81cfe9462884788b2481d8f44dc86e8953f1ffc3 Mon Sep 17 00:00:00 2001 From: Jeffrey Altman Date: Thu, 12 Jun 2008 15:22:22 +0000 Subject: [PATCH] DEVEL15-windows-scache-deletion-and-lock-verification-20080612 LICENSE MIT Add lock assertions to various functions. Obtain a missing lock around a call to cm_RemoveSCacheFromHashTable(). Correct an abstraction layer violation. cm_scache_t objects should be marked deleted in cm_Unlink() and cm_RemoveDir() and not in smb_CloseFID(). Cleanup of deleted cm_scache_t objects should be performed in cm_ReleaseSCache() when the reference count hits zero. Prototype cm_AdjustScacheLRU() and re-implement it using osi_QAddH(). (cherry picked from commit 52490d7968c2008912ab0887bdcde7fbba43b837) --- src/WINNT/afsd/cm_scache.c | 47 +++++++++++++++++++++++++++++++----- src/WINNT/afsd/cm_scache.h | 2 ++ src/WINNT/afsd/cm_vnodeops.c | 24 ++++++++++++++++++ src/WINNT/afsd/smb.c | 9 ------- 4 files changed, 67 insertions(+), 15 deletions(-) diff --git a/src/WINNT/afsd/cm_scache.c b/src/WINNT/afsd/cm_scache.c index e6d6d9672..357f74492 100644 --- a/src/WINNT/afsd/cm_scache.c +++ b/src/WINNT/afsd/cm_scache.c @@ -47,21 +47,20 @@ extern osi_mutex_t cm_Freelance_Lock; /* must be called with cm_scacheLock write-locked! */ void cm_AdjustScacheLRU(cm_scache_t *scp) { - if (scp == cm_data.scacheLRULastp) - cm_data.scacheLRULastp = (cm_scache_t *) osi_QPrev(&scp->q); + lock_AssertWrite(&cm_scacheLock); osi_QRemoveHT((osi_queue_t **) &cm_data.scacheLRUFirstp, (osi_queue_t **) &cm_data.scacheLRULastp, &scp->q); - osi_QAdd((osi_queue_t **) &cm_data.scacheLRUFirstp, &scp->q); - if (!cm_data.scacheLRULastp) - cm_data.scacheLRULastp = scp; + osi_QAddH((osi_queue_t **) &cm_data.scacheLRUFirstp, (osi_queue_t **) &cm_data.scacheLRULastp, &scp->q); } -/* call with scache write-locked and mutex held */ +/* call with cm_scacheLock write-locked and scp rw held */ void cm_RemoveSCacheFromHashTable(cm_scache_t *scp) { cm_scache_t **lscpp; cm_scache_t *tscp; int i; + lock_AssertWrite(&cm_scacheLock); + lock_AssertWrite(&scp->rw); if (scp->flags & CM_SCACHEFLAG_INHASH) { /* hash it out first */ i = CM_SCACHE_HASH(&scp->fid); @@ -96,7 +95,9 @@ long cm_RecycleSCache(cm_scache_t *scp, afs_int32 flags) return -1; } + lock_ObtainWrite(&scp->rw); cm_RemoveSCacheFromHashTable(scp); + lock_ReleaseWrite(&scp->rw); #if 0 if (flags & CM_SCACHE_RECYCLEFLAG_DESTROY_BUFFERS) { @@ -235,6 +236,7 @@ cm_scache_t *cm_GetNewSCache(void) cm_scache_t *scp; int retry = 0; + lock_AssertWrite(&cm_scacheLock); #if 0 /* first pass - look for deleted objects */ for ( scp = cm_data.scacheLRULastp; @@ -1804,8 +1806,12 @@ void cm_ReleaseSCacheNoLock(cm_scache_t *scp) #endif { afs_int32 refCount; + long lockstate; + osi_assertx(scp != NULL, "null cm_scache_t"); lock_AssertAny(&cm_scacheLock); + + lockstate = lock_GetRWLockState(&cm_scacheLock); refCount = InterlockedDecrement(&scp->refCount); #ifdef DEBUG_REFCOUNT if (refCount < 0) @@ -1816,6 +1822,21 @@ void cm_ReleaseSCacheNoLock(cm_scache_t *scp) osi_Log2(afsd_logp,"cm_ReleaseSCacheNoLock scp 0x%p ref %d",scp, refCount); afsi_log("%s:%d cm_ReleaseSCacheNoLock scp 0x%p ref %d", file, line, scp, refCount); #endif + + if (scp->flags & CM_SCACHEFLAG_DELETED) { + int deleted = 0; + lock_ObtainWrite(&scp->rw); + if (scp->flags & CM_SCACHEFLAG_DELETED) + deleted = 1; + lock_ReleaseWrite(&scp->rw); + if (deleted) { + if (lockstate != OSI_RWLOCK_WRITEHELD) + lock_ConvertRToW(&cm_scacheLock); + cm_RecycleSCache(scp, 0); + if (lockstate != OSI_RWLOCK_WRITEHELD) + lock_ConvertWToR(&cm_scacheLock); + } + } } #ifdef DEBUG_REFCOUNT @@ -1838,6 +1859,20 @@ void cm_ReleaseSCache(cm_scache_t *scp) osi_Log2(afsd_logp,"cm_ReleaseSCache scp 0x%p ref %d",scp, refCount); afsi_log("%s:%d cm_ReleaseSCache scp 0x%p ref %d", file, line, scp, refCount); #endif + + if (scp->flags & CM_SCACHEFLAG_DELETED) { + int deleted = 0; + lock_ObtainWrite(&scp->rw); + if (scp->flags & CM_SCACHEFLAG_DELETED) + deleted = 1; + lock_ReleaseWrite(&scp->rw); + if (deleted) { + lock_ConvertRToW(&cm_scacheLock); + cm_RecycleSCache(scp, 0); + lock_ConvertWToR(&cm_scacheLock); + } + } + lock_ReleaseRead(&cm_scacheLock); } diff --git a/src/WINNT/afsd/cm_scache.h b/src/WINNT/afsd/cm_scache.h index b56177cc3..cdf4419a5 100644 --- a/src/WINNT/afsd/cm_scache.h +++ b/src/WINNT/afsd/cm_scache.h @@ -409,4 +409,6 @@ extern void cm_SuspendSCache(void); extern long cm_RecycleSCache(cm_scache_t *scp, afs_int32 flags); extern void cm_RemoveSCacheFromHashTable(cm_scache_t *scp); + +extern void cm_AdjustScacheLRU(cm_scache_t *scp); #endif /* __CM_SCACHE_H_ENV__ */ diff --git a/src/WINNT/afsd/cm_vnodeops.c b/src/WINNT/afsd/cm_vnodeops.c index a0ae47025..8346bafb1 100644 --- a/src/WINNT/afsd/cm_vnodeops.c +++ b/src/WINNT/afsd/cm_vnodeops.c @@ -1590,6 +1590,7 @@ long cm_Unlink(cm_scache_t *dscp, char *namep, cm_user_t *userp, cm_req_t *reqp) AFSVolSync volSync; struct rx_connection * callp; cm_dirOp_t dirop; + cm_scache_t *scp = NULL; #ifdef AFS_FREELANCE_CLIENT if (cm_freelanceEnabled && dscp == cm_data.rootSCachep) { @@ -1599,6 +1600,8 @@ long cm_Unlink(cm_scache_t *dscp, char *namep, cm_user_t *userp, cm_req_t *reqp) } #endif + code = cm_Lookup(dscp, namep, CM_FLAG_NOMOUNTCHASE, userp, reqp, &scp); + /* make sure we don't screw up the dir status during the merge */ code = cm_BeginDirOp(dscp, userp, reqp, CM_DIRLOCK_NONE, &dirop); @@ -1661,6 +1664,15 @@ long cm_Unlink(cm_scache_t *dscp, char *namep, cm_user_t *userp, cm_req_t *reqp) } cm_EndDirOp(&dirop); + if (scp) { + cm_ReleaseSCache(scp); + if (code == 0) { + lock_ObtainWrite(&scp->rw); + scp->flags |= CM_SCACHEFLAG_DELETED; + lock_ReleaseWrite(&scp->rw); + } + } + return code; } @@ -3125,6 +3137,9 @@ long cm_RemoveDir(cm_scache_t *dscp, char *namep, cm_user_t *userp, AFSVolSync volSync; struct rx_connection * callp; cm_dirOp_t dirop; + cm_scache_t *scp = NULL; + + code = cm_Lookup(dscp, namep, CM_FLAG_NOMOUNTCHASE, userp, reqp, &scp); /* before starting the RPC, mark that we're changing the directory data, * so that someone who does a chmod on the dir will wait until our @@ -3187,6 +3202,15 @@ long cm_RemoveDir(cm_scache_t *dscp, char *namep, cm_user_t *userp, } cm_EndDirOp(&dirop); + if (scp) { + cm_ReleaseSCache(scp); + if (code == 0) { + lock_ObtainWrite(&scp->rw); + scp->flags |= CM_SCACHEFLAG_DELETED; + lock_ReleaseWrite(&scp->rw); + } + } + /* and return error code */ return code; } diff --git a/src/WINNT/afsd/smb.c b/src/WINNT/afsd/smb.c index 7d8d904b6..30e69e02b 100644 --- a/src/WINNT/afsd/smb.c +++ b/src/WINNT/afsd/smb.c @@ -6393,7 +6393,6 @@ long smb_CloseFID(smb_vc_t *vcp, smb_fid_t *fidp, cm_user_t *userp, char *pathp = NULL; cm_scache_t * scp = NULL; cm_scache_t *delscp = NULL; - int deleted = 0; int nullcreator = 0; osi_Log4(smb_logp, "smb_CloseFID Closing fidp 0x%x (fid=%d scp=0x%x vcp=0x%x)", @@ -6512,7 +6511,6 @@ long smb_CloseFID(smb_vc_t *vcp, smb_fid_t *fidp, cm_user_t *userp, if (delscp->fileType == CM_SCACHETYPE_DIRECTORY) { code = cm_RemoveDir(dscp, fullPathp, userp, &req); if (code == 0) { - deleted = 1; if (dscp->flags & CM_SCACHEFLAG_ANYWATCH) smb_NotifyChange(FILE_ACTION_REMOVED, FILE_NOTIFY_CHANGE_DIR_NAME | FILE_NOTIFY_CHANGE_CREATION, @@ -6521,7 +6519,6 @@ long smb_CloseFID(smb_vc_t *vcp, smb_fid_t *fidp, cm_user_t *userp, } else { code = cm_Unlink(dscp, fullPathp, userp, &req); if (code == 0) { - deleted = 1; if (dscp->flags & CM_SCACHEFLAG_ANYWATCH) smb_NotifyChange(FILE_ACTION_REMOVED, FILE_NOTIFY_CHANGE_FILE_NAME | FILE_NOTIFY_CHANGE_CREATION, @@ -6566,12 +6563,6 @@ long smb_CloseFID(smb_vc_t *vcp, smb_fid_t *fidp, cm_user_t *userp, cm_ReleaseSCache(dscp); if (delscp) { - if (deleted) { - lock_ObtainWrite(&delscp->rw); - if (deleted) - delscp->flags |= CM_SCACHEFLAG_DELETED; - lock_ReleaseWrite(&delscp->rw); - } cm_ReleaseSCache(delscp); } -- 2.39.5