Re: [Xen-devel] [PATCH] x86/NUMA: make init_node_heap() respect Xen heap limit

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Thu, 2015-08-27 at 02:37 -0600, Jan Beulich wrote:
> On NUMA systems, where we try to use node local memory for the basic
> control structures of the buddy allocator, this special case needs to
> take into consideration a possible address width limit placed on the
> Xen heap. In turn this (but also other, more abstract considerations)
> requires that xenheap_max_mfn() not be called more than once (at most
> we might permit it to be called a second time with a larger value than
> was passed the first time), and be called only before calling
> end_boot_allocator().
> 
> While inspecting all the involved code, a couple of off-by-one issues
> were found (and are being corrected here at once):
> - arch_init_memory() cleared one too many page table slots
> - the highmem_start based invocation of xenheap_max_mfn() passed too
>   big a value
> - xenheap_max_mfn() calculated the wrong bit count in edge cases
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

@@ -428,14 +434,18 @@ static unsigned long init_node_heap(int
>      }
>  #ifdef DIRECTMAP_VIRT_END
>      else if ( *use_tail && nr >= needed &&
> -              (mfn + nr) <= (virt_to_mfn(eva - 1) + 1) )
> +              (mfn + nr) <= (virt_to_mfn(eva - 1) + 1) &&
> +              (!xenheap_bits ||
> +               !((mfn + nr - 1) >> (xenheap_bits - PAGE_SHIFT))) )

This logic appears twice (with just s/nr/needed/ the second time), and it
is a reasonably complex set of conditions. Moving it into a helper might be
a nice cleanup, which would also allow a descriptive name to be used and
also perhaps separating the various conditions into their own if (...)
return which might aid readability.

Ian.

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