From: Jeffrey Altman <jaltman@secure-endpoints.com>
Date: Fri, 7 Mar 2008 01:03:45 +0000 (+0000)
Subject: windows-interlocked-volume-refcount-20080306
X-Git-Tag: BP-openafs-windows-kdfs-ifs~48
X-Git-Url: https://git.openafs.org/?p=openafs.git;a=commitdiff_plain;h=fb154e60e3cb6cf9e934f9a75c5ca67473ac36a2

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

diff --git a/src/WINNT/afsd/cm_buf.c b/src/WINNT/afsd/cm_buf.c
index 4df4884..3f8ddc7 100644
--- a/src/WINNT/afsd/cm_buf.c
+++ b/src/WINNT/afsd/cm_buf.c
@@ -1138,7 +1138,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 34be33f..e688ea2 100644
--- a/src/WINNT/afsd/cm_scache.c
+++ b/src/WINNT/afsd/cm_scache.c
@@ -744,7 +744,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 b24b894..4e90c40 100644
--- a/src/WINNT/afsd/cm_volume.c
+++ b/src/WINNT/afsd/cm_volume.c
@@ -549,11 +549,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);
 }
 
 
@@ -778,12 +774,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 */
@@ -799,15 +795,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;
 }	
@@ -865,7 +861,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 */
@@ -923,9 +918,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
--- 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__ */