Re: [Xen-devel] [PATCH] xen: set P2M entry to INVALID_P2M_ENTRY for ballooned out pages

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 26 Aug 2013, Wei Liu wrote:
> On Thu, Aug 22, 2013 at 02:00:05PM +0100, David Vrabel wrote:
> > On 22/08/13 13:57, Wei Liu wrote:
> > > In commit cd9151e2: xen/balloon: set a mapping for ballooned out pages
> > > we have the ballooned out page's mapping set to a scratch page. That 
> > > commit
> > > also set the P2M entry of ballooned out page to the scratch, which is
> > > not necessary. That triggers BUG_ONs in __set_phys_to_machine when the
> > > page is ballooned in again.
> > > 
> > > We only need to ensure that the ballooned out pages have valid mapping.
> > > P2M entries of those pages should still be set to INVALID_P2M_ENTRY.
> > 
> > I know I suggested this but I would like to get Stefano's acked-by or
> > reviewed-by for this.
> > 
> 
> Stefano, what do you think?

I think it is wrong: if the page has a valid mapping it should also have
a valid p2m entry. It's easier to think about and it's going to avoid
bugs when generic linux code ends up calling a pvop that on Xen looks
into the p2m.

Which one of the BUG_ON in __set_phys_to_machine are hit?  We could
avoid calling set_phys_to_machine in decrease_reservation on
XENFEAT_auto_translated_physmap guests, that would avoid the first
BUG_ON in __set_phys_to_machine. Arguably it's the right thing to do
anyway.

Regarding the second BUG_ON:

if (unlikely(pfn >= MAX_P2M_PFN)) {
    BUG_ON(mfn != INVALID_P2M_ENTRY);
    return true;
}

We shouldn't really be hitting it, right? Why is pfn >= MAX_P2M_PFN?

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel