From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gui Jianfeng Subject: Re: [PATCH 1/3 v2] KVM MMU: make kvm_mmu_zap_page() return the number of zapped sp in total. Date: Wed, 05 May 2010 08:18:59 +0800 Message-ID: <4BE0B973.7000101@cn.fujitsu.com> References: <4BD017CD.1090500@cn.fujitsu.com> <4BD118D7.4040102@cn.fujitsu.com> <4BD12A3D.8050605@cn.fujitsu.com> <4BD136FE.9060803@cn.fujitsu.com> <20100426173609.GC21425@amt.cnet> <4BDED1EE.3050008@cn.fujitsu.com> <20100504132526.GA17508@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: Avi Kivity , Xiao Guangrong , kvm@vger.kernel.org To: Marcelo Tosatti Return-path: Received: from cn.fujitsu.com ([222.73.24.84]:62105 "EHLO song.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-OK) by vger.kernel.org with ESMTP id S932602Ab0EEAUS (ORCPT ); Tue, 4 May 2010 20:20:18 -0400 In-Reply-To: <20100504132526.GA17508@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: Marcelo Tosatti wrote: > On Mon, May 03, 2010 at 09:38:54PM +0800, Gui Jianfeng wrote: >> Hi Marcelo >> >> Actually, it doesn't only affect kvm_mmu_change_mmu_pages() but also affects kvm_mmu_remove_some_alloc_mmu_pages() >> which is called by mmu shrink routine. This will induce upper layer get a wrong number, so i think this should be >> fixed. Here is a updated version. >> >> --- >> From: Gui Jianfeng >> >> Currently, in kvm_mmu_change_mmu_pages(kvm, page), "used_pages--" is performed after calling >> kvm_mmu_zap_page() in spite of that whether "page" is actually reclaimed. Because root sp won't >> be reclaimed by kvm_mmu_zap_page(). So making kvm_mmu_zap_page() return total number of reclaimed >> sp makes more sense. A new flag is put into kvm_mmu_zap_page() to indicate whether the top page is >> reclaimed. kvm_mmu_remove_some_alloc_mmu_pages() also rely on kvm_mmu_zap_page() to return a total >> relcaimed number. > > Isnt it simpler to have kvm_mmu_zap_page return the number of pages it > actually freed? Then always restart the hash walk if return is positive. > OK, although in some cases we might encounter unneeded hash walk restart, but it's not a big problem. I don't object this solution, will post a new patch. Thanks, Gui > > >