Re: [Xen-devel] [RFC PATCH] page_alloc: use first half of higher order chunks when halving

[Tim Deegan <tim@xxxxxxx>]

At 13:22 +0200 on 25 Mar (1395750124), Matt Wilson wrote:
> From: Matt Rushton <mrushton@xxxxxxxxxx>
> 
> This patch makes the Xen heap allocator use the first half of higher
> order chunks instead of the second half when breaking them down for
> smaller order allocations.
> 
> Linux currently remaps the memory overlapping PCI space one page at a
> time. Before this change this resulted in the mfns being allocated in
> reverse order and led to discontiguous dom0 memory. This forced dom0
> to use bounce buffers for doing DMA and resulted in poor performance.

This seems like something better fixed on the dom0 side, by asking
explicitly for contiguous memory in cases where it makes a difference.
On the Xen side, this change seems harmless, but we might like to keep
the explicitly reversed allocation on debug builds, to flush out
guests that rely on their memory being contiguous.

> This change more gracefully handles the dom0 use case and returns
> contiguous memory for subsequent allocations.
> 
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Keir Fraser <keir@xxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> Cc: Tim Deegan <tim@xxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Signed-off-by: Matt Rushton <mrushton@xxxxxxxxxx>
> Signed-off-by: Matt Wilson <msw@xxxxxxxxxx>
> ---
>  xen/common/page_alloc.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
> index 601319c..27e7f18 100644
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -677,9 +677,10 @@ static struct page_info *alloc_heap_pages(
>      /* We may have to halve the chunk a number of times. */
>      while ( j != order )
>      {
> -        PFN_ORDER(pg) = --j;
> +        struct page_info *pg2;
> +        pg2 = pg + (1 << --j);
> +        PFN_ORDER(pg) = j;
>          page_list_add_tail(pg, &heap(node, zone, j));
> -        pg += 1 << j;

AFAICT this uses the low half (pg) for the allocation _and_ puts it on
the freelist, and just leaks the high half (pg2).  Am I missing something?

Tim.


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