windows-misc-fixes-20061006

#ifdef DEBUG_REFCOUNT the ref count log entries so they aren't always
compiled in

comment out the remaining location where the write lock on cm_scacheLock
is dropped in order to obtain the scache mutex on the object returned
from cm_GetNewSCache().  Dropping the lock results in more than one
thread being given the same cm_scache_t which is more dangerous than
blowing away the contents of the object without holding the mutex

ensure that cm_BufWrite is always called with a non-NULL scp.  Add
an assertion to double check that we do it all the time.

diff --git a/src/WINNT/afsd/cm_buf.c b/src/WINNT/afsd/cm_buf.c
index 529c122..a8a6347 100644 (file)
--- a/src/WINNT/afsd/cm_buf.c
+++ b/src/WINNT/afsd/cm_buf.c
@@ -535,7 +535,7 @@ long buf_CleanAsyncLocked(cm_buf_t *bp, cm_req_t *reqp)
 {
     long code = 0;
     long isdirty = 0;
-       cm_scache_t * scp = NULL;
+    cm_scache_t * scp = NULL;
 
     osi_assert(bp->magic == CM_BUF_MAGIC);
 
@@ -544,19 +544,22 @@ long buf_CleanAsyncLocked(cm_buf_t *bp, cm_req_t *reqp)
         lock_ReleaseMutex(&bp->mx);
 
        scp = cm_FindSCache(&bp->fid);
-       osi_Log2(buf_logp, "buf_CleanAsyncLocked starts I/O on scp 0x%p buf 0x%p", scp, bp);
-        code = (*cm_buf_opsp->Writep)(scp, &bp->offset,
-                                       cm_data.buf_blockSize, 0, bp->userp,
-                                       reqp);
-       osi_Log3(buf_logp, "buf_CleanAsyncLocked I/O on scp 0x%p buf 0x%p, done=%d", scp, bp, code);
-
        if (scp) {
+           osi_Log2(buf_logp, "buf_CleanAsyncLocked starts I/O on scp 0x%p buf 0x%p", scp, bp);
+           code = (*cm_buf_opsp->Writep)(scp, &bp->offset,
+                                          cm_data.buf_blockSize, 0, bp->userp,
+                                          reqp);
+           osi_Log3(buf_logp, "buf_CleanAsyncLocked I/O on scp 0x%p buf 0x%p, done=%d", scp, bp, code);
+
            cm_ReleaseSCache(scp);
            scp = NULL;
-       }
-                
-        lock_ObtainMutex(&bp->mx);
-       /* if the Write routine returns No Such File, clear the dirty flag 
+       } else {
+           osi_Log1(buf_logp, "buf_CleanAsyncLocked unable to start I/O - scp not found buf 0x%p", bp);
+           code = CM_ERROR_NOSUCHFILE;
+       }    
+        
+       lock_ObtainMutex(&bp->mx);
+       /* if the Write routine returns No Such File, clear the dirty flag
         * because we aren't going to be able to write this data to the file
         * server.
         */
@@ -564,6 +567,8 @@ long buf_CleanAsyncLocked(cm_buf_t *bp, cm_req_t *reqp)
            bp->flags &= ~CM_BUF_DIRTY;
            bp->flags |= CM_BUF_ERROR;
            bp->error = CM_ERROR_NOSUCHFILE;
+           bp->dataVersion = -1; /* bad */
+           bp->dirtyCounter++;
        }
 
 #ifdef DISKCACHE95

diff --git a/src/WINNT/afsd/cm_daemon.c b/src/WINNT/afsd/cm_daemon.c
index 06488f1..b2848fc 100644 (file)
--- a/src/WINNT/afsd/cm_daemon.c
+++ b/src/WINNT/afsd/cm_daemon.c
@@ -83,12 +83,13 @@ void cm_BkgDaemon(long parm)
         osi_assert(cm_bkgQueueCount-- > 0);
         lock_ReleaseWrite(&cm_daemonLock);
 
+#ifdef DEBUG_REFCOUNT
        osi_Log2(afsd_logp,"cm_BkgDaemon (before) scp 0x%x ref %d",rp->scp, rp->scp->refCount);
-
+#endif
         (*rp->procp)(rp->scp, rp->p1, rp->p2, rp->p3, rp->p4, rp->userp);
-                
+#ifdef DEBUG_REFCOUNT                
        osi_Log2(afsd_logp,"cm_BkgDaemon (after) scp 0x%x ref %d",rp->scp, rp->scp->refCount);
-
+#endif
        cm_ReleaseUser(rp->userp);
         cm_ReleaseSCache(rp->scp);
         free(rp);

diff --git a/src/WINNT/afsd/cm_dcache.c b/src/WINNT/afsd/cm_dcache.c
index c6c1d8b..56c0fff 100644 (file)
--- a/src/WINNT/afsd/cm_dcache.c
+++ b/src/WINNT/afsd/cm_dcache.c
@@ -74,6 +74,7 @@ long cm_BufWrite(void *vscp, osi_hyper_t *offsetp, long length, long flags,
     int require_64bit_ops = 0;
 
     osi_assert(userp != NULL);
+    osi_assert(scp != NULL);
 
     /* now, the buffer may or may not be filled with good data (buf_GetNew
      * drops lots of locks, and may indeed return a properly initialized

diff --git a/src/WINNT/afsd/cm_scache.c b/src/WINNT/afsd/cm_scache.c
index c0a54ec..76b8a89 100644 (file)
--- a/src/WINNT/afsd/cm_scache.c
+++ b/src/WINNT/afsd/cm_scache.c
@@ -609,7 +609,11 @@ long cm_GetSCache(cm_fid_t *fidp, cm_scache_t **outScpp, cm_user_t *userp,
        }
 
 #if not_too_dangerous
-       /* dropping the cm_scacheLock is dangerous */
+       /* dropping the cm_scacheLock allows more than one thread
+        * to obtain the same cm_scache_t from the LRU list.  Since
+        * the refCount is known to be zero at this point we have to
+        * assume that no one else is using the one this is returned.
+        */
        lock_ReleaseWrite(&cm_scacheLock);
        lock_ObtainMutex(&scp->mx);
        lock_ObtainWrite(&cm_scacheLock);
@@ -697,9 +701,16 @@ long cm_GetSCache(cm_fid_t *fidp, cm_scache_t **outScpp, cm_user_t *userp,
 
     osi_assert(!(scp->flags & CM_SCACHEFLAG_INHASH));
 
+#if not_too_dangerous
+    /* dropping the cm_scacheLock allows more than one thread
+     * to obtain the same cm_scache_t from the LRU list.  Since
+     * the refCount is known to be zero at this point we have to
+     * assume that no one else is using the one this is returned.
+     */
     lock_ReleaseWrite(&cm_scacheLock);
     lock_ObtainMutex(&scp->mx);
     lock_ObtainWrite(&cm_scacheLock);
+#endif
     scp->fid = *fidp;
     scp->volp = volp;  /* a held reference */
 
@@ -722,7 +733,9 @@ long cm_GetSCache(cm_fid_t *fidp, cm_scache_t **outScpp, cm_user_t *userp,
     scp->flags |= CM_SCACHEFLAG_INHASH;
     scp->refCount = 1;
     osi_Log1(afsd_logp,"cm_GetSCache sets refCount to 1 scp 0x%x", scp);
+#if not_too_dangerous
     lock_ReleaseMutex(&scp->mx);
+#endif
 
     /* XXX - The following fields in the cm_scache are 
      * uninitialized:
@@ -1413,7 +1426,9 @@ void cm_HoldSCacheNoLock(cm_scache_t *scp)
 {
     osi_assert(scp != 0);
     scp->refCount++;
-    osi_Log2(afsd_logp,"cm_HoldSCacheNoLock scp 0x%x ref %d",scp, scp->refCount);
+#ifdef DEBUG_REFCOUNT
+    osi_Log2(afsd_logp,"cm_HoldSCacheNoLock scp 0x%p ref %d",scp, scp->refCount);
+#endif
 }
 
 void cm_HoldSCache(cm_scache_t *scp)
@@ -1421,7 +1436,9 @@ void cm_HoldSCache(cm_scache_t *scp)
     osi_assert(scp != 0);
     lock_ObtainWrite(&cm_scacheLock);
     scp->refCount++;
-    osi_Log2(afsd_logp,"cm_HoldSCache scp 0x%x ref %d",scp, scp->refCount);
+#ifdef DEBUG_REFCOUNT
+    osi_Log2(afsd_logp,"cm_HoldSCache scp 0x%p ref %d",scp, scp->refCount);
+#endif
     lock_ReleaseWrite(&cm_scacheLock);
 }
 
@@ -1431,7 +1448,9 @@ void cm_ReleaseSCacheNoLock(cm_scache_t *scp)
     if (scp->refCount == 0)
        osi_Log1(afsd_logp,"cm_ReleaseSCacheNoLock about to panic scp 0x%x",scp);
     osi_assert(scp->refCount-- >= 0);
-    osi_Log2(afsd_logp,"cm_ReleaseSCacheNoLock scp 0x%x ref %d",scp,scp->refCount);
+#ifdef DEBUG_REFCOUNT
+    osi_Log2(afsd_logp,"cm_ReleaseSCacheNoLock scp 0x%p ref %d",scp,scp->refCount);
+#endif
 }
 
 void cm_ReleaseSCache(cm_scache_t *scp)
@@ -1442,7 +1461,9 @@ void cm_ReleaseSCache(cm_scache_t *scp)
        osi_Log1(afsd_logp,"cm_ReleaseSCache about to panic scp 0x%x",scp);
     osi_assert(scp->refCount != 0);
     scp->refCount--;
-    osi_Log2(afsd_logp,"cm_ReleaseSCache scp 0x%x ref %d",scp,scp->refCount);
+#ifdef DEBUG_REFCOUNT
+    osi_Log2(afsd_logp,"cm_ReleaseSCache scp 0x%p ref %d",scp,scp->refCount);
+#endif
     lock_ReleaseWrite(&cm_scacheLock);
 }