Re: [PATCH] Allow a ufs cache_dir entry to coexist with a shared memory cache entry

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.

Amos
Received on Mon Oct 15 2012 - 21:59:33 MDT