From 1a672914ab050811c99b6307c657630ab9b5c8ee Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Wed, 21 Dec 2011 15:04:32 -0500 Subject: [PATCH] afs: Do not always ignore errors in afs_GetDSlot Currently afs_UFSGetDSlot will silently swallow any error in reading the specified dslot from disk, and will return a "blank" dcache to the caller. However, many callers of afs_GetDSlot will be asking for a dcache that we know exists, and more importantly, we know is on the global hash table. If a disk error is encountered and we're given a "blank" dcache, we will erroneously believe the dcache entry is not on the hash table, causing corruption of the hash table later on. So instead, modify all callers of afs_GetDSlot to use either afs_GetValidDSlot or afs_GetNewDSlot. Calling afs_GetValidDSlot indicates that the given dentry index is known to be valid, and any error encountered while reading the entry from disk should result in an error (for disk I/O errors we have no control over, this results in a NULL dentry returned; for internal consistency errors we panic). Calling afs_GetNewDSlot indicates that the specified index may not exist or may not be valid, and so returning a "blank" dentry in that case is fine. For memcache, the situation is the same, except any time we go to "disk" it is an (internal) error, since there is no disk. Change-Id: I53ea6e99649e4d6d5cbde58929dfcee1d45a3e7b Reviewed-on: http://gerrit.openafs.org/6417 Tested-by: BuildBot Reviewed-by: Derrick Brashear --- src/afs/afs_chunkops.h | 5 ++-- src/afs/afs_dcache.c | 72 +++++++++++++++++++++++++++++++++------------- src/afs/afs_disconnected.c | 4 +-- src/afs/afs_pioctl.c | 2 +- src/afs/afs_prototypes.h | 4 +-- src/afs/afs_segments.c | 12 ++++---- 6 files changed, 66 insertions(+), 33 deletions(-) diff --git a/src/afs/afs_chunkops.h b/src/afs/afs_chunkops.h index df6b4e6..1a4d38b 100644 --- a/src/afs/afs_chunkops.h +++ b/src/afs/afs_chunkops.h @@ -60,7 +60,7 @@ struct afs_cacheOps { int (*close) (struct osi_file * fp); int (*vreadUIO) (afs_dcache_id_t *, struct uio *); int (*vwriteUIO) (struct vcache *, afs_dcache_id_t *, struct uio *); - struct dcache *(*GetDSlot) (afs_int32 aslot); + struct dcache *(*GetDSlot) (afs_int32 aslot, int needvalid); struct volume *(*GetVolSlot) (void); int (*HandleLink) (struct vcache * avc, struct vrequest * areq); }; @@ -71,7 +71,8 @@ struct afs_cacheOps { #define afs_CFileRead(file, offset, data, size) (*(afs_cacheType->fread))(file, offset, data, size) #define afs_CFileWrite(file, offset, data, size) (*(afs_cacheType->fwrite))(file, offset, data, size) #define afs_CFileClose(handle) (*(afs_cacheType->close))(handle) -#define afs_GetDSlot(slot) (*(afs_cacheType->GetDSlot))(slot) +#define afs_GetValidDSlot(slot) (*(afs_cacheType->GetDSlot))(slot, 1) +#define afs_GetNewDSlot(slot) (*(afs_cacheType->GetDSlot))(slot, 0) #define afs_GetVolSlot() (*(afs_cacheType->GetVolSlot))() #define afs_HandleLink(avc, areq) (*(afs_cacheType->HandleLink))(avc, areq) diff --git a/src/afs/afs_dcache.c b/src/afs/afs_dcache.c index 6dac860..420b92e 100644 --- a/src/afs/afs_dcache.c +++ b/src/afs/afs_dcache.c @@ -672,7 +672,7 @@ afs_GetDownD(int anumber, int *aneedSpace, afs_int32 buckethint) * during the truncate operation. */ for (i = 0; i < victimPtr; i++) { - tdc = afs_GetDSlot(victims[i]); + tdc = afs_GetValidDSlot(victims[i]); /* We got tdc->tlock(R) here */ if (tdc->refCount == 1) victimDCs[i] = tdc; @@ -689,7 +689,7 @@ afs_GetDownD(int anumber, int *aneedSpace, afs_int32 buckethint) * have to verify, before proceeding, that there are no other * references to this dcache entry, even now. Note that we * compare with 1, since we bumped it above when we called - * afs_GetDSlot to preserve the entry's identity. + * afs_GetValidDSlot to preserve the entry's identity. */ if (tdc && tdc->refCount == 1) { unsigned char chunkFlags; @@ -1085,7 +1085,7 @@ afs_FreeDiscardedDCache(void) /* * Get an entry from the list of discarded cache elements */ - tdc = afs_GetDSlot(afs_discardDCList); + tdc = afs_GetNewDSlot(afs_discardDCList); osi_Assert(tdc->refCount == 1); ReleaseReadLock(&tdc->tlock); @@ -1311,7 +1311,7 @@ afs_TryToSmush(struct vcache *avc, afs_ucred_t *acred, int sync) i = afs_dvnextTbl[index]; /* next pointer this hash table */ if (afs_indexUnique[index] == avc->f.fid.Fid.Unique) { int releaseTlock = 1; - tdc = afs_GetDSlot(index); + tdc = afs_GetValidDSlot(index); if (!FidCmp(&tdc->f.fid, &avc->f.fid)) { if (sync) { if ((afs_indexFlags[index] & IFDataMod) == 0 @@ -1400,7 +1400,7 @@ afs_DCacheMissingChunks(struct vcache *avc) for (index = afs_dvhashTbl[i]; index != NULLIDX; index = i) { i = afs_dvnextTbl[index]; if (afs_indexUnique[index] == avc->f.fid.Fid.Unique) { - tdc = afs_GetDSlot(index); + tdc = afs_GetValidDSlot(index); if (!FidCmp(&tdc->f.fid, &avc->f.fid)) { totalChunks--; } @@ -1453,7 +1453,7 @@ afs_FindDCache(struct vcache *avc, afs_size_t abyte) ObtainWriteLock(&afs_xdcache, 278); for (index = afs_dchashTbl[i]; index != NULLIDX;) { if (afs_indexUnique[index] == avc->f.fid.Fid.Unique) { - tdc = afs_GetDSlot(index); + tdc = afs_GetValidDSlot(index); ReleaseReadLock(&tdc->tlock); if (!FidCmp(&tdc->f.fid, &avc->f.fid) && chunk == tdc->f.chunk) { break; /* leaving refCount high for caller */ @@ -1501,7 +1501,7 @@ afs_AllocDCache(struct vcache *avc, afs_int32 chunk, afs_int32 lock, || ((lock & 2) && afs_freeDCList != NULLIDX)) { afs_indexFlags[afs_freeDCList] &= ~IFFree; - tdc = afs_GetDSlot(afs_freeDCList); + tdc = afs_GetNewDSlot(afs_freeDCList); osi_Assert(tdc->refCount == 1); ReleaseReadLock(&tdc->tlock); ObtainWriteLock(&tdc->lock, 604); @@ -1509,7 +1509,7 @@ afs_AllocDCache(struct vcache *avc, afs_int32 chunk, afs_int32 lock, afs_freeDCCount--; } else { afs_indexFlags[afs_discardDCList] &= ~IFDiscarded; - tdc = afs_GetDSlot(afs_discardDCList); + tdc = afs_GetNewDSlot(afs_discardDCList); osi_Assert(tdc->refCount == 1); ReleaseReadLock(&tdc->tlock); ObtainWriteLock(&tdc->lock, 605); @@ -1769,7 +1769,7 @@ afs_GetDCache(struct vcache *avc, afs_size_t abyte, us = NULLIDX; for (index = afs_dchashTbl[i]; index != NULLIDX;) { if (afs_indexUnique[index] == avc->f.fid.Fid.Unique) { - tdc = afs_GetDSlot(index); + tdc = afs_GetValidDSlot(index); ReleaseReadLock(&tdc->tlock); /* * Locks held: @@ -2567,7 +2567,7 @@ afs_WriteThroughDSlots(void) */ struct dcache * -afs_MemGetDSlot(afs_int32 aslot) +afs_MemGetDSlot(afs_int32 aslot, int needvalid) { struct dcache *tdc; int existing = 0; @@ -2587,6 +2587,9 @@ afs_MemGetDSlot(afs_int32 aslot) ConvertWToRLock(&tdc->tlock); return tdc; } + + osi_Assert(!needvalid); + if (!afs_freeDSList) afs_GetDownDSlot(4); if (!afs_freeDSList) { @@ -2651,12 +2654,13 @@ unsigned int last_error = 0, lasterrtime = 0; * afs_xdcache lock write-locked. */ struct dcache * -afs_UFSGetDSlot(afs_int32 aslot) +afs_UFSGetDSlot(afs_int32 aslot, int needvalid) { afs_int32 code; struct dcache *tdc; int existing = 0; int entryok; + int off; AFS_STATCNT(afs_UFSGetDSlot); if (CheckLock(&afs_xdcache) != -1) @@ -2699,16 +2703,48 @@ afs_UFSGetDSlot(afs_int32 aslot) /* * Seek to the aslot'th entry and read it in. */ + off = sizeof(struct fcache)*aslot + sizeof(struct afs_fheader); code = afs_osi_Read(afs_cacheInodep, - sizeof(struct fcache) * aslot + - sizeof(struct afs_fheader), (char *)(&tdc->f), + off, (char *)(&tdc->f), sizeof(struct fcache)); entryok = 1; - if (code != sizeof(struct fcache)) + if (code != sizeof(struct fcache)) { entryok = 0; - if (!afs_CellNumValid(tdc->f.fid.Cell)) +#if defined(KERNEL_HAVE_UERROR) + last_error = getuerror(); +#endif + lasterrtime = osi_Time(); + if (needvalid) { + struct osi_stat tstat; + if (afs_osi_Stat(afs_cacheInodep, &tstat)) { + tstat.size = -1; + } + afs_warn("afs: disk cache read error in CacheItems off %d/%d " + "code %d/%d\n", + off, (int)tstat.size, + (int)code, (int)sizeof(struct fcache)); + /* put tdc back on the free dslot list */ + QRemove(&tdc->lruq); + tdc->index = NULLIDX; + tdc->lruq.next = (struct afs_q *)afs_freeDSList; + afs_freeDSList = tdc; + return NULL; + } + } + if (!afs_CellNumValid(tdc->f.fid.Cell)) { entryok = 0; + if (needvalid) { + osi_Panic("afs: needed valid dcache but index %d off %d has " + "invalid cell num %d\n", + (int)aslot, off, (int)tdc->f.fid.Cell); + } + } + + if (needvalid && tdc->f.fid.Fid.Volume == 0) { + osi_Panic("afs: invalid zero-volume dcache entry at slot %d off %d", + (int)aslot, off); + } if (!entryok) { tdc->f.fid.Cell = 0; @@ -2716,10 +2752,6 @@ afs_UFSGetDSlot(afs_int32 aslot) tdc->f.chunk = -1; hones(tdc->f.versionNo); tdc->dflags |= DFEntryMod; -#if defined(KERNEL_HAVE_UERROR) - last_error = getuerror(); -#endif - lasterrtime = osi_Time(); afs_indexUnique[aslot] = tdc->f.fid.Fid.Unique; tdc->f.states &= ~(DRO|DBackup|DRW); afs_DCMoveBucket(tdc, 0, 0); @@ -2874,7 +2906,7 @@ afs_InitCacheFile(char *afile, ino_t ainode) return EINVAL; ObtainWriteLock(&afs_xdcache, 282); - tdc = afs_GetDSlot(index); + tdc = afs_GetNewDSlot(index); ReleaseReadLock(&tdc->tlock); ReleaseWriteLock(&afs_xdcache); diff --git a/src/afs/afs_disconnected.c b/src/afs/afs_disconnected.c index 223b838..272bc4f 100644 --- a/src/afs/afs_disconnected.c +++ b/src/afs/afs_disconnected.c @@ -71,7 +71,7 @@ afs_FindDCacheByFid(struct VenusFid *afid) ObtainWriteLock(&afs_xdcache, 758); for (index = afs_dvhashTbl[i]; index != NULLIDX;) { if (afs_indexUnique[index] == afid->Fid.Unique) { - tdc = afs_GetDSlot(index); + tdc = afs_GetValidDSlot(index); ReleaseReadLock(&tdc->tlock); if (!FidCmp(&tdc->f.fid, afid)) { break; /* leaving refCount high for caller */ @@ -853,7 +853,7 @@ afs_ProcessOpCreate(struct vcache *avc, struct vrequest *areq, ObtainWriteLock(&afs_xdcache, 743); for (index = afs_dvhashTbl[hash]; index != NULLIDX; index = hash) { hash = afs_dvnextTbl[index]; - tdc = afs_GetDSlot(index); + tdc = afs_GetValidDSlot(index); ReleaseReadLock(&tdc->tlock); if (afs_indexUnique[index] == avc->f.fid.Fid.Unique) { if (!FidCmp(&tdc->f.fid, &avc->f.fid)) { diff --git a/src/afs/afs_pioctl.c b/src/afs/afs_pioctl.c index 200a00e..83f159c 100644 --- a/src/afs/afs_pioctl.c +++ b/src/afs/afs_pioctl.c @@ -3546,7 +3546,7 @@ DECL_PIOCTL(PFlushVolumeData) for (i = 0; i < afs_cacheFiles; i++) { if (!(afs_indexFlags[i] & IFEverUsed)) continue; /* never had any data */ - tdc = afs_GetDSlot(i); + tdc = afs_GetValidDSlot(i); 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_prototypes.h b/src/afs/afs_prototypes.h index eb2e080..940df15 100644 --- a/src/afs/afs_prototypes.h +++ b/src/afs/afs_prototypes.h @@ -250,7 +250,7 @@ extern void afs_FlushDCache(struct dcache *adc); extern void shutdown_dcache(void); extern void afs_CacheTruncateDaemon(void); extern afs_int32 afs_fsfragsize; -extern struct dcache *afs_MemGetDSlot(afs_int32 aslot); +extern struct dcache *afs_MemGetDSlot(afs_int32 aslot, int needvalid); extern struct dcache *afs_GetDCache(struct vcache *avc, afs_size_t abyte, struct vrequest *areq, @@ -272,7 +272,7 @@ extern void afs_TryToSmush(struct vcache *avc, extern void updateV2DC(int lockVc, struct vcache *v, struct dcache *d, int src); extern void afs_WriteThroughDSlots(void); -extern struct dcache *afs_UFSGetDSlot(afs_int32 aslot); +extern struct dcache *afs_UFSGetDSlot(afs_int32 aslot, int needvalid); extern int afs_WriteDCache(struct dcache *adc, int atime); extern int afs_wakeup(struct vcache *avc); extern int afs_InitCacheFile(char *afile, ino_t ainode); diff --git a/src/afs/afs_segments.c b/src/afs/afs_segments.c index 515c363..8ae6680 100644 --- a/src/afs/afs_segments.c +++ b/src/afs/afs_segments.c @@ -252,7 +252,7 @@ afs_StoreAllSegments(struct vcache *avc, struct vrequest *areq, for (j = 0; index != NULLIDX;) { if ((afs_indexFlags[index] & IFDataMod) && (afs_indexUnique[index] == avc->f.fid.Fid.Unique)) { - tdc = afs_GetDSlot(index); /* refcount+1. */ + tdc = afs_GetValidDSlot(index); /* refcount+1. */ ReleaseReadLock(&tdc->tlock); if (!FidCmp(&tdc->f.fid, &avc->f.fid) && tdc->f.chunk >= minj) { off = tdc->f.chunk - minj; @@ -356,7 +356,7 @@ afs_StoreAllSegments(struct vcache *avc, struct vrequest *areq, index != NULLIDX && safety < afs_cacheFiles + 2;) { if (afs_indexUnique[index] == avc->f.fid.Fid.Unique) { - tdc = afs_GetDSlot(index); + tdc = afs_GetValidDSlot(index); ReleaseReadLock(&tdc->tlock); if (!FidCmp(&tdc->f.fid, &avc->f.fid) @@ -520,7 +520,7 @@ afs_InvalidateAllSegments(struct vcache *avc) for (index = afs_dvhashTbl[hash]; index != NULLIDX;) { if (afs_indexUnique[index] == avc->f.fid.Fid.Unique) { - tdc = afs_GetDSlot(index); + tdc = afs_GetValidDSlot(index); ReleaseReadLock(&tdc->tlock); if (!FidCmp(&tdc->f.fid, &avc->f.fid)) dcListMax++; @@ -534,7 +534,7 @@ afs_InvalidateAllSegments(struct vcache *avc) for (index = afs_dvhashTbl[hash]; index != NULLIDX;) { if (afs_indexUnique[index] == avc->f.fid.Fid.Unique) { - tdc = afs_GetDSlot(index); + tdc = afs_GetValidDSlot(index); ReleaseReadLock(&tdc->tlock); if (!FidCmp(&tdc->f.fid, &avc->f.fid)) { /* same file? we'll zap it */ @@ -712,7 +712,7 @@ afs_TruncateAllSegments(struct vcache *avc, afs_size_t alen, dcCount = 0; for (index = afs_dvhashTbl[code]; index != NULLIDX;) { if (afs_indexUnique[index] == avc->f.fid.Fid.Unique) { - tdc = afs_GetDSlot(index); + tdc = afs_GetValidDSlot(index); ReleaseReadLock(&tdc->tlock); if (!FidCmp(&tdc->f.fid, &avc->f.fid)) dcCount++; @@ -730,7 +730,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_GetDSlot(index); + tdc = afs_GetValidDSlot(index); ReleaseReadLock(&tdc->tlock); if (!FidCmp(&tdc->f.fid, &avc->f.fid)) { /* same file, and modified, we'll store it back */ -- 1.9.4