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)

diff --git a/src/WINNT/afsd/cm_scache.c b/src/WINNT/afsd/cm_scache.c
index e6d6d9672bf066f08ba7c97ace3eddfa63ab592d..357f74492284ac4ac44f37876f97ef3ceb6f57a2 100644 (file)
--- 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 b56177cc339e20f13bdb09118486bc784824ceca..cdf4419a541140d4fadc5cdc105d991f76d8816e 100644 (file)
--- 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 a0ae4702532595590e99e53663a5bc9baf92d5d1..8346bafb11a8f199391370dc6c286b2fd1db0a61 100644 (file)
--- 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 7d8d904b68eaf8326b86617f630f2bc997988542..30e69e02b66c39008308c5232e25c9f0441b8477 100644 (file)
--- 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);
     }