From 00fd34a65c7914bfd480d892cc95e9f694e29f5f Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Thu, 1 Nov 2012 11:51:42 -0500 Subject: [PATCH] afs: Handle easy GetValidDSlot errors Many callers of GetValidDSlot currently assume they will always get back a valid dcache, and will panic on getting NULL. However, for many of these callers, handling the NULL case is quite easy, since the failure to get a dcache can just result in an error directly, or obtaining the dcache is best-effort or just an optimization. This commit just handles the "easy" cases; some other callers require more complex handling. Reviewed-on: http://gerrit.openafs.org/8375 Tested-by: BuildBot Reviewed-by: Derrick Brashear (cherry picked from commit f74a0a7bbb37a8ab6050e833cf8d66abdff31854) Change-Id: Ic2c463edebcb821562541004bd4181483c90c3e6 Reviewed-on: http://gerrit.openafs.org/9287 Reviewed-by: Andrew Deason Reviewed-by: Derrick Brashear Tested-by: BuildBot Reviewed-by: Stephan Wiesand --- src/afs/afs_dcache.c | 40 +++++++++++++++++++++++++++++++--------- src/afs/afs_segments.c | 40 ++++++++++++++++++++++++++++++---------- 2 files changed, 61 insertions(+), 19 deletions(-) diff --git a/src/afs/afs_dcache.c b/src/afs/afs_dcache.c index cd125408e..8f3d5d627 100644 --- a/src/afs/afs_dcache.c +++ b/src/afs/afs_dcache.c @@ -1315,7 +1315,12 @@ 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 (!tdc) { + /* afs_TryToSmush is best-effort; we may not actually discard + * everything, so failure to discard a dcache due to an i/o + * error is okay. */ + continue; + } if (!FidCmp(&tdc->f.fid, &avc->f.fid)) { if (sync) { if ((afs_indexFlags[index] & IFDataMod) == 0 @@ -1457,17 +1462,21 @@ afs_FindDCache(struct vcache *avc, afs_size_t abyte) */ i = DCHash(&avc->f.fid, chunk); ObtainWriteLock(&afs_xdcache, 278); - for (index = afs_dchashTbl[i]; index != NULLIDX;) { + for (index = afs_dchashTbl[i]; index != NULLIDX; index = afs_dcnextTbl[index]) { if (afs_indexUnique[index] == avc->f.fid.Fid.Unique) { tdc = afs_GetValidDSlot(index); - if (!tdc) osi_Panic("afs_FindDCache tdc"); + if (!tdc) { + /* afs_FindDCache is best-effort; we may not find the given + * file/offset, so if we cannot find the given dcache due to + * i/o errors, that is okay. */ + continue; + } ReleaseReadLock(&tdc->tlock); if (!FidCmp(&tdc->f.fid, &avc->f.fid) && chunk == tdc->f.chunk) { break; /* leaving refCount high for caller */ } afs_PutDCache(tdc); } - index = afs_dcnextTbl[index]; } if (index != NULLIDX) { hset(afs_indexTimes[tdc->index], afs_indexCounter); @@ -1758,6 +1767,7 @@ afs_GetDCache(struct vcache *avc, afs_size_t abyte, */ if (!tdc) { /* If the hint wasn't the right dcache entry */ + int dslot_error = 0; /* * Hash on the [fid, chunk] and get the corresponding dcache index * after write-locking the dcache. @@ -1775,12 +1785,16 @@ afs_GetDCache(struct vcache *avc, afs_size_t abyte, ObtainWriteLock(&afs_xdcache, 280); us = NULLIDX; - for (index = afs_dchashTbl[i]; index != NULLIDX;) { + for (index = afs_dchashTbl[i]; index != NULLIDX; us = index, index = afs_dcnextTbl[index]) { if (afs_indexUnique[index] == avc->f.fid.Fid.Unique) { tdc = afs_GetValidDSlot(index); if (!tdc) { - ReleaseWriteLock(&afs_xdcache); - goto done; + /* we got an i/o error when trying to get the given dslot, + * but do not bail out just yet; it is possible the dcache + * we're looking for is elsewhere, so it doesn't matter if + * we can't load this one. */ + dslot_error = 1; + continue; } ReleaseReadLock(&tdc->tlock); /* @@ -1803,8 +1817,6 @@ afs_GetDCache(struct vcache *avc, afs_size_t abyte, afs_PutDCache(tdc); tdc = 0; } - us = index; - index = afs_dcnextTbl[index]; } /* @@ -1820,6 +1832,16 @@ afs_GetDCache(struct vcache *avc, afs_size_t abyte, afs_Trace2(afs_iclSetp, CM_TRACE_GETDCACHE1, ICL_TYPE_POINTER, avc, ICL_TYPE_INT32, chunk); + if (dslot_error) { + /* We couldn't find the dcache we want, but we hit some i/o + * errors when trying to find it, so we're not sure if the + * dcache we want is in the cache or not. Error out, so we + * don't try to possibly create 2 separate dcaches for the + * same exact data. */ + ReleaseWriteLock(&afs_xdcache); + goto done; + } + /* Make sure there is a free dcache entry for us to use */ if (afs_discardDCList == NULLIDX && afs_freeDCList == NULLIDX) { while (1) { diff --git a/src/afs/afs_segments.c b/src/afs/afs_segments.c index 192a3d4b4..631cb7c0f 100644 --- a/src/afs/afs_segments.c +++ b/src/afs/afs_segments.c @@ -359,11 +359,22 @@ afs_StoreAllSegments(struct vcache *avc, struct vrequest *areq, ObtainWriteLock(&afs_xdcache, 285); for (j = 0, safety = 0, index = afs_dvhashTbl[hash]; - index != NULLIDX && safety < afs_cacheFiles + 2;) { + index != NULLIDX && safety < afs_cacheFiles + 2; + index = afs_dvnextTbl[index]) { if (afs_indexUnique[index] == avc->f.fid.Fid.Unique) { tdc = afs_GetValidDSlot(index); - if (!tdc) osi_Panic("afs_StoreAllSegments tdc dv"); + if (!tdc) { + /* This is okay; since manipulating the dcaches at this + * point is best-effort. We only get a dcache here to + * increment the dv and turn off DWriting. If we were + * supposed to do that for a dcache, but could not + * due to an I/O error, it just means the dv won't + * be updated so we don't be able to use that cached + * chunk in the future. That's inefficient, but not + * an error. */ + continue; + } ReleaseReadLock(&tdc->tlock); if (!FidCmp(&tdc->f.fid, &avc->f.fid) @@ -385,8 +396,6 @@ afs_StoreAllSegments(struct vcache *avc, struct vrequest *areq, afs_PutDCache(tdc); } } - - index = afs_dvnextTbl[index]; } ReleaseWriteLock(&afs_xdcache); @@ -669,7 +678,7 @@ afs_TruncateAllSegments(struct vcache *avc, afs_size_t alen, afs_size_t newSize; int dcCount, dcPos; - struct dcache **tdcArray; + struct dcache **tdcArray = NULL; AFS_STATCNT(afs_TruncateAllSegments); avc->f.m.Date = osi_Time(); @@ -745,10 +754,21 @@ afs_TruncateAllSegments(struct vcache *avc, afs_size_t alen, tdcArray = osi_Alloc(dcCount * sizeof(struct dcache *)); dcPos = 0; - for (index = afs_dvhashTbl[code]; index != NULLIDX;) { + for (index = afs_dvhashTbl[code]; index != NULLIDX; index = afs_dvnextTbl[index]) { if (afs_indexUnique[index] == avc->f.fid.Fid.Unique) { tdc = afs_GetValidDSlot(index); - if (!tdc) osi_Panic("afs_TruncateAllSegments tdc"); + if (!tdc) { + /* make sure we put back all of the tdcArray members before + * bailing out */ + /* remember, the last valid tdc is at dcPos-1, so start at + * dcPos-1, not at dcPos itself. */ + for (dcPos = dcPos - 1; dcPos >= 0; dcPos--) { + tdc = tdcArray[dcPos]; + afs_PutDCache(tdc); + } + code = EIO; + goto done; + } ReleaseReadLock(&tdc->tlock); if (!FidCmp(&tdc->f.fid, &avc->f.fid)) { /* same file, and modified, we'll store it back */ @@ -761,7 +781,6 @@ afs_TruncateAllSegments(struct vcache *avc, afs_size_t alen, afs_PutDCache(tdc); } } - index = afs_dvnextTbl[index]; } ReleaseWriteLock(&afs_xdcache); @@ -795,11 +814,12 @@ afs_TruncateAllSegments(struct vcache *avc, afs_size_t alen, afs_PutDCache(tdc); } - osi_Free(tdcArray, dcCount * sizeof(struct dcache *)); - code = 0; done: + if (tdcArray) { + osi_Free(tdcArray, dcCount * sizeof(struct dcache *)); + } #if (defined(AFS_SUN5_ENV)) ObtainWriteLock(&avc->vlock, 547); if (--avc->activeV == 0 && (avc->vstates & VRevokeWait)) { -- 2.39.5