From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Mon, 22 Jul 2013 17:04:00 +0000 (-0600)
Subject: Fixed StoreEntry::mayStartSwapOut() logic to handle terminated swapouts.
X-Git-Tag: SQUID_3_5_0_1~444^2~31
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=0e3b2ff0253ed8c6cbdba41200a4ae484d5cd6b7;p=thirdparty%2Fsquid.git

Fixed StoreEntry::mayStartSwapOut() logic to handle terminated swapouts.

StoreEntry::mayStartSwapOut() should return true if a swapout can start. If
swapout was started earlier but then terminated for some reason (setting sio
to nil), the method should not return true. Checking swap_status ==
SWAPOUT_DONE does not work reliably because the status may be reset to
SWAPOUT_NONE in some cases (and the check was too late anyway). Checking
decision == swPossible does not work at all because while swapout start was
possible at some point, it is no longer possible after we started swapping
out.

Added MemObject::SwapOut::swStarted to detect started swapouts reliably.
---

diff --git a/src/MemObject.h b/src/MemObject.h
index 36d3390c3d..79232f8fab 100644
--- a/src/MemObject.h
+++ b/src/MemObject.h
@@ -133,7 +133,7 @@ public:
         StoreIOState::Pointer sio;
 
         /// Decision states for StoreEntry::swapoutPossible() and related code.
-        typedef enum { swNeedsCheck = 0, swImpossible = -1, swPossible = +1 } Decision;
+        typedef enum { swNeedsCheck = 0, swImpossible = -1, swPossible = +1, swStarted } Decision;
         Decision decision; ///< current decision state
     };
 
diff --git a/src/store_swapout.cc b/src/store_swapout.cc
index 581a6db08a..f77fab1aaa 100644
--- a/src/store_swapout.cc
+++ b/src/store_swapout.cc
@@ -69,6 +69,7 @@ storeSwapOutStart(StoreEntry * e)
            e->swap_dirn << ", fileno " << std::hex << std::setw(8) << std::setfill('0') <<
            std::uppercase << e->swap_filen);
     e->swap_status = SWAPOUT_WRITING;
+    mem->swapout.decision = MemObject::SwapOut::swStarted;
     /* If we start swapping out objects with OutOfBand Metadata,
      * then this code needs changing
      */
@@ -372,18 +373,12 @@ StoreEntry::mayStartSwapOut()
     assert(mem_obj);
     MemObject::SwapOut::Decision &decision = mem_obj->swapout.decision;
 
-    // if we decided that swapout is not possible, do not repeat same checks
+    // if we decided that starting is not possible, do not repeat same checks
     if (decision == MemObject::SwapOut::swImpossible) {
         debugs(20, 3, HERE << " already rejected");
         return false;
     }
 
-    // if we decided that swapout is possible, do not repeat same checks
-    if (decision == MemObject::SwapOut::swPossible) {
-        debugs(20, 3,  HERE << "already allowed");
-        return true;
-    }
-
     // if we swapped out already, do not start over
     if (swap_status == SWAPOUT_DONE) {
         debugs(20, 3,  HERE << "already did");
@@ -391,6 +386,19 @@ StoreEntry::mayStartSwapOut()
         return false;
     }
 
+    // if we stared swapping out already, do not start over
+    if (decision == MemObject::SwapOut::swStarted) {
+        debugs(20, 3, "already started");
+        decision = MemObject::SwapOut::swImpossible;
+        return false;
+    }
+
+    // if we decided that swapout is possible, do not repeat same checks
+    if (decision == MemObject::SwapOut::swPossible) {
+        debugs(20, 3, "already allowed");
+        return true;
+    }
+
     if (!checkCachable()) {
         debugs(20, 3,  HERE << "not cachable");
         decision = MemObject::SwapOut::swImpossible;