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().

diff --git a/src/WINNT/afsd/cm_scache.c b/src/WINNT/afsd/cm_scache.c
index 610bd08cc9f5fe7fc608107787fa822d9749377c..1f023af2e1b617ef189ba855bef88faf4a798a2d 100644 (file)
--- a/src/WINNT/afsd/cm_scache.c
+++ b/src/WINNT/afsd/cm_scache.c
@@ -45,21 +45,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);
@@ -94,7 +93,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) {
@@ -233,6 +234,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;
@@ -1802,8 +1804,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)
@@ -1814,6 +1820,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
@@ -1836,6 +1857,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 8e704fc7c88790207ff8fbf7ff3d60978bd6a25b..ac4990b284523d6727ae2f1e45c050017ab35f3f 100644 (file)
--- a/src/WINNT/afsd/cm_scache.h
+++ b/src/WINNT/afsd/cm_scache.h
@@ -405,4 +405,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 2a5b211d6a59343efb161a06d6d03a4523a33aff..8d533615447f4d52449768ea4ecea0c1239af690 100644 (file)
--- a/src/WINNT/afsd/cm_vnodeops.c
+++ b/src/WINNT/afsd/cm_vnodeops.c
@@ -1610,6 +1610,7 @@ long cm_Unlink(cm_scache_t *dscp, char *namep, char * normalizedName,
     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) {
@@ -1619,6 +1620,8 @@ long cm_Unlink(cm_scache_t *dscp, char *namep, char * normalizedName,
     }
 #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);
 
@@ -1681,6 +1684,15 @@ long cm_Unlink(cm_scache_t *dscp, char *namep, char * normalizedName,
     }
     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;
 }
 
@@ -3163,6 +3175,9 @@ long cm_RemoveDir(cm_scache_t *dscp, char *namep, char *normalizedNamep,
     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
@@ -3225,6 +3240,15 @@ long cm_RemoveDir(cm_scache_t *dscp, char *namep, char *normalizedNamep,
     }
     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 bee69de43bddc47f83e7a34643aa903c7d38aa9b..9ef5047cdb04b2d88cc7a36115afc261aac73ba1 100644 (file)
--- a/src/WINNT/afsd/smb.c
+++ b/src/WINNT/afsd/smb.c
@@ -6141,7 +6141,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)",
@@ -6261,7 +6260,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, originalNamep, 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,
@@ -6270,7 +6268,6 @@ long smb_CloseFID(smb_vc_t *vcp, smb_fid_t *fidp, cm_user_t *userp,
         } else {
             code = cm_Unlink(dscp, originalNamep, 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,
@@ -6320,12 +6317,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);
     }