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 <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>

diff --git a/src/afs/afs_chunkops.h b/src/afs/afs_chunkops.h
index df6b4e62cf6461744d80e51739965ad7cd1d9113..1a4d38b34acceb83e8c320d3efc9f5bab353709d 100644 (file)
--- 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 6dac8602421b53b0b8cd64b57cba538dfc62330a..420b92e792120ba5bec6e60f8529302842bed8b1 100644 (file)
--- 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 223b838509a711ea04b5c27459cf8e86368b90a7..272bc4fea692a75246120dd3cdb4738278241a3f 100644 (file)
--- 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 200a00ed062edb2a6dd59078488776b4da4c268a..83f159c95b907441bd000c6b751705f74221ed42 100644 (file)
--- 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 eb2e080ec4757246814ba09979acf019dfc76a16..940df15bb31f8d830ad78330d79935655e1f7d71 100644 (file)
--- 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 515c363545d77f18c452f18b5eeedaefe4efc050..8ae668006e9d6651a828119b3c0b1cc2425c5827 100644 (file)
--- 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 */