From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tim Deegan Subject: Re: [PATCH 2 of 2] x86/mem_sharing: Rectify test for "empty" physmap entry in sharing_add_to_physmap Date: Wed, 25 Apr 2012 13:41:47 +0100 Message-ID: <20120425124147.GD51354@ocelot.phlegethon.org> References: <51646b89b1822fe74ffb.1335295219@xdev.gridcentric.ca> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <51646b89b1822fe74ffb.1335295219@xdev.gridcentric.ca> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andres Lagar-Cavilla Cc: andres@gridcentric.ca, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org At 15:20 -0400 on 24 Apr (1335280819), Andres Lagar-Cavilla wrote: > xen/arch/x86/mm/mem_sharing.c | 7 ++++--- > xen/include/asm-x86/p2m.h | 8 ++++++++ > 2 files changed, 12 insertions(+), 3 deletions(-) > > > Signed-off-by: Andres Lagar-Cavilla > > diff -r 5be9a05f17fd -r 51646b89b182 xen/arch/x86/mm/mem_sharing.c > --- a/xen/arch/x86/mm/mem_sharing.c > +++ b/xen/arch/x86/mm/mem_sharing.c > @@ -1073,9 +1073,10 @@ int mem_sharing_add_to_physmap(struct do > if ( spage->sharing->handle != sh ) > goto err_unlock; > > - /* Make sure the target page is a hole in the physmap */ > - if ( mfn_valid(cmfn) || > - (!(p2m_is_ram(cmfn_type))) ) > + /* Make sure the target page is a hole in the physmap. These are typically > + * p2m_mmio_dm, but also accept p2m_invalid and paged out pages. See the > + * definition of p2m_is_hole in p2m.h. */ > + if ( !p2m_is_hole(cmfn_type) || mfn_valid(cmfn) ) Hmm. Is the mfn_valid() to handle p2m_ram_paging_in entries sometimes having an MFN and sometimes not? I think it would be nicer to either always replace paging-in pages or never do it. In any case, it's bogus to test mfn_valid() for any of the other types. Cheers, Tim.