Windows: out of order locks cm_CheckCBExpiration
authorJeffrey Altman <jaltman@your-file-system.com>
Sun, 30 Jan 2011 04:24:16 +0000 (23:24 -0500)
committerJeffrey Altman <jaltman@openafs.org>
Mon, 31 Jan 2011 14:54:06 +0000 (06:54 -0800)
The recent refactoring of cm_CheckCBExpiration introduced
a lock ordering error between the cm_scache_t rw lock and the
cm_scacheLock.  This patchset fixes the error by dropping the
cm_scacheLock as each cm_scache_t is being processed.

Change-Id: Ib9e45abc5a43ca550d4a2a7923e3b30017e9fbf9
Reviewed-on: http://gerrit.openafs.org/3773
Tested-by: Jeffrey Altman <jaltman@openafs.org>
Reviewed-by: Jeffrey Altman <jaltman@openafs.org>

src/WINNT/afsd/cm_callback.c

index ac6a725..dda6bf6 100644 (file)
@@ -1930,7 +1930,7 @@ void cm_CheckCBExpiration(void)
 {
     afs_uint32 i;
     cm_scache_t *scp;
-    cm_volume_t *volp = NULL;
+    cm_volume_t *volp;
     enum volstatus volstate;
     time_t now, downTime;
         
@@ -1940,10 +1940,9 @@ void cm_CheckCBExpiration(void)
     lock_ObtainWrite(&cm_scacheLock);
     for (i=0; i<cm_data.scacheHashTableSize; i++) {
         for (scp = cm_data.scacheHashTablep[i]; scp; scp=scp->nextp) {
-            if (volp) {
-                cm_PutVolume(volp);
-                volp = NULL;
-            }
+            volp = NULL;
+            cm_HoldSCacheNoLock(scp);
+            lock_ReleaseWrite(&cm_scacheLock);
 
             /*
              * If this is not a PURERO object and there is no callback
@@ -1951,7 +1950,7 @@ void cm_CheckCBExpiration(void)
              */
             if (!(scp->flags & CM_SCACHEFLAG_PURERO) &&
                 (scp->cbServerp == NULL || scp->cbExpires == 0 || now < scp->cbExpires))
-                continue;
+                goto scp_complete;
 
             /*
              * Determine the volume state and update the callback info
@@ -1979,23 +1978,20 @@ void cm_CheckCBExpiration(void)
 
             /* If there is no callback or it hasn't expired, there is nothing to do */
             if (scp->cbServerp == NULL || scp->cbExpires == 0 || now < scp->cbExpires)
-                continue;
+                goto scp_complete;
 
             /* If the volume is known not to be online, do not expire the callback */
             if (volstate != vl_online)
-                continue;
+                goto scp_complete;
 
             /*
              * If all the servers are down and the callback expired after the
              * issuing server went down, do not expire the callback
              */
             if (cm_CBServersDownTime(scp, volp, &downTime) && downTime && downTime < scp->cbExpires)
-                continue;
+                goto scp_complete;
 
             /* The callback has expired, discard the status info */
-            cm_HoldSCacheNoLock(scp);
-            lock_ReleaseWrite(&cm_scacheLock);
-
             osi_Log4(afsd_logp, "Callback Expiration Discarding SCache scp 0x%p vol %u vn %u uniq %u",
                      scp, scp->fid.volume, scp->fid.vnode, scp->fid.unique);
             lock_ObtainWrite(&scp->rw);
@@ -2004,14 +2000,14 @@ void cm_CheckCBExpiration(void)
 
             cm_CallbackNotifyChange(scp);
 
+          scp_complete:
+            if (volp)
+                cm_PutVolume(volp);
+
             lock_ObtainWrite(&cm_scacheLock);
             cm_ReleaseSCacheNoLock(scp);
         }
     }
-    if (volp) {
-        cm_PutVolume(volp);
-        volp = NULL;
-    }
     lock_ReleaseWrite(&cm_scacheLock);
 
     osi_Log0(afsd_logp, "CheckCBExpiration Complete");