afs: Add some comments on GetValidDSlot panics
[openafs.git] / src / afs / afs_segments.c
index 47058f9..db75daa 100644 (file)
@@ -178,7 +178,7 @@ afs_StoreAllSegments(struct vcache *avc, struct vrequest *areq,
     hset(newDV, avc->f.m.DataVersion);
     hash = DVHash(&avc->f.fid);
     foreign = (avc->f.states & CForeign);
-    dcList = (struct dcache **)osi_AllocLargeSpace(AFS_LRALLOCSIZ);
+    dcList = osi_AllocLargeSpace(AFS_LRALLOCSIZ);
     afs_Trace2(afs_iclSetp, CM_TRACE_STOREALL, ICL_TYPE_POINTER, avc,
               ICL_TYPE_OFFSET, ICL_HANDLE_OFFSET(avc->f.m.Length));
 #if !defined(AFS_AIX32_ENV) && !defined(AFS_SGI65_ENV)
@@ -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);
 
@@ -418,6 +427,9 @@ afs_StoreAllSegments(struct vcache *avc, struct vrequest *areq,
                        UpgradeSToWLock(&tdc->lock, 678);
                        hset(tdc->f.versionNo, avc->f.m.DataVersion);
                        tdc->dflags |= DFEntryMod;
+                       /* DWriting may not have gotten cleared above, if all
+                        * we did was a StoreMini */
+                       tdc->f.states &= ~DWriting;
                        ConvertWToSLock(&tdc->lock);
                    }
                }
@@ -528,7 +540,19 @@ 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");
+           if (!tdc) {
+               /* In the case of fatal errors during stores, we MUST
+                * invalidate all of the relevant chunks. Otherwise, the chunks
+                * will be left with the 'new' data that was never successfully
+                * written to the server, but the DV in the dcache is still the
+                * old DV. So, we may indefintely serve serve applications data
+                * that is not actually in the file on the fileserver. If we
+                * cannot afs_GetValidDSlot the appropriate entries, currently
+                * there is no way to ensure the dcache is invalidated. So for
+                * now, to avoid risking serving bad data from the cache, panic
+                * instead. */
+               osi_Panic("afs_InvalidateAllSegments tdc count");
+           }
            ReleaseReadLock(&tdc->tlock);
            if (!FidCmp(&tdc->f.fid, &avc->f.fid))
                dcListMax++;
@@ -543,7 +567,13 @@ 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");
+           if (!tdc) {
+               /* We cannot proceed after getting this error; we risk serving
+                * incorrect data to applications. So panic instead. See the
+                * above comment next to the previous afs_GetValidDSlot call
+                * for details. */
+               osi_Panic("afs_InvalidateAllSegments tdc store");
+           }
            ReleaseReadLock(&tdc->tlock);
            if (!FidCmp(&tdc->f.fid, &avc->f.fid)) {
                /* same file? we'll zap it */
@@ -666,7 +696,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();
@@ -742,10 +772,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 */
@@ -758,7 +799,6 @@ afs_TruncateAllSegments(struct vcache *avc, afs_size_t alen,
                afs_PutDCache(tdc);
            }
        }
-       index = afs_dvnextTbl[index];
     }
 
     ReleaseWriteLock(&afs_xdcache);
@@ -775,6 +815,7 @@ afs_TruncateAllSegments(struct vcache *avc, afs_size_t alen,
        ObtainSharedLock(&tdc->lock, 672);
        if (newSize < tdc->f.chunkBytes && newSize < MAX_AFS_UINT32) {
            UpgradeSToWLock(&tdc->lock, 673);
+           tdc->f.states |= DWriting;
            tfile = afs_CFileOpen(&tdc->f.inode);
            afs_CFileTruncate(tfile, (afs_int32)newSize);
            afs_CFileClose(tfile);
@@ -791,11 +832,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)) {