Re: [PATCH 2/3] xen-balloon: Add interface to retrieve ballooned pages

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Wed, Mar 09, 2011 at 04:22:04PM -0500, Daniel De Graaf wrote:
> On 03/09/2011 04:03 PM, Konrad Rzeszutek Wilk wrote:
> > On Mon, Mar 07, 2011 at 01:06:47PM -0500, Daniel De Graaf wrote:
> >> Pages that have been ballooned are useful for other Xen drivers doing
> >> grant table actions, because these pages have valid struct page/PFNs but
> >> have no valid MFN so are available for remapping.
> >>
> >> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> >> ---
> >>  drivers/xen/balloon.c |   54 +++++++++++++++++++++++++++++++++++++++++++++++++
> >>  include/xen/balloon.h |    3 ++
> >>  2 files changed, 57 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
> >> index b0a7a92..be53596 100644
> >> --- a/drivers/xen/balloon.c
> >> +++ b/drivers/xen/balloon.c
> >> @@ -328,6 +328,60 @@ void balloon_set_new_target(unsigned long target)
> >>  }
> >>  EXPORT_SYMBOL_GPL(balloon_set_new_target);
> >>  
> >> +/**
> >> + * get_ballooned_pages - get pages that have been ballooned out
> >> + * @nr_pages: Number of pages to get
> >> + * @pages: pages returned
> >> + * @force: Try to balloon out more pages if needed
> >> + * @return number of pages retrieved
> >> + */
> >> +int get_ballooned_pages(int nr_pages, struct page** pages, int force)
> >> +{
> >> +	int rv = 0;
> >> +	struct page* page;
> >> +	mutex_lock(&balloon_mutex);
> >> +	/* Pages are pulled off the back of the queue to prefer highmem */
> >> +	while (rv < nr_pages) {
> >> +		if (list_empty(&ballooned_pages)) {
> >> +			if (!force)
> >> +				break;
> >> +			if (decrease_reservation(nr_pages - rv))
> > 
> > I am looking at the implementation of decrease_reservation with
> > Daniel's Kipper patches ("xen/balloon: Protect against CPU exhaust by event/x process")
> > and his code will the functionality of trying to balloon out the amount of
> > pages you want. But if it can't balloon out all, it will try the best it
> > can. Meaning if nr_pages is 256 and rv is 0, and we only say get 100 of them,
> > this particular loop would keep on going forever.. until it got the rest
> > of those 156 pages which it might not.
> > 
> > Perhaps rebasing on top of Daniel's patch and utilize the BP_EAGAIN logic
> > to back-off, or just return -EBUSY with the amount returned?
> > 
> 
> I think adding a parameter to decrease_reservation specifying what GFP_* flags
> will be used in allocation might be useful. The correct rebase of my most recent
> patch on top of Daniel Kiper's patch would be to return -ENOMEM when
> decrease_reservation returns BP_EAGAIN. Since GFP_BALLOON does not try very hard

<nods>
> in the memory allocation, this might produce memory errors more often than it
> should, whereas the alloc_xenballooned_pages call may wish to just use GFP_HIGHUSER
> and risk invoking the OOM killer if required.

OK. Looking forward to the patches.
> 
> Would you prefer I submitted all three patches rebased on top of Daniel Kipper's
> R4 patches (or do you have ak git tree with those already included)?

#devel/balloon-hotplug (jsut pushed it so it might take 10 minutes for it to show up).