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

[Max Reitz]

On 2015-02-05 at 08:59, Alberto Garcia wrote:
> 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.

Oh, right. I was wondering because cache_hits is not reset to 0 in 
qcow2_cache_entry_flush(); but that function is not meant for emptying 
an entry but only making sure it's not dirty, and qcow2_cache_empty() 
indeed sets cache_hits to 0.

Thus:

Reviewed-by: Max Reitz <address@hidden>