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

[Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>]

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@xxxxxxxxxxxxx>
>> ---
>>  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
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.

Would you prefer I submitted all three patches rebased on top of Daniel Kipper's
R4 patches (or do you have a git tree with those already included)?

>> +                            force = 0;
>> +            } else {
>> +                    page = list_entry(ballooned_pages.prev,
>> +                            struct page, lru);
>> +                    list_del(&page->lru);
>> +                    pages[rv++] = page;
>> +            }
>> +    }
>> +    mutex_unlock(&balloon_mutex);
>> +    return rv;
>> +}
>> +EXPORT_SYMBOL(get_ballooned_pages);
>> +
>> +/**
>> + * put_ballooned_pages - return pages retrieved with get_ballooned_pages
>> + * @nr_pages: Number of pages
>> + * @pages: pages to return
>> + */
>> +void put_ballooned_pages(int nr_pages, struct page** pages)
>> +{
>> +    int i;
>> +
>> +    mutex_lock(&balloon_mutex);
>> +
>> +    for (i = 0; i < nr_pages; i++) {
>> +            if (PageHighMem(pages[i])) {
>> +                    list_add_tail(&pages[i]->lru, &ballooned_pages);
>> +            } else {
>> +                    list_add(&pages[i]->lru, &ballooned_pages);
>> +            }
>> +    }
>> +
>> +    mutex_unlock(&balloon_mutex);
>> +}
>> +EXPORT_SYMBOL(put_ballooned_pages);
>> +
>>  static int __init balloon_init(void)
>>  {
>>      unsigned long pfn, nr_pages, extra_pfn_end;
>> diff --git a/include/xen/balloon.h b/include/xen/balloon.h
>> index b2b7c21..5fc25fa 100644
>> --- a/include/xen/balloon.h
>> +++ b/include/xen/balloon.h
>> @@ -19,3 +19,6 @@ struct balloon_stats {
>>  extern struct balloon_stats balloon_stats;
>>  
>>  void balloon_set_new_target(unsigned long target);
>> +
>> +int get_ballooned_pages(int nr_pages, struct page** pages, int force);
>> +void put_ballooned_pages(int nr_pages, struct page** pages);
>> -- 
>> 1.7.3.4
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@xxxxxxxxxxxxxxxxxxx
>> http://lists.xensource.com/xen-devel
> 


-- 
Daniel De Graaf
National Security Agency

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