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.

Reviewed-on: http://gerrit.openafs.org/6417
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
(cherry picked from commit 1a672914ab050811c99b6307c657630ab9b5c8ee)

Change-Id: Ia30e7de919ccc14708112886e492e60f93fe6639
Reviewed-on: http://gerrit.openafs.org/7940
Reviewed-by: Derrick Brashear <shadow@dementix.org>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

diff --git a/src/afs/afs_chunkops.h b/src/afs/afs_chunkops.h
index ec619af9b93db18a08b861b158469eef0fad8579..2aa19c1a75fea15f97a8870df92d5b4079e5a8b6 100644 (file)
--- a/src/afs/afs_chunkops.h
+++ b/src/afs/afs_chunkops.h
@@ -63,7 +63,7 @@ struct afs_cacheOps {
                  int noLock);
     int (*vwrite) (struct vcache * avc, struct uio * auio, int aio,
                   afs_ucred_t * acred, int noLock);
-    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);
 };
@@ -74,7 +74,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 9a91127ce4860a4d42ae281e9097cbe7638b0da4..0612260afcfed1e73576be9b5eb629bac9bb7c7e 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 9907786ffd94cb70b35a0cab340b90c069922015..fddf2aca09d962db23ab6fe66175db29a72eaa7e 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 1aa152c7bf20163d036a20a33b07fa6c40c8e92c..90cfcddfe15da15774dd93ba8e4dab0a9651f23d 100644 (file)
--- a/src/afs/afs_pioctl.c
+++ b/src/afs/afs_pioctl.c
@@ -3483,7 +3483,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 f73ae3291424a3548b7ab759ae19aa4334f3c061..9266c336e0c0aa6cb4978eb304923d54601efed3 100644 (file)
--- a/src/afs/afs_prototypes.h
+++ b/src/afs/afs_prototypes.h
@@ -258,7 +258,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,
@@ -280,7 +280,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 da5960715f4efb37a03f54ea1640a8070fb5fca4..48fdf18d6047d47476b1522d701999039be22e88 100644 (file)
--- a/src/afs/afs_segments.c
+++ b/src/afs/afs_segments.c
@@ -254,7 +254,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;
@@ -358,7 +358,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)
@@ -522,7 +522,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++;
@@ -536,7 +536,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 */
@@ -714,7 +714,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++;
@@ -732,7 +732,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 */