From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50240) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YJN2X-00035J-G0 for qemu-devel@nongnu.org; Thu, 05 Feb 2015 09:04:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YJN2S-0007nW-CN for qemu-devel@nongnu.org; Thu, 05 Feb 2015 09:04:17 -0500 Received: from mx1.redhat.com ([209.132.183.28]:33377) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YJN2R-0007nO-SZ for qemu-devel@nongnu.org; Thu, 05 Feb 2015 09:04:12 -0500 Message-ID: <54D37828.60308@redhat.com> Date: Thu, 05 Feb 2015 09:03:20 -0500 From: Max Reitz MIME-Version: 1.0 References: <1423140931-11823-1-git-send-email-berto@igalia.com> <54D374AE.2090500@redhat.com> <20150205135934.GA18629@igalia.com> In-Reply-To: <20150205135934.GA18629@igalia.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] block: Give always priority to unused entries in the qcow2 L2 cache List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alberto Garcia Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi 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