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

(cherry picked from commit ffe26eb18da581d2df02f9531d97e31e1c6dee65)

diff --git a/src/WINNT/afsd/cm_buf.c b/src/WINNT/afsd/cm_buf.c
index ee1a182f91bc1e50b0c300faa38e65c3c4c928ba..cc5f78556320dbf9dd37720210132f3fb85ff2d9 100644 (file)
--- a/src/WINNT/afsd/cm_buf.c
+++ b/src/WINNT/afsd/cm_buf.c
@@ -581,7 +581,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);
 
@@ -590,19 +590,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.
         */
@@ -610,6 +613,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 d90cf808fa4d8670512edd1b74df5270c7876f4c..c4232a67c34fc7c5a2e8cb78d3b2542430e7a671 100644 (file)
--- a/src/WINNT/afsd/cm_daemon.c
+++ b/src/WINNT/afsd/cm_daemon.c
@@ -89,12 +89,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 1d3f4ae195598f34877358ec1da831e6303861c1..971c27e278044716d758b8f90d3c915492798b38 100644 (file)
--- a/src/WINNT/afsd/cm_dcache.c
+++ b/src/WINNT/afsd/cm_dcache.c
@@ -76,6 +76,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 4d658f12dab591e5bb724dbf0fdd1fc7e7c6453d..edfa338ca47614f395836f711eeebde6b2f38c25 100644 (file)
--- a/src/WINNT/afsd/cm_scache.c
+++ b/src/WINNT/afsd/cm_scache.c
@@ -611,7 +611,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);
@@ -699,9 +703,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 */
 
@@ -724,7 +735,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:
@@ -1415,7 +1428,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)
@@ -1423,7 +1438,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);
 }
 
@@ -1433,7 +1450,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)
@@ -1444,7 +1463,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);
 }