Re: [Xen-devel] Balloon driver bug in increase_reservation

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Sep 02, 2013 at 03:48:43PM +0100, Ian Campbell wrote:
> On Mon, 2013-09-02 at 15:43 +0100, Wei Liu wrote:
> > Hi, Stefano
> > 
> > I found another bug in the balloon scratch page code. As I didn't follow
> > the discussion on scratch page so I cannot propose a proper fix at the
> > moment.
> > 
> > The problem is that in balloon.c:increase_reservation, when a ballooned
> > page is resued, it can have a valid P2M entry pointing to the scratch,
> > hitting the BUG_ON
> > 
> >    BUG_ON(!xen_feature(XENFEAT_auto_translated_physmap) &&
> >           phys_to_machine_mapping_valid(pfn));
> > 
> > As balloon worker might run by a CPU other then the one that returns the
> > page, checking pfn_to_mfn(pfn) == local_cpu_scratch_page_mfn wouldn't
> > work.  Checking pfn_to_mfn(pfn) belongs to the set of all scratch page
> > mfns is not desirable.
> 
> This makes me think that whoever suggested that pfn_to_mfn for a
> ballooned page out to return INVALID_MFN was right.
> 

If there are many balloon pages the check can be expensive. If we make
the scratch page globally shared by all CPUs the check can be less
expensive? I don't understand why we need to have one page per CPU at
first glance.

Wei.

> Even though we happen to have put a mapping there there pages are
> "logically empty" and that's really what callers such as
> phys_to_machine_mapping_valid care about, not whether the processor
> thinks there is a mapping or not.
> 
> > 
> > My thoughts so far:
> >  1. remove that BUG_ON (looks like a wrong fix though)
> >  2. make balloon scratch page global
> > 
> > Other thoughts?
> > 
> > Wei.
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxx
> > http://lists.xen.org/xen-devel
> 

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