On 10/15/2012 03:59 PM, Amos Jeffries wrote: > On 16.10.2012 09:04, Alex Rousskov wrote: >> Hello, >> >> The attached patch allows 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 but >> suitable for memory cache entries from being released if there are no >> cache_dirs to vote for them. >> >> >> This is a second revision of the fix. The first one (trunk r12231) was >> reverted because it did not pass tests/testRock unit tests on some >> platforms. The unit tests assume that the entry slot is not locked after >> the entry is stored, but the first revision of the fix allowed idle >> entries to remain in store_table and, hence, their slots were locked and >> could not be replaced, causing testRock assertions. This revision >> allows the idle entry to be destroyed (and its slot unlocked) if >> [non-shared] memory caching is disabled. >> >> It is not clear to me why only some of the platforms were affected by >> this. Should not memory caching be disabled everywhere during testRock >> (because testRock does not set memory cache capacity and memory cache >> entry size limits)? >> >> The changes passed build farm tests (except a couple of nodes with >> Jenkins-related build problems unrelated to the code). >> >> >> Thank you, >> >> Alex. > > > Cool, lets give it a whirl. +1.
Committed as trunk r12395. I will watch for testRock failures after this change (combined with the flexible arrays fix). Thank you, Alex.
