From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paul Mackerras Subject: Re: [PATCH 4/7] KVM: PPC: Book3S HV: Fix dirty map for hugepages Date: Mon, 26 May 2014 13:30:43 +1000 Message-ID: <20140526033043.GA31285@drongo> References: <1400919721-14264-1-git-send-email-paulus@samba.org> <1400919721-14264-5-git-send-email-paulus@samba.org> <5381BF50.5060701@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm-ppc@vger.kernel.org, kvm@vger.kernel.org To: Alexander Graf Return-path: Content-Disposition: inline In-Reply-To: <5381BF50.5060701@suse.de> Sender: kvm-ppc-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On Sun, May 25, 2014 at 12:00:48PM +0200, Alexander Graf wrote: > > Please document in the function header that the return value is the number > of pages that are dirty. Alternatively rename the function. OK. > > for (i = 0; i < memslot->npages; ++i) { > >- if (kvm_test_clear_dirty(kvm, rmapp) && map) > >- __set_bit_le(i, map); > >+ int ret = kvm_test_clear_dirty(kvm, rmapp); > > Please give this one a better name. npages maybe? OK. > >+ if (ret && map) > >+ for (j = i - (i % ret); ret; ++j, --ret) > > Why do we go down from the index? When a large page is faulted in by kvmppc_book3s_hv_page_fault(), we put the guest HPTE in the rmap chain corresponding to the guest physical address that the guest was trying to access, not the GPA of the start of the large page. So we need i - (i % ret) to get the index of the first system page in the large page. However, on reflection I think that we should always put the guest HPTE in the rmap chain corresponding to the base address of the large page. In that case i % ret would always be zero. I'll repost. Paul.