From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrea Arcangeli Subject: Re: [PATCH] Handle vma regions with no backing page Date: Tue, 3 Jun 2008 13:39:37 +0200 Message-ID: <20080603113937.GE8158@duo.random> References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: aliguori@us.ibm.com, Han Weidong , "Kay, Allen M" , Muli Ben-Yehuda , Amit Shah , kvm@vger.kernel.org, avi@qumranet.com To: Ben-Ami Yassour Return-path: Received: from host36-195-149-62.serverdedicati.aruba.it ([62.149.195.36]:46235 "EHLO mx.cpushare.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753865AbYFCLjj (ORCPT ); Tue, 3 Jun 2008 07:39:39 -0400 Content-Disposition: inline In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On Tue, Jun 03, 2008 at 02:17:55PM +0300, Ben-Ami Yassour wrote: > Anthony Liguori wrote on 04/29/2008 05:32:09 PM: > >> Subject >> >> [PATCH] Handle vma regions with no backing page >> >> This patch allows VMA's that contain no backing page to be used for guest >> memory. This is a drop-in replacement for Ben-Ami's first page in his >> direct >> mmio series. Here, we continue to allow mmio pages to be represented in >> the >> rmap. > >> struct page *gfn_to_page(struct kvm *kvm, gfn_t gfn) >> { >> - return pfn_to_page(gfn_to_pfn(kvm, gfn)); >> + pfn_t pfn; >> + >> + pfn = gfn_to_pfn(kvm, gfn); >> + if (pfn_valid(pfn)) >> + return pfn_to_page(pfn); >> + >> + return NULL; >> } > > We noticed that pfn_valid does not always works as expected by this patch > to indicate that a pfn has a backing page. > We have seen a case where CONFIG_NUMA was not set and then where pfn_valid > returned 1 for an mmio pfn. > We then changed the config file with CONFIG_NUMA set and it worked fine as > expected (since a different implementation of pfn_valid was used). > > How should we overcome this issue? There's a page_is_ram() too, but that's the e820 map check and it means it's RAM not that there's a page backing store. Certainly if it's not ram we should go ahead with just the pfn but it'd be a workaround. I really think it'd be better off to fix pfn_valid to work for NUMA. I can't see how pfn_valid can be ok to return true when there's no backing page... Probably pfn_valid was used for debugging todate, but if you check vm_normal_page you'll see that it is not used just for debugging and it seems VM_MIXEDMAP will break as much as KVM. I can't see how VM_MIXEDMAP can be sane doing pfn_to_page(pfn) and pretending this is a normal page, when there's no 'struct page' backing the pfn.