DEVEL15-windows-interlocked-volume-refcount-20080306

LICENSE MIT

Switch cm_volume_t objects to InterlockedIncrement/InterlockedDecrement
for reference counting.

Remove protections against null pointers being passed into cm_GetVolume()
Instead, do not call cm_GetVolume() if the pointer is NULL.

Fix a buffer data version comparison that should be bad version number
instead of <= 0.

(cherry picked from commit fb154e60e3cb6cf9e934f9a75c5ca67473ac36a2)

diff --git a/src/WINNT/afsd/cm_buf.c b/src/WINNT/afsd/cm_buf.c
index 5d89806..53200ac 100644 (file)
--- a/src/WINNT/afsd/cm_buf.c
+++ b/src/WINNT/afsd/cm_buf.c
@@ -1188,7 +1188,7 @@ long buf_CountFreeList(void)
          * has been invalidate (by having its DV stomped upon), then
          * count it as free, since it isn't really being utilized.
          */
-        if (!(bufp->flags & CM_BUF_INHASH) || bufp->dataVersion <= 0)
+        if (!(bufp->flags & CM_BUF_INHASH) || bufp->dataVersion == CM_BUF_VERSION_BAD)
             count++;
     }       
     lock_ReleaseRead(&buf_globalLock);

diff --git a/src/WINNT/afsd/cm_scache.c b/src/WINNT/afsd/cm_scache.c
index 8f97caf..41271a9 100644 (file)
--- a/src/WINNT/afsd/cm_scache.c
+++ b/src/WINNT/afsd/cm_scache.c
@@ -746,7 +746,8 @@ long cm_GetSCache(cm_fid_t *fidp, cm_scache_t **outScpp, cm_user_t *userp,
 #endif
         scp->fid = *fidp;
         scp->volp = cm_data.rootSCachep->volp;
-       cm_GetVolume(scp->volp);        /* grab an additional reference */
+        if (scp->volp)
+           cm_GetVolume(scp->volp);    /* grab an additional reference */
         scp->dotdotFid.cell=AFS_FAKE_ROOT_CELL_ID;
         scp->dotdotFid.volume=AFS_FAKE_ROOT_VOL_ID;
         scp->dotdotFid.unique=1;

diff --git a/src/WINNT/afsd/cm_volume.c b/src/WINNT/afsd/cm_volume.c
index 3d87d94..f6d09e6 100644 (file)
--- a/src/WINNT/afsd/cm_volume.c
+++ b/src/WINNT/afsd/cm_volume.c
@@ -553,11 +553,7 @@ long cm_UpdateVolume(struct cm_cell *cellp, cm_user_t *userp, cm_req_t *reqp,
 
 void cm_GetVolume(cm_volume_t *volp)
 {
-    if (volp) {
-       lock_ObtainWrite(&cm_volumeLock);
-       volp->refCount++;
-       lock_ReleaseWrite(&cm_volumeLock);
-    }
+    InterlockedIncrement(&volp->refCount);
 }
 
 
@@ -782,12 +778,12 @@ long cm_GetVolumeByName(struct cm_cell *cellp, char *volumeNamep,
     }
     else {
         lock_ReleaseRead(&cm_volumeLock);
-        if (volp) {
-            cm_GetVolume(volp);
-            lock_ObtainMutex(&volp->mx);
-        } else {
+        
+        if (!volp)
             return CM_ERROR_NOSUCHVOLUME;
-        }
+
+        cm_GetVolume(volp);
+        lock_ObtainMutex(&volp->mx);
     }
 
     /* if we get here we are holding the mutex */
@@ -803,15 +799,15 @@ long cm_GetVolumeByName(struct cm_cell *cellp, char *volumeNamep,
         code = CM_ERROR_NOSUCHVOLUME;
 
     if (code == 0) {
-               *outVolpp = volp;
+        *outVolpp = volp;
                
-               if (!(flags & CM_GETVOL_FLAG_NO_LRU_UPDATE)) {
-               lock_ObtainWrite(&cm_volumeLock);
-                       cm_AdjustVolumeLRU(volp);
-                       lock_ReleaseWrite(&cm_volumeLock);
-               }
+        if (!(flags & CM_GETVOL_FLAG_NO_LRU_UPDATE)) {
+            lock_ObtainWrite(&cm_volumeLock);
+            cm_AdjustVolumeLRU(volp);
+            lock_ReleaseWrite(&cm_volumeLock);
+        }
     } else
-               cm_PutVolume(volp);
+        cm_PutVolume(volp);
 
     return code;
 }      
@@ -869,7 +865,6 @@ void cm_ForceUpdateVolume(cm_fid_t *fidp, cm_user_t *userp, cm_req_t *reqp)
 #ifdef SEARCH_ALL_VOLUMES
     osi_assertx(volp == volp2, "unexpected cm_vol_t");
 #endif
-
     lock_ReleaseRead(&cm_volumeLock);
 
     /* hold the volume if we found it */
@@ -927,9 +922,8 @@ cm_serverRef_t **cm_GetVolServers(cm_volume_t *volp, afs_uint32 volume)
 
 void cm_PutVolume(cm_volume_t *volp)
 {
-    lock_ObtainWrite(&cm_volumeLock);
-    osi_assertx(volp->refCount-- > 0, "cm_volume_t refCount 0");
-    lock_ReleaseWrite(&cm_volumeLock);
+    afs_int32 refCount = InterlockedDecrement(&volp->refCount);
+    osi_assertx(refCount >= 0, "cm_volume_t refCount underflow has occurred");
 }
 
 /* return the read-only volume, if there is one, or the read-write volume if

diff --git a/src/WINNT/afsd/cm_volume.h b/src/WINNT/afsd/cm_volume.h
index a125b6c..fd0f158 100644 (file)
--- a/src/WINNT/afsd/cm_volume.h
+++ b/src/WINNT/afsd/cm_volume.h
@@ -36,7 +36,7 @@ typedef struct cm_volume {
     struct cm_fid dotdotFid;           /* parent of volume root */
     osi_mutex_t mx;
     afs_uint32 flags;                  /* by mx */
-    afs_uint32 refCount;               /* by cm_volumeLock */
+    afs_int32 refCount;                        /* by Interlocked operations */
     time_t cbExpiresRO;                 /* latest RO expiration time; by cm_scacheLock */
 } cm_volume_t;
 
@@ -121,4 +121,6 @@ extern void cm_VolumeStatusNotification(cm_volume_t * volp, afs_uint32 volID, en
 extern enum volstatus cm_GetVolumeStatus(cm_volume_t *volp, afs_uint32 volID);
 
 extern void cm_VolumeRenewROCallbacks(void);
+
+extern osi_rwlock_t cm_volumeLock;
 #endif /*  __CM_VOLUME_H_ENV__ */