[Xen-devel] Re: [PATCH v2] Use ballooned pages for gntdev

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

On Thu, 2011-03-10 at 14:56 +0000, Daniel De Graaf wrote:
> On 03/10/2011 04:18 AM, Ian Campbell wrote:
> > On Wed, 2011-03-09 at 23:07 +0000, Daniel De Graaf wrote:
> >> Split up the balloon module, similar to how grants and events are
> >> handled. This allow gntdev to use ballooned pages without adding a
> >> dependency on the balloon module.
> >>
> >> The interface is slightly different from that exposed in 2.6.32; the
> >> page vector is allocated by the caller, not by the balloon driver. This
> >> allows more freedom in how the returned pages are used, since they do
> >> not all have to be returned to the balloon driver at the same time. It
> >> also tries to reuse already ballooned pages rather than always
> >> ballooning new pages to ensure they are in low memory as the 2.6.32
> >> version did.
> >>
> >> Changes since v1:
> >>    * Rebased on konrad/devel/{balloon-hotplug,next-2.6.38} merge
> >>    * Better function names
> >>    * Adjust balloon stats for requested pages
> >>
> >> [PATCH 1/3] xen-balloon: Move core balloon functionality out of module
> >> [PATCH 2/3] xen-balloon: Add interface to retrieve ballooned pages
> >> [PATCH 3/3] xen-gntdev: Use ballooned pages for grant mappings
> > 
> > Does this introduces a new potential failure case? When a normal balloon
> > operation takes place is it is now possible for the ballooned_pages list
> > to be empty (because gntdev has stolen stuff from it) where before we
> > knew the list was non-empty at that point?
> > 
> > e.g. increase_reservation includes:
> >             page = balloon_retrieve();
> >             BUG_ON(page == NULL);
> > as well as:
> >     page = balloon_first_page();
> >     for (i = 0; i < nr_pages; i++) {
> >             BUG_ON(page == NULL);
> >             frame_list[i] = page_to_pfn(page);
> >             page = balloon_next_page(page);
> >     }
> > 
> > Are we sure we aren't about to exercise those BUG_ONs?
> 
> This is safe because increase_reservation is protected by the balloon
> mutex. There is a loop just before the hypercall that verifies that
> there are enough pages in the list.

Which loop? I don't see anything like that in either
increase_reservation or the caller (balloon process).

> > As a secondary concern, assuming we don't crash, does the balloon
> > process correctly sleep until explicitly kicked when ballooned_pages
> > becomes empty? Or does it keep needlessly waking up on the jiffy timer?
> > IOW does credit in balloon_process become 0 when this situation occurs?
> > 
> > Or does adjusting the balloon stats ensure this all works out alright?
> 
> Adjusting the statistics should make this work out properly. Since only
> pages actually in the list will be counted in balloon_low+balloon_high,
> which is the lower bound for credit, we will always be able to satisfy
> the reservation changes requested by balloon_process. The jiffy timer is
> used only when Linux or Xen cannot satisfy the memory allocations required
> to adjust the balloon size.

OK.

> 
> > An interesting experiment would probably be to allocate a bunch of
> > address space via gntdev and then manually balloon the guest above it's
> > current allocation, back below the lower limit due to the gntdev
> > allocation, back up to starting point etc etc.
> > 
> > Ian.
> > 
> 
> That does sound like a useful test case.
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel