windows-misc-fix-20061004
authorJeffrey Altman <jaltman@secure-endpoints.com>
Thu, 5 Oct 2006 06:39:46 +0000 (06:39 +0000)
committerJeffrey Altman <jaltman@secure-endpoints.com>
Thu, 5 Oct 2006 06:39:46 +0000 (06:39 +0000)
more cleanup from recent patches.  comment out the recycling code because
it is not possible to implement it using the current locking hierarchy.

change cm_BufWrite to take a pointer to cm_scache_t instead of a fid
which must be used to look up a new reference to the cm_scache_t.
more often than not we already have the scp and in the one case we
don't we can let the caller look up the scp and then call cm_BufWrite
if it is found.  If not, we have saved a function call and a bunch
of lock operations.

add a lot more logging.

improve the scp mutex handling within smb_CloseFID

src/WINNT/afsd/cm_buf.c
src/WINNT/afsd/cm_daemon.c
src/WINNT/afsd/cm_dcache.c
src/WINNT/afsd/cm_dnlc.c
src/WINNT/afsd/cm_scache.c
src/WINNT/afsd/cm_scache.h
src/WINNT/afsd/smb.c

index 460bf84..a279116 100644 (file)
@@ -535,6 +535,7 @@ long buf_CleanAsyncLocked(cm_buf_t *bp, cm_req_t *reqp)
 {
     long code = 0;
     long isdirty = 0;
+       cm_scache_t * scp = NULL;
 
     osi_assert(bp->magic == CM_BUF_MAGIC);
 
@@ -542,11 +543,17 @@ long buf_CleanAsyncLocked(cm_buf_t *bp, cm_req_t *reqp)
        isdirty = 1;
         lock_ReleaseMutex(&bp->mx);
 
-       osi_Log1(buf_logp, "buf_CleanAsyncLocked starts I/O on 0x%p", bp);
-        code = (*cm_buf_opsp->Writep)(&bp->fid, &bp->offset,
+       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_Log2(buf_logp, "buf_CleanAsyncLocked I/O on 0x%p, done=%d", bp, code);
+       osi_Log3(buf_logp, "buf_CleanAsyncLocked I/O on scp 0x%p buf 0x%p, done=%d", scp, bp, code);
+
+       if (scp) {
+           cm_ReleaseSCache(scp);
+           scp = NULL;
+       }
                 
         lock_ObtainMutex(&bp->mx);
        /* if the Write routine returns No Such File, clear the dirty flag 
@@ -1557,6 +1564,7 @@ long buf_DirtyBuffersExist(cm_fid_t *fidp)
     return 0;
 }
 
+#if 0
 long buf_CleanDirtyBuffers(cm_scache_t *scp)
 {
     cm_buf_t *bp;
@@ -1583,4 +1591,4 @@ long buf_CleanDirtyBuffers(cm_scache_t *scp)
     }
     return 0;
 }
-
+#endif
index 5b1fe41..06488f1 100644 (file)
@@ -83,9 +83,13 @@ void cm_BkgDaemon(long parm)
         osi_assert(cm_bkgQueueCount-- > 0);
         lock_ReleaseWrite(&cm_daemonLock);
 
+       osi_Log2(afsd_logp,"cm_BkgDaemon (before) scp 0x%x ref %d",rp->scp, rp->scp->refCount);
+
         (*rp->procp)(rp->scp, rp->p1, rp->p2, rp->p3, rp->p4, rp->userp);
                 
-        cm_ReleaseUser(rp->userp);
+       osi_Log2(afsd_logp,"cm_BkgDaemon (after) scp 0x%x ref %d",rp->scp, rp->scp->refCount);
+
+       cm_ReleaseUser(rp->userp);
         cm_ReleaseSCache(rp->scp);
         free(rp);
 
index 9363f1b..a36e43b 100644 (file)
@@ -44,7 +44,7 @@ extern osi_mutex_t cm_Freelance_Lock;
 /* functions called back from the buffer package when reading or writing data,
  * or when holding or releasing a vnode pointer.
  */
-long cm_BufWrite(void *vfidp, osi_hyper_t *offsetp, long length, long flags,
+long cm_BufWrite(void *vscp, osi_hyper_t *offsetp, long length, long flags,
                  cm_user_t *userp, cm_req_t *reqp)
 {
     /* store the data back from this buffer; the buffer is locked and held,
@@ -54,8 +54,7 @@ long cm_BufWrite(void *vfidp, osi_hyper_t *offsetp, long length, long flags,
      * bufp->scp.
      */
     long code;
-    cm_fid_t *fidp = vfidp;
-    cm_scache_t *scp;
+    cm_scache_t *scp = vscp;
     long nbytes;
     long temp;
     AFSFetchStatus outStatus;
@@ -80,17 +79,12 @@ long cm_BufWrite(void *vfidp, osi_hyper_t *offsetp, long length, long flags,
      * drops lots of locks, and may indeed return a properly initialized
      * buffer, although more likely it will just return a new, empty, buffer.
      */
-    scp = cm_FindSCache(fidp);
-    if (scp == NULL) {
-        return CM_ERROR_NOSUCHFILE;    /* shouldn't happen */
-    }
-
-    cm_AFSFidFromFid(&tfid, fidp);
 
     lock_ObtainMutex(&scp->mx);
+    cm_AFSFidFromFid(&tfid, &scp->fid);
+
     if (scp->flags & CM_SCACHEFLAG_DELETED) {
        lock_ReleaseMutex(&scp->mx);
-        cm_ReleaseSCache(scp);
        return CM_ERROR_NOSUCHFILE;
     }
 
@@ -98,7 +92,6 @@ long cm_BufWrite(void *vfidp, osi_hyper_t *offsetp, long length, long flags,
     if (code) {
         osi_Log1(afsd_logp, "cm_SetupStoreBIOD code %x", code);
         lock_ReleaseMutex(&scp->mx);
-        cm_ReleaseSCache(scp);
         return code;
     }
 
@@ -106,7 +99,6 @@ long cm_BufWrite(void *vfidp, osi_hyper_t *offsetp, long length, long flags,
         osi_Log0(afsd_logp, "cm_SetupStoreBIOD length 0");
         lock_ReleaseMutex(&scp->mx);
         cm_ReleaseBIOD(&biod, 1);      /* should be a NOOP */
-        cm_ReleaseSCache(scp);
         return 0;
     }
 
@@ -299,7 +291,6 @@ long cm_BufWrite(void *vfidp, osi_hyper_t *offsetp, long length, long flags,
     }
     lock_ReleaseMutex(&scp->mx);
     cm_ReleaseBIOD(&biod, 1);
-    cm_ReleaseSCache(scp);
 
     return code;
 }
@@ -431,13 +422,11 @@ long cm_BufRead(cm_buf_t *bufp, long nbytes, long *bytesReadp, cm_user_t *userp)
 /* stabilize scache entry, and return with it locked so 
  * it stays stable.
  */
-long cm_BufStabilize(void *parmp, cm_user_t *userp, cm_req_t *reqp)
+long cm_BufStabilize(void *vscp, cm_user_t *userp, cm_req_t *reqp)
 {
-    cm_scache_t *scp;
+    cm_scache_t *scp = vscp;
     long code;
 
-    scp = parmp;
-        
     lock_ObtainMutex(&scp->mx);
     code = cm_SyncOp(scp, NULL, userp, reqp, 0, 
                      CM_SCACHESYNC_NEEDCALLBACK | CM_SCACHESYNC_GETSTATUS | CM_SCACHESYNC_SETSIZE);
@@ -450,11 +439,9 @@ long cm_BufStabilize(void *parmp, cm_user_t *userp, cm_req_t *reqp)
 }
 
 /* undoes the work that cm_BufStabilize does: releases lock so things can change again */
-long cm_BufUnstabilize(void *parmp, cm_user_t *userp)
+long cm_BufUnstabilize(void *vscp, cm_user_t *userp)
 {
-    cm_scache_t *scp;
-        
-    scp = parmp;
+    cm_scache_t *scp = vscp;
         
     cm_SyncOpDone(scp, NULL, CM_SCACHESYNC_NEEDCALLBACK | CM_SCACHESYNC_GETSTATUS | CM_SCACHESYNC_SETSIZE);
 
@@ -623,7 +610,7 @@ void cm_BkgStore(cm_scache_t *scp, afs_uint32 p1, afs_uint32 p2, afs_uint32 p3,
 
        osi_Log4(afsd_logp, "Starting BKG store scp 0x%p, offset 0x%x:%08x, length 0x%x", scp, p2, p1, p3);
 
-       code = cm_BufWrite(&scp->fid, &toffset, length, /* flags */ 0, userp, &req);
+       code = cm_BufWrite(scp, &toffset, length, /* flags */ 0, userp, &req);
     }
 
     lock_ObtainMutex(&scp->mx);
@@ -650,7 +637,7 @@ void cm_ClearPrefetchFlag(long code, cm_scache_t *scp, osi_hyper_t *base)
 }
 
 /* do the prefetch */
-void cm_BkgPrefetch(cm_scache_t *scp, long p1, long p2, long p3, long p4,
+void cm_BkgPrefetch(cm_scache_t *scp, afs_uint32 p1, afs_uint32 p2, afs_uint32 p3, afs_uint32 p4,
                     cm_user_t *userp)
 {
     long length;
@@ -756,7 +743,7 @@ long cm_SetupStoreBIOD(cm_scache_t *scp, osi_hyper_t *inOffsetp, long inSize,
     long flags;                        /* flags to cm_SyncOp */
         
     /* clear things out */
-    biop->scp = scp;           /* don't hold */
+    biop->scp = scp;                   /* do not hold; held by caller */
     biop->offset = *inOffsetp;
     biop->length = 0;
     biop->bufListp = NULL;
@@ -991,7 +978,7 @@ long cm_SetupFetchBIOD(cm_scache_t *scp, osi_hyper_t *offsetp,
     osi_queueData_t *heldBufListEndp;  /* first one */
     int reserving;
 
-    biop->scp = scp;
+    biop->scp = scp;                   /* do not hold; held by caller */
     biop->offset = *offsetp;
     /* null out the list of buffers */
     biop->bufListp = biop->bufListEndp = NULL;
@@ -1205,7 +1192,7 @@ long cm_SetupFetchBIOD(cm_scache_t *scp, osi_hyper_t *offsetp,
  */
 void cm_ReleaseBIOD(cm_bulkIO_t *biop, int isStore)
 {
-    cm_scache_t *scp;
+    cm_scache_t *scp;          /* do not release; not held in biop */
     cm_buf_t *bufp;
     osi_queueData_t *qdp;
     osi_queueData_t *nqdp;
index 9cc0bf9..df60841 100644 (file)
@@ -185,14 +185,14 @@ cm_dnlcLookup (cm_scache_t *adp, cm_lookupSearch_t* sp)
     int safety, match;
   
     if (!cm_useDnlc)
-       return 0;
+       return NULL;
     if ( cm_debugDnlc ) 
        osi_Log2(afsd_logp, "cm_dnlcLookup dir %x name %s", 
                adp, osi_LogSaveString(afsd_logp,aname));
 
     dnlcHash( ts, key );  /* leaves ts pointing at the NULL */
     if (ts - aname >= CM_AFSNCNAMESIZE) 
-       return 0;
+       return NULL;
 
     skey = key & (NHSIZE -1);
 
@@ -262,7 +262,7 @@ cm_dnlcLookup (cm_scache_t *adp, cm_lookupSearch_t* sp)
            if ( cm_debugDnlc ) 
                osi_Log0(afsd_logp, "DNLC cycle"); 
            cm_dnlcPurge();
-           return(0);
+           return(NULL);
        }
     }
 
index c740afa..6305431 100644 (file)
@@ -50,14 +50,20 @@ void cm_AdjustLRU(cm_scache_t *scp)
         cm_data.scacheLRULastp = scp;
 }
 
-/* called with cm_scacheLock write-locked; recycles an existing scp. */
+/* called with cm_scacheLock write-locked; recycles an existing scp. 
+ *
+ * this function ignores all of the locking hierarchy.  
+ */
 long cm_RecycleSCache(cm_scache_t *scp, afs_int32 flags)
 {
     cm_scache_t **lscpp;
     cm_scache_t *tscp;
     int i;
 
-    lock_ObtainMutex(&scp->mx);
+    if (scp->refCount != 0) {
+       return -1;
+    }
+
     if (scp->flags & CM_SCACHEFLAG_INHASH) {
        /* hash it out first */
        i = CM_SCACHE_HASH(&scp->fid);
@@ -70,9 +76,9 @@ long cm_RecycleSCache(cm_scache_t *scp, afs_int32 flags)
                break;
            }
        }
-       osi_assertx(tscp, "afsd: scache hash screwup");
     }
 
+#if 0
     if (flags & CM_SCACHE_RECYCLEFLAG_DESTROY_BUFFERS) {
        osi_queueData_t *qdp;
        cm_buf_t *bufp;
@@ -82,7 +88,6 @@ long cm_RecycleSCache(cm_scache_t *scp, afs_int32 flags)
            osi_QRemove((osi_queue_t **) &scp->bufWritesp, &qdp->q);
            osi_QDFree(qdp);
            if (bufp) {
-               lock_ReleaseMutex(&scp->mx);
                lock_ObtainMutex(&bufp->mx);
                bufp->cmFlags &= ~CM_BUF_CMSTORING;
                bufp->flags &= ~CM_BUF_DIRTY;
@@ -96,7 +101,6 @@ long cm_RecycleSCache(cm_scache_t *scp, afs_int32 flags)
                }
                lock_ReleaseMutex(&bufp->mx);
                buf_Release(bufp);
-               lock_ObtainMutex(&scp->mx);
            }
         }
        while(qdp = scp->bufReadsp) {
@@ -104,7 +108,6 @@ long cm_RecycleSCache(cm_scache_t *scp, afs_int32 flags)
            osi_QRemove((osi_queue_t **) &scp->bufReadsp, &qdp->q);
            osi_QDFree(qdp);
            if (bufp) {
-               lock_ReleaseMutex(&scp->mx);
                lock_ObtainMutex(&bufp->mx);
                bufp->cmFlags &= ~CM_BUF_CMFETCHING;
                bufp->flags &= ~CM_BUF_DIRTY;
@@ -118,7 +121,6 @@ long cm_RecycleSCache(cm_scache_t *scp, afs_int32 flags)
                }
                lock_ReleaseMutex(&bufp->mx);
                buf_Release(bufp);
-               lock_ObtainMutex(&scp->mx);
            }
         }
        buf_CleanDirtyBuffers(scp); 
@@ -127,6 +129,7 @@ long cm_RecycleSCache(cm_scache_t *scp, afs_int32 flags)
        osi_assert(scp->bufWritesp == NULL);
        osi_assert(scp->bufReadsp == NULL);
     }
+#endif
 
     /* invalidate so next merge works fine;
      * also initialize some flags */
@@ -183,10 +186,6 @@ long cm_RecycleSCache(cm_scache_t *scp, afs_int32 flags)
      * while we hold the global refcount lock.
      */
     cm_FreeAllACLEnts(scp);
-
-    osi_Wakeup((long)&scp->flags);
-
-    lock_ReleaseMutex(&scp->mx);
     return 0;
 }
 
@@ -199,6 +198,7 @@ cm_scache_t *cm_GetNewSCache(void)
     cm_scache_t *scp;
     int retry = 0;
 
+#if 0
     /* first pass - look for deleted objects */
     for ( scp = cm_data.scacheLRULastp;
          scp;
@@ -234,6 +234,7 @@ cm_scache_t *cm_GetNewSCache(void)
        }       
     }  
     osi_Log0(afsd_logp, "GetNewSCache no deleted or recycled entries available for reuse");
+#endif 
 
     if (cm_data.currentSCaches >= cm_data.maxSCaches) {
        /* There were no deleted scache objects that we could use.  Try to find
@@ -511,9 +512,6 @@ cm_scache_t *cm_FindSCache(cm_fid_t *fidp)
     hash = CM_SCACHE_HASH(fidp);
 
     if (fidp->cell == 0) {
-#ifdef DEBUG
-       DebugBreak();
-#endif
        return NULL;
     }
 
@@ -551,7 +549,7 @@ long cm_GetSCache(cm_fid_t *fidp, cm_scache_t **outScpp, cm_user_t *userp,
          fidp->volume==cm_data.rootFid.volume &&
          fidp->vnode==0x0 && fidp->unique==0x0)
     {
-        osi_Log0(afsd_logp,"cm_getSCache called with root cell/volume and vnode=0 and unique=0");
+        osi_Log0(afsd_logp,"cm_GetSCache called with root cell/volume and vnode=0 and unique=0");
     }
 
     // yj: check if we have the scp, if so, we don't need
@@ -583,7 +581,7 @@ long cm_GetSCache(cm_fid_t *fidp, cm_scache_t **outScpp, cm_user_t *userp,
               fidp->volume==AFS_FAKE_ROOT_VOL_ID &&
               fidp->vnode==0x1 && fidp->unique==0x1);
     if (cm_freelanceEnabled && isRoot) {
-        osi_Log0(afsd_logp,"cm_getSCache Freelance and isRoot");
+        osi_Log0(afsd_logp,"cm_GetSCache Freelance and isRoot");
         /* freelance: if we are trying to get the root scp for the first
          * time, we will just put in a place holder entry. 
          */
@@ -591,7 +589,7 @@ long cm_GetSCache(cm_fid_t *fidp, cm_scache_t **outScpp, cm_user_t *userp,
     }
          
     if (cm_freelanceEnabled && special) {
-        osi_Log0(afsd_logp,"cm_getSCache Freelance and special");
+        osi_Log0(afsd_logp,"cm_GetSCache Freelance and special");
         if (fidp->vnode > 1 && fidp->vnode <= cm_noLocalMountPoints + 2) {
            lock_ObtainMutex(&cm_Freelance_Lock);
             mp =(cm_localMountPoints+fidp->vnode-2)->mountPointStringp;
@@ -601,12 +599,14 @@ long cm_GetSCache(cm_fid_t *fidp, cm_scache_t **outScpp, cm_user_t *userp,
         }
         scp = cm_GetNewSCache();
        if (scp == NULL) {
-           osi_Log0(afsd_logp,"cm_getSCache unable to obtain *new* scache entry");
+           osi_Log0(afsd_logp,"cm_GetSCache unable to obtain *new* scache entry");
             lock_ReleaseWrite(&cm_scacheLock);
            return CM_ERROR_WOULDBLOCK;
        }
 
+       lock_ReleaseWrite(&cm_scacheLock);
        lock_ObtainMutex(&scp->mx);
+       lock_ObtainWrite(&cm_scacheLock);
         scp->fid = *fidp;
         scp->volp = cm_data.rootSCachep->volp;
         scp->dotdotFid.cell=AFS_FAKE_ROOT_CELL_ID;
@@ -618,6 +618,7 @@ long cm_GetSCache(cm_fid_t *fidp, cm_scache_t **outScpp, cm_user_t *userp,
         cm_data.hashTablep[hash]=scp;
         scp->flags |= CM_SCACHEFLAG_INHASH;
         scp->refCount = 1;
+       osi_Log1(afsd_logp,"cm_GetSCache (freelance) sets refCount to 1 scp 0x%x", scp);
         if (fidp->vnode > 1 && fidp->vnode <= cm_noLocalMountPoints + 2)
             scp->fileType = (cm_localMountPoints+fidp->vnode-2)->fileType;
         else 
@@ -679,13 +680,16 @@ long cm_GetSCache(cm_fid_t *fidp, cm_scache_t **outScpp, cm_user_t *userp,
     /* now, if we don't have the fid, recycle something */
     scp = cm_GetNewSCache();
     if (scp == NULL) {
-       osi_Log0(afsd_logp,"cm_getSCache unable to obtain *new* scache entry");
+       osi_Log0(afsd_logp,"cm_GetNewSCache unable to obtain *new* scache entry");
        lock_ReleaseWrite(&cm_scacheLock);
        return CM_ERROR_WOULDBLOCK;
     }
+    osi_Log2(afsd_logp,"cm_GetNewSCache returns scp 0x%x flags 0x%x", scp, scp->flags);
 
     osi_assert(!(scp->flags & CM_SCACHEFLAG_INHASH));
+    lock_ReleaseWrite(&cm_scacheLock);
     lock_ObtainMutex(&scp->mx);
+    lock_ObtainWrite(&cm_scacheLock);
     scp->fid = *fidp;
     scp->volp = volp;  /* a held reference */
 
@@ -707,6 +711,7 @@ long cm_GetSCache(cm_fid_t *fidp, cm_scache_t **outScpp, cm_user_t *userp,
     cm_data.hashTablep[hash] = scp;
     scp->flags |= CM_SCACHEFLAG_INHASH;
     scp->refCount = 1;
+    osi_Log1(afsd_logp,"cm_GetSCache sets refCount to 1 scp 0x%x", scp);
     lock_ReleaseMutex(&scp->mx);
 
     /* XXX - The following fields in the cm_scache are 
@@ -731,7 +736,7 @@ cm_scache_t * cm_FindSCacheParent(cm_scache_t * scp)
     cm_fid_t    parent_fid;
     cm_scache_t * pscp = NULL;
 
-    lock_ObtainWrite(&cm_scacheLock);
+    lock_ObtainRead(&cm_scacheLock);
     parent_fid = scp->fid;
     parent_fid.vnode = scp->parentVnode;
     parent_fid.unique = scp->parentUnique;
@@ -746,7 +751,7 @@ cm_scache_t * cm_FindSCacheParent(cm_scache_t * scp)
            }
        }
     }
-    lock_ReleaseWrite(&cm_scacheLock);
+    lock_ReleaseRead(&cm_scacheLock);
 
     return pscp;
 }
@@ -1396,31 +1401,37 @@ void cm_AFSFidFromFid(AFSFid *afsFidp, cm_fid_t *fidp)
 void cm_HoldSCacheNoLock(cm_scache_t *scp)
 {
     osi_assert(scp != 0);
-    osi_assert(scp->refCount >= 0);
     scp->refCount++;
+    osi_Log2(afsd_logp,"cm_HoldSCacheNoLock scp 0x%x ref %d",scp, scp->refCount);
 }
 
 void cm_HoldSCache(cm_scache_t *scp)
 {
     osi_assert(scp != 0);
     lock_ObtainWrite(&cm_scacheLock);
-    osi_assert(scp->refCount >= 0);
     scp->refCount++;
+    osi_Log2(afsd_logp,"cm_HoldSCache scp 0x%x ref %d",scp, scp->refCount);
     lock_ReleaseWrite(&cm_scacheLock);
 }
 
 void cm_ReleaseSCacheNoLock(cm_scache_t *scp)
 {
-    osi_assert(scp != 0);
+    osi_assert(scp != NULL);
+    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);
 }
 
 void cm_ReleaseSCache(cm_scache_t *scp)
 {
-    osi_assert(scp != 0);
+    osi_assert(scp != NULL);
     lock_ObtainWrite(&cm_scacheLock);
+    if (scp->refCount == 0)
+       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);
     lock_ReleaseWrite(&cm_scacheLock);
 }
 
index 724a62d..6e034ae 100644 (file)
@@ -242,6 +242,7 @@ typedef struct cm_scache {
                        (CM_SCACHEFLAG_WATCHED | CM_SCACHEFLAG_WATCHEDSUBTREE)
 
 #define CM_SCACHEFLAG_EACCESS           0x200000 /* Bulk Stat returned EACCES */
+#define CM_SCACHEFLAG_RECYCLING                0x400000
 
 /* sync flags for calls to the server.  The CM_SCACHEFLAG_FETCHING,
  * CM_SCACHEFLAG_STORING and CM_SCACHEFLAG_SIZESTORING flags correspond to the
index 2824c49..8bada6f 100644 (file)
@@ -5604,7 +5604,9 @@ long smb_CloseFID(smb_vc_t *vcp, smb_fid_t *fidp, cm_user_t *userp,
     cm_req_t req;
     cm_scache_t *dscp = fidp->NTopen_dscp;
     char *pathp = fidp->NTopen_pathp;
-    cm_scache_t * scp;
+    cm_scache_t * scp = fidp->scp;
+    int deleted = 0;
+    int nullcreator = 0;
 
     osi_Log3(smb_logp, "smb_CloseFID Closing fidp 0x%x (fid=%d vcp=0x%x)",
              fidp, fidp->fid, vcp);
@@ -5641,10 +5643,6 @@ long smb_CloseFID(smb_vc_t *vcp, smb_fid_t *fidp, cm_user_t *userp,
         lock_ObtainMutex(&fidp->mx);
     }
 
-    scp = fidp->scp;
-    if (scp)
-       cm_HoldSCache(scp);
-
     /* watch for ioctl closes, and read-only opens */
     if (scp != NULL &&
         (fidp->flags & (SMB_FID_OPENWRITE | SMB_FID_DELONCLOSE))
@@ -5704,7 +5702,7 @@ long smb_CloseFID(smb_vc_t *vcp, smb_fid_t *fidp, cm_user_t *userp,
         if (scp->fileType == CM_SCACHETYPE_DIRECTORY) {
             code = cm_RemoveDir(dscp, fullPathp, userp, &req);
            if (code == 0) {
-               scp->flags |= CM_SCACHEFLAG_DELETED;
+               deleted = 1;
                if (dscp->flags & CM_SCACHEFLAG_ANYWATCH)
                    smb_NotifyChange(FILE_ACTION_REMOVED,
                                      FILE_NOTIFY_CHANGE_DIR_NAME | FILE_NOTIFY_CHANGE_CREATION,
@@ -5713,7 +5711,7 @@ long smb_CloseFID(smb_vc_t *vcp, smb_fid_t *fidp, cm_user_t *userp,
         } else {
             code = cm_Unlink(dscp, fullPathp, userp, &req);
            if (code == 0) {                            
-               scp->flags |= CM_SCACHEFLAG_DELETED;
+               deleted = 1;
                if (dscp->flags & CM_SCACHEFLAG_ANYWATCH)
                    smb_NotifyChange(FILE_ACTION_REMOVED,
                                      FILE_NOTIFY_CHANGE_FILE_NAME | FILE_NOTIFY_CHANGE_CREATION,
@@ -5728,10 +5726,7 @@ long smb_CloseFID(smb_vc_t *vcp, smb_fid_t *fidp, cm_user_t *userp,
     /* if this was a newly created file, then clear the creator
      * in the stat cache entry. */
     if (fidp->flags & SMB_FID_CREATED) {
-        lock_ObtainMutex(&scp->mx);
-       if (scp->creator == userp)
-           scp->creator = NULL;
-       lock_ReleaseMutex(&scp->mx);
+       nullcreator = 1;
        fidp->flags &= ~SMB_FID_CREATED;
     }
 
@@ -5744,13 +5739,23 @@ long smb_CloseFID(smb_vc_t *vcp, smb_fid_t *fidp, cm_user_t *userp,
         free(fidp->NTopen_wholepathp);
         fidp->NTopen_wholepathp = NULL;
     }
+    fidp->scp = NULL;
     lock_ReleaseMutex(&fidp->mx);
 
     if (dscp)
        cm_ReleaseSCache(dscp);
 
-    if (scp)
+    if (scp) {
+       if (deleted || nullcreator) {
+           lock_ObtainMutex(&scp->mx);
+           if (nullcreator && scp->creator == userp)
+               scp->creator = NULL;
+           if (deleted)
+               scp->flags |= CM_SCACHEFLAG_DELETED;
+           lock_ReleaseMutex(&scp->mx);
+       }
        cm_ReleaseSCache(scp);
+    }
 
     if (pathp)
        free(pathp);