Re: [Qemu-devel] [PATCH] block: Give always priority to unused entries in the qcow2 L2 cache

[Kevin Wolf]

Am 05.02.2015 um 14:59 hat Alberto Garcia geschrieben:
> On Thu, Feb 05, 2015 at 08:48:30AM -0500, Max Reitz wrote:
> 
> > >-        c->entries[i].cache_hits /= 2;
> > >+        if (c->entries[i].cache_hits > 1) {
> > >+            c->entries[i].cache_hits /= 2;
> > >+        }
> > >      }
> > >      if (min_index == -1) {
> > 
> > Hm, I can't see where the code is actually giving priority to unused
> > entries. qcow2_cache_find_entry_to_replace() is the only place which
> > selects the entry to be used
> 
> Yes, and it looks for the one with the lowest cache hit count. That is
> the only criteria:
> 
>         if (c->entries[i].cache_hits < min_count) {
>             min_index = i;
>             min_count = c->entries[i].cache_hits;
>         }
> 
> If there are several with the same hit count then the first one is
> chosen.
> 
> Since dividing the hit count by two everytime there's a cache miss can
> make it go down to zero, an existing entry with cache_hits == 0 will
> always be chosen before any empty one located afterwards in the array.
> 
> By never allowing the hit count to go down to zero, we make sure that
> all unused entries are chosen first before a valid one is discarded.

But does this actually improve a lot? cache_hits is only 0 for the first
few accesses and it never becomes 0 again after that. The result might
be that soon all the entries have cache_hits == 1, and we get the same
problem as you're describing - only the first few entries will be
reused.

If this happens a lot in practice and we get much more cache misses than
cache hits that would be required to keep tables in memory, we may need
to rethink the whole eviction strategy rather than changing just a small
detail.

Do you have any specific workload that is improved with your patch, and
do you have numbers for the effect of the change?

Kevin