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

[Max Reitz <mreitz@redhat.com>]

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 <mreitz@redhat.com>