From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben-Ami Yassour Subject: Re: [PATCH] Handle vma regions with no backing page Date: Wed, 04 Jun 2008 18:09:24 +0300 Message-ID: <1212592164.26322.10.camel@lnx-benami> References: <20080603113937.GE8158@duo.random> Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Cc: aliguori@us.ibm.com, Han Weidong , "Kay, Allen M" , Muli Ben-Yehuda , Amit Shah , kvm@vger.kernel.org, avi@qumranet.com To: Andrea Arcangeli Return-path: Received: from mtagate8.de.ibm.com ([195.212.29.157]:11723 "EHLO mtagate8.de.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752636AbYFDPKG (ORCPT ); Wed, 4 Jun 2008 11:10:06 -0400 Received: from d12nrmr1607.megacenter.de.ibm.com (d12nrmr1607.megacenter.de.ibm.com [9.149.167.49]) by mtagate8.de.ibm.com (8.13.8/8.13.8) with ESMTP id m54F9Pgp328968 for ; Wed, 4 Jun 2008 15:09:25 GMT Received: from d12av03.megacenter.de.ibm.com (d12av03.megacenter.de.ibm.com [9.149.165.213]) by d12nrmr1607.megacenter.de.ibm.com (8.13.8/8.13.8/NCO v8.7) with ESMTP id m54F9PPg4145398 for ; Wed, 4 Jun 2008 17:09:25 +0200 Received: from d12av03.megacenter.de.ibm.com (loopback [127.0.0.1]) by d12av03.megacenter.de.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id m54F9Oio031479 for ; Wed, 4 Jun 2008 17:09:25 +0200 In-Reply-To: <20080603113937.GE8158@duo.random> Sender: kvm-owner@vger.kernel.org List-ID: On Tue, 2008-06-03 at 13:39 +0200, Andrea Arcangeli wrote: > 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. It does work for NUMA, it does not work without the NUMA option. > 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.