[PATCH] linux/balloon: prefer using pages from balloon in alloc_empty_pages_and_pagevec()

[PATCH] linux/balloon: prefer using pages from balloon in alloc_empty_pages_and_pagevec()

[Jan Beulich]

This is both faster and less demanding on kernel resources.

Likely also something that could be done in the pv-ops tree (though it
would need some adjustment to deal with the balloon_order!=0 case).

Signed-off-by: Jan Beulich <jbeulich@novell.com>

--- head-2009-12-07.orig/drivers/xen/balloon/balloon.c	2009-12-15 09:37:50.000000000 +0100
+++ head-2009-12-07/drivers/xen/balloon/balloon.c	2009-12-15 09:47:04.000000000 +0100
@@ -607,6 +607,17 @@ struct page **alloc_empty_pages_and_page
 		return NULL;
 
 	for (i = 0; i < nr_pages; i++) {
+		balloon_lock(flags);
+		page = balloon_first_page();
+		if (page && !PageHighMem(page)) {
+			UNLIST_PAGE(page);
+			bs.balloon_low--;
+			balloon_unlock(flags);
+			pagevec[i] = page;
+			continue;
+		}
+		balloon_unlock(flags);
+
 		page = pagevec[i] = alloc_page(GFP_KERNEL|__GFP_COLD);
 		if (page == NULL)
 			goto err;

Re: [PATCH] linux/balloon: prefer using pages from balloon in alloc_empty_pages_and_pagevec()

[Jan Beulich]

>>> "Jan Beulich" <JBeulich@novell.com> 16.12.09 16:13 >>>
>Likely also something that could be done in the pv-ops tree (though it
>would need some adjustment to deal with the balloon_order!=0 case).

... which seems to be broken at present anyway: The main loops in
{alloc,free}_empty_pages_and_pagevec() both did not get their
upper bound updated correctly (should be npages, but was left to
be nr_pages), for alloc_empty_pages_and_pagevec() the wrong value
is also being used to allocate pagevec[] (though I really think that this
shouldn't allocate more than a nr_pages vector to avoid a rather large
but unnecessary kmalloc() when nr_pages is small), and in its error
path only a single page gets freed (rather than the whole
balloon_order chunk).

Jan

Re: [PATCH] linux/balloon: prefer using pages from balloon in alloc_empty_pages_and_pagevec()

[Dave McCracken]

On Wednesday 16 December 2009, Jan Beulich wrote:
> ... which seems to be broken at present anyway: The main loops in
> {alloc,free}_empty_pages_and_pagevec() both did not get their
> upper bound updated correctly (should be npages, but was left to
> be nr_pages), for alloc_empty_pages_and_pagevec() the wrong value
> is also being used to allocate pagevec[] (though I really think that this
> shouldn't allocate more than a nr_pages vector to avoid a rather large
> but unnecessary kmalloc() when nr_pages is small), and in its error
> path only a single page gets freed (rather than the whole
> balloon_order chunk).

You are correct.  The endpoint of the main loops should be npages, as should 
the variable used in the kmalloc().

I could only allocate the pagevec to be nr_pages and just fill it in with the 
partial order 9 page rather than rounding it up.  I'll ponder that.

I don't see anything wrong with the error path.  It correctly walks back down 
the pagevec and frees all the pages it's allocated so far.

Dave McCracken
Oracle Corp.

Re: [PATCH] linux/balloon: prefer using pages from balloon in alloc_empty_pages_and_pagevec()

[Jan Beulich]

>>> Dave McCracken <dcm@mccr.org> 17.12.09 21:33 >>>
>I don't see anything wrong with the error path.  It correctly walks back down 
>the pagevec and frees all the pages it's allocated so far.

Oh, you're probably looking at the code past the err: label, but I was
referring to the handling of a failed apply_to_page_range().

Jan

Re: [PATCH] linux/balloon: prefer using pages from balloon in alloc_empty_pages_and_pagevec()

[Dave McCracken]

On Friday 18 December 2009, Jan Beulich wrote:
> >>> Dave McCracken <dcm@mccr.org> 17.12.09 21:33 >>>
> >
> >I don't see anything wrong with the error path.  It correctly walks back
> > down  the pagevec and frees all the pages it's allocated so far.
> 
> Oh, you're probably looking at the code past the err: label, but I was
> referring to the handling of a failed apply_to_page_range().

Ok, yeah.  It frees up the page that's currently being worked on, then jumps 
to err: to clean up all the other pages in the pagevec and free the pagevec.  
Is there something wrong about it that I'm missing?

Dave McCracken
Oracle Corp

Re: [PATCH] linux/balloon: prefer using pages from balloon in alloc_empty_pages_and_pagevec()

[Jan Beulich]

>>> Dave McCracken <dcm@mccr.org> 18.12.09 15:32 >>>
>On Friday 18 December 2009, Jan Beulich wrote:
>> >>> Dave McCracken <dcm@mccr.org> 17.12.09 21:33 >>>
>> >
>> >I don't see anything wrong with the error path.  It correctly walks back
>> > down  the pagevec and frees all the pages it's allocated so far.
>> 
>> Oh, you're probably looking at the code past the err: label, but I was
>> referring to the handling of a failed apply_to_page_range().
>
>Ok, yeah.  It frees up the page that's currently being worked on, then jumps 
>to err: to clean up all the other pages in the pagevec and free the pagevec.  
>Is there something wrong about it that I'm missing?

It should __free_pages(..., order) rather than __free_page(...).

Jan

Re: [PATCH] linux/balloon: prefer using pages from balloon in alloc_empty_pages_and_pagevec()

[Dave McCracken]

On Friday 18 December 2009, Jan Beulich wrote:
> >Ok, yeah.  It frees up the page that's currently being worked on, then
> > jumps  to err: to clean up all the other pages in the pagevec and free
> > the pagevec. Is there something wrong about it that I'm missing?
> 
> It should __free_pages(..., order) rather than __free_page(...).

Gah, you're right.  Of course.  Sorry.  I'll add that fix to my patch.

Dave McCracken