From 9a55866037f3742b0d418fc699424301cf7a27eb Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Wed, 21 Dec 2011 16:05:40 -0500 Subject: [PATCH] afs: Cope with afs_GetValidDSlot errors Make callers of afs_GetValidDSlot deal with getting a NULL dcache, which can occur if an error is encountered. Some of these just panic at least for now, since a code path for recovery is complex, but this is at least better than dereferencing a NULL pointer. Reviewed-on: http://gerrit.openafs.org/6418 Tested-by: BuildBot Reviewed-by: Derrick Brashear (cherry picked from commit 9ed26da26f4a5a3fef3bf0a7b6f9dae751ce6659) Change-Id: I79c6fb3ae6279b5da482f95b4d4ed457beeaf1dd Reviewed-on: http://gerrit.openafs.org/7941 Reviewed-by: Derrick Brashear Tested-by: BuildBot --- src/afs/afs_buffer.c | 2 +- src/afs/afs_daemons.c | 2 +- src/afs/afs_dcache.c | 34 ++++++++++++++++++++++------------ src/afs/afs_disconnected.c | 10 ++++++---- src/afs/afs_pioctl.c | 3 +++ src/afs/afs_segments.c | 20 +++++++++++++++++++- 6 files changed, 52 insertions(+), 19 deletions(-) diff --git a/src/afs/afs_buffer.c b/src/afs/afs_buffer.c index 6563656..bf04c42 100644 --- a/src/afs/afs_buffer.c +++ b/src/afs/afs_buffer.c @@ -557,7 +557,7 @@ DNew(struct dcache *adc, int page) * DFlush due to lock hierarchy issues */ if ((page + 1) * AFS_BUFFER_PAGESIZE > adc->f.chunkBytes) { afs_AdjustSize(adc, (page + 1) * AFS_BUFFER_PAGESIZE); - afs_WriteDCache(adc, 1); + osi_Assert(afs_WriteDCache(adc, 1) == 0); } ObtainWriteLock(&tb->lock, 265); tb->lockers++; diff --git a/src/afs/afs_daemons.c b/src/afs/afs_daemons.c index 36140b3..5dfef24 100644 --- a/src/afs/afs_daemons.c +++ b/src/afs/afs_daemons.c @@ -1176,7 +1176,7 @@ afs_sgidaemon(void) SPUNLOCK(afs_sgibklock, s); AFS_GLOCK(); tdc->dflags &= ~DFEntryMod; - afs_WriteDCache(tdc, 1); + osi_Assert(afs_WriteDCache(tdc, 1) == 0); AFS_GUNLOCK(); s = SPLOCK(afs_sgibklock); } diff --git a/src/afs/afs_dcache.c b/src/afs/afs_dcache.c index 0612260..02a83eb 100644 --- a/src/afs/afs_dcache.c +++ b/src/afs/afs_dcache.c @@ -674,13 +674,15 @@ afs_GetDownD(int anumber, int *aneedSpace, afs_int32 buckethint) for (i = 0; i < victimPtr; i++) { tdc = afs_GetValidDSlot(victims[i]); /* We got tdc->tlock(R) here */ - if (tdc->refCount == 1) + if (tdc && tdc->refCount == 1) victimDCs[i] = tdc; else victimDCs[i] = 0; - ReleaseReadLock(&tdc->tlock); - if (!victimDCs[i]) - afs_PutDCache(tdc); + if (tdc) { + ReleaseReadLock(&tdc->tlock); + if (!victimDCs[i]) + afs_PutDCache(tdc); + } } for (i = 0; i < victimPtr; i++) { /* q is first elt in dcache entry */ @@ -1201,7 +1203,7 @@ afs_GetDownDSlot(int anumber) } #else tdc->dflags &= ~DFEntryMod; - afs_WriteDCache(tdc, 1); + osi_Assert(afs_WriteDCache(tdc, 1) == 0); #endif } @@ -1312,6 +1314,7 @@ afs_TryToSmush(struct vcache *avc, afs_ucred_t *acred, int sync) if (afs_indexUnique[index] == avc->f.fid.Fid.Unique) { int releaseTlock = 1; tdc = afs_GetValidDSlot(index); + if (!tdc) osi_Panic("afs_TryToSmush tdc"); if (!FidCmp(&tdc->f.fid, &avc->f.fid)) { if (sync) { if ((afs_indexFlags[index] & IFDataMod) == 0 @@ -1401,11 +1404,13 @@ afs_DCacheMissingChunks(struct vcache *avc) i = afs_dvnextTbl[index]; if (afs_indexUnique[index] == avc->f.fid.Fid.Unique) { tdc = afs_GetValidDSlot(index); - if (!FidCmp(&tdc->f.fid, &avc->f.fid)) { - totalChunks--; - } - ReleaseReadLock(&tdc->tlock); - afs_PutDCache(tdc); + if (tdc) { + if (!FidCmp(&tdc->f.fid, &avc->f.fid)) { + totalChunks--; + } + ReleaseReadLock(&tdc->tlock); + afs_PutDCache(tdc); + } } } ReleaseWriteLock(&afs_xdcache); @@ -1454,6 +1459,7 @@ afs_FindDCache(struct vcache *avc, afs_size_t abyte) for (index = afs_dchashTbl[i]; index != NULLIDX;) { if (afs_indexUnique[index] == avc->f.fid.Fid.Unique) { tdc = afs_GetValidDSlot(index); + if (!tdc) osi_Panic("afs_FindDCache tdc"); ReleaseReadLock(&tdc->tlock); if (!FidCmp(&tdc->f.fid, &avc->f.fid) && chunk == tdc->f.chunk) { break; /* leaving refCount high for caller */ @@ -1770,6 +1776,10 @@ afs_GetDCache(struct vcache *avc, afs_size_t abyte, for (index = afs_dchashTbl[i]; index != NULLIDX;) { if (afs_indexUnique[index] == avc->f.fid.Fid.Unique) { tdc = afs_GetValidDSlot(index); + if (!tdc) { + ReleaseWriteLock(&afs_xdcache); + goto done; + } ReleaseReadLock(&tdc->tlock); /* * Locks held: @@ -2523,7 +2533,7 @@ afs_WriteThroughDSlots(void) if (wrLock && (tdc->dflags & DFEntryMod)) { tdc->dflags &= ~DFEntryMod; ObtainWriteLock(&afs_xdcache, 620); - afs_WriteDCache(tdc, 1); + osi_Assert(afs_WriteDCache(tdc, 1) == 0); ReleaseWriteLock(&afs_xdcache); touchedit = 1; } @@ -3002,7 +3012,7 @@ afs_InitCacheFile(char *afile, ino_t ainode) tdc->f.states &= ~DWriting; tdc->dflags &= ~DFEntryMod; /* don't set f.modTime; we're just cleaning up */ - afs_WriteDCache(tdc, 0); + osi_Assert(afs_WriteDCache(tdc, 0) == 0); ReleaseWriteLock(&afs_xdcache); ReleaseWriteLock(&tdc->lock); afs_PutDCache(tdc); diff --git a/src/afs/afs_disconnected.c b/src/afs/afs_disconnected.c index fddf2ac..a587538 100644 --- a/src/afs/afs_disconnected.c +++ b/src/afs/afs_disconnected.c @@ -72,11 +72,13 @@ afs_FindDCacheByFid(struct VenusFid *afid) for (index = afs_dvhashTbl[i]; index != NULLIDX;) { if (afs_indexUnique[index] == afid->Fid.Unique) { tdc = afs_GetValidDSlot(index); - ReleaseReadLock(&tdc->tlock); - if (!FidCmp(&tdc->f.fid, afid)) { - break; /* leaving refCount high for caller */ + if (tdc) { + ReleaseReadLock(&tdc->tlock); + if (!FidCmp(&tdc->f.fid, afid)) { + break; /* leaving refCount high for caller */ + } + afs_PutDCache(tdc); } - afs_PutDCache(tdc); } index = afs_dvnextTbl[index]; } diff --git a/src/afs/afs_pioctl.c b/src/afs/afs_pioctl.c index 90cfcdd..b0b1b98 100644 --- a/src/afs/afs_pioctl.c +++ b/src/afs/afs_pioctl.c @@ -3484,6 +3484,9 @@ DECL_PIOCTL(PFlushVolumeData) if (!(afs_indexFlags[i] & IFEverUsed)) continue; /* never had any data */ tdc = afs_GetValidDSlot(i); + if (!tdc) { + continue; + } if (tdc->refCount <= 1) { /* too high, in use by running sys call */ ReleaseReadLock(&tdc->tlock); if (tdc->f.fid.Fid.Volume == volume && tdc->f.fid.Cell == cell) { diff --git a/src/afs/afs_segments.c b/src/afs/afs_segments.c index 48fdf18..c9308ed 100644 --- a/src/afs/afs_segments.c +++ b/src/afs/afs_segments.c @@ -255,6 +255,11 @@ afs_StoreAllSegments(struct vcache *avc, struct vrequest *areq, if ((afs_indexFlags[index] & IFDataMod) && (afs_indexUnique[index] == avc->f.fid.Fid.Unique)) { tdc = afs_GetValidDSlot(index); /* refcount+1. */ + if (!tdc) { + ReleaseWriteLock(&afs_xdcache); + code = EIO; + goto done; + } ReleaseReadLock(&tdc->tlock); if (!FidCmp(&tdc->f.fid, &avc->f.fid) && tdc->f.chunk >= minj) { off = tdc->f.chunk - minj; @@ -317,6 +322,7 @@ afs_StoreAllSegments(struct vcache *avc, struct vrequest *areq, minj += NCHUNKSATONCE; } while (!code && moredata); + done: UpgradeSToWLock(&avc->lock, 29); /* send a trivial truncation store if did nothing else */ @@ -359,6 +365,7 @@ afs_StoreAllSegments(struct vcache *avc, struct vrequest *areq, if (afs_indexUnique[index] == avc->f.fid.Fid.Unique) { tdc = afs_GetValidDSlot(index); + if (!tdc) osi_Panic("afs_StoreAllSegments tdc dv"); ReleaseReadLock(&tdc->tlock); if (!FidCmp(&tdc->f.fid, &avc->f.fid) @@ -523,6 +530,7 @@ afs_InvalidateAllSegments(struct vcache *avc) for (index = afs_dvhashTbl[hash]; index != NULLIDX;) { if (afs_indexUnique[index] == avc->f.fid.Fid.Unique) { tdc = afs_GetValidDSlot(index); + if (!tdc) osi_Panic("afs_InvalidateAllSegments tdc count"); ReleaseReadLock(&tdc->tlock); if (!FidCmp(&tdc->f.fid, &avc->f.fid)) dcListMax++; @@ -537,6 +545,7 @@ afs_InvalidateAllSegments(struct vcache *avc) for (index = afs_dvhashTbl[hash]; index != NULLIDX;) { if (afs_indexUnique[index] == avc->f.fid.Fid.Unique) { tdc = afs_GetValidDSlot(index); + if (!tdc) osi_Panic("afs_InvalidateAllSegments tdc store"); ReleaseReadLock(&tdc->tlock); if (!FidCmp(&tdc->f.fid, &avc->f.fid)) { /* same file? we'll zap it */ @@ -715,6 +724,11 @@ afs_TruncateAllSegments(struct vcache *avc, afs_size_t alen, for (index = afs_dvhashTbl[code]; index != NULLIDX;) { if (afs_indexUnique[index] == avc->f.fid.Fid.Unique) { tdc = afs_GetValidDSlot(index); + if (!tdc) { + ReleaseWriteLock(&afs_xdcache); + code = EIO; + goto done; + } ReleaseReadLock(&tdc->tlock); if (!FidCmp(&tdc->f.fid, &avc->f.fid)) dcCount++; @@ -733,6 +747,7 @@ afs_TruncateAllSegments(struct vcache *avc, afs_size_t alen, for (index = afs_dvhashTbl[code]; index != NULLIDX;) { if (afs_indexUnique[index] == avc->f.fid.Fid.Unique) { tdc = afs_GetValidDSlot(index); + if (!tdc) osi_Panic("afs_TruncateAllSegments tdc"); ReleaseReadLock(&tdc->tlock); if (!FidCmp(&tdc->f.fid, &avc->f.fid)) { /* same file, and modified, we'll store it back */ @@ -780,6 +795,9 @@ afs_TruncateAllSegments(struct vcache *avc, afs_size_t alen, osi_Free(tdcArray, dcCount * sizeof(struct dcache *)); + code = 0; + + done: #if (defined(AFS_SUN5_ENV)) ObtainWriteLock(&avc->vlock, 547); if (--avc->activeV == 0 && (avc->vstates & VRevokeWait)) { @@ -789,5 +807,5 @@ afs_TruncateAllSegments(struct vcache *avc, afs_size_t alen, ReleaseWriteLock(&avc->vlock); #endif - return 0; + return code; } -- 1.9.4