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.
This commit is contained in:
Jeffrey Altman 2006-10-06 17:31:30 +00:00
parent ffe5092c31
commit ffe26eb18d
4 changed files with 47 additions and 19 deletions

View File

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

View File

@ -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);

View File

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

View File

@ -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);
}