From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Wed, 25 Jul 2012 23:57:51 +0000 (-0600)
Subject: Allow a ufs cache_dir entry to coexist with a shared memory cache entry
X-Git-Tag: sourceformat-review-1~161
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9e9dd16734312924388b3215148e5b77c43b94d5;p=thirdparty%2Fsquid.git

Allow a ufs cache_dir entry to coexist with a shared memory cache entry
instead of being released when it becomes idle.

The original boolean version of the StoreController::dereference() code
(r11730) was written to make sure that idle unlocked local store_table entries
are released if nobody needs them (to avoid creating inconsistencies with
shared caches that could be modified in a different process).

Then, in r11786, we realized that the original code was destroying non-shared
memory cache entries if there were no cache_dirs to vote for keeping them in
store_table. I fixed that by changing the StoreController::dereference() logic
from "remove if nobody needs it" to "remove if somebody objects to keeping
it". That solved the problem at hand, but prohibited an entry to exist in
a non-shared cache_dir and in a shared memory cache at the same time.

We now go back to the original "remove if nobody needs it" design but also
give non-shared memory cache a vote so that it can protect idle non-shared
memory cached entries from being released if there are no cache_dirs to vote
for them.
---

diff --git a/src/store_dir.cc b/src/store_dir.cc
index ace530e68b..eb84dcd184 100644
--- a/src/store_dir.cc
+++ b/src/store_dir.cc
@@ -708,25 +708,28 @@ StoreController::reference(StoreEntry &e)
 bool
 StoreController::dereference(StoreEntry & e)
 {
-    bool keepInStoreTable = true; // keep if there are no objections
-
     // special entries do not belong to any specific Store, but are IN_MEMORY
     if (EBIT_TEST(e.flags, ENTRY_SPECIAL))
-        return keepInStoreTable;
+        return true;
+
+    bool keepInStoreTable = false; // keep only if somebody needs it there
 
     /* Notify the fs that we're not referencing this object any more */
 
     if (e.swap_filen > -1)
-        keepInStoreTable = swapDir->dereference(e) && keepInStoreTable;
+        keepInStoreTable = swapDir->dereference(e) || keepInStoreTable;
 
     // Notify the memory cache that we're not referencing this object any more
     if (memStore && e.mem_status == IN_MEMORY)
-        keepInStoreTable = memStore->dereference(e) && keepInStoreTable;
+        keepInStoreTable = memStore->dereference(e) || keepInStoreTable;
 
     // TODO: move this code to a non-shared memory cache class when we have it
     if (e.mem_obj) {
         if (mem_policy->Dereferenced)
             mem_policy->Dereferenced(mem_policy, &e, &e.mem_obj->repl);
+        // non-shared memory cache relies on store_table
+        if (!memStore)
+            keepInStoreTable = true || keepInStoreTable;
     }
 
     return keepInStoreTable;
@@ -834,7 +837,7 @@ StoreController::handleIdleEntry(StoreEntry &e)
                             (mem_node::InUseCount() <= store_pages_max);
     }
 
-    // An idle, unlocked entry that belongs to a SwapDir which controls
+    // An idle, unlocked entry that only belongs to a SwapDir which controls
     // its own index, should not stay in the global store_table.
     if (!dereference(e)) {
         debugs(20, 5, HERE << "destroying unlocked entry: " << &e << ' ' << e);