afs: avoid afs_GetDownDSlot panic on afs_WriteDCache failure 64/13364/4
authorMark Vitale <mvitale@sinenomine.net>
Mon, 29 Oct 2018 20:48:14 +0000 (16:48 -0400)
committerBenjamin Kaduk <kaduk@mit.edu>
Sat, 5 Jan 2019 23:52:38 +0000 (18:52 -0500)
If afs_GetDownDSlot() finds insuffcient free slots in the
afs_freeDSList, it will walk the afs_DLRU attempting to flush and free
eligible dcaches.  However, if an error occurs during the flush to
CacheItems (afs_WriteDCache()), e.g., -EINTR, afs_GetDownDSlot() will
assert.

However, a panic in this case is overkill, since afs_GetDownDSlot() is a
best-effort attempt to free dslots.  The caller (afs_UFSGetDSlot()) will
allocate more dcaches if needed.

Instead:
- Refactor afs_GetDownDSlot() by moving the QRemove() call to after the
afs_WriteDCache logic, so it accompanies the logic that puts the dcache
back on the freelist.  This is safe because we hold the afs_xdcache W
lock for the duration of the routine.
- If afs_WriteDCache() returns an error, return early and let the caller
handle any recovery.

Change-Id: Ifd0d56120095c9792998ff935776bbd339a76c8a
Reviewed-on: https://gerrit.openafs.org/13364
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

src/afs/afs_dcache.c

index 4b93915..15e63ff 100644 (file)
@@ -1282,8 +1282,6 @@ afs_GetDownDSlot(int anumber)
        if (tdc->refCount == 0) {
            if ((ix = tdc->index) == NULLIDX)
                osi_Panic("getdowndslot");
-           /* pull the entry out of the lruq and put it on the free list */
-           QRemove(&tdc->lruq);
 
            /* write-through if modified */
            if (tdc->dflags & DFEntryMod) {
@@ -1305,12 +1303,23 @@ afs_GetDownDSlot(int anumber)
                    AFS_GLOCK();
                }
 #else
+               int code;
+
+               code = afs_WriteDCache(tdc, 1);
+               if (code) {
+                   /*
+                    * We couldn't flush it at this time; return early because
+                    * if afs_WriteDCache() failed once it is likely to
+                    * continue failing for subsequent dcaches.
+                    */
+                   return;
+               }
                tdc->dflags &= ~DFEntryMod;
-               osi_Assert(afs_WriteDCache(tdc, 1) == 0);
 #endif
            }
 
-           /* finally put the entry in the free list */
+           /* pull the entry out of the lruq and put it on the free list */
+           QRemove(&tdc->lruq);
            afs_indexTable[ix] = NULL;
            afs_indexFlags[ix] &= ~IFEverUsed;
            tdc->index = NULLIDX;