From 0e3b2ff0253ed8c6cbdba41200a4ae484d5cd6b7 Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Mon, 22 Jul 2013 11:04:00 -0600 Subject: [PATCH] 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. --- src/MemObject.h | 2 +- src/store_swapout.cc | 22 +++++++++++++++------- 2 files changed, 16 insertions(+), 8 deletions(-) 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; -- 2.47.2