Re: [Xen-devel] [PATCH v4 2/8] mm: Extract allocation loop from alloc_heap_pages()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 19.05.17 at 17:50, <boris.ostrovsky@xxxxxxxxxx> wrote:
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -694,22 +694,15 @@ static void page_list_add_scrub(struct page_info *pg, 
> unsigned int node,
>          page_list_add(pg, &heap(node, zone, order));
>  }
>  
> -/* Allocate 2^@order contiguous pages. */
> -static struct page_info *alloc_heap_pages(
> -    unsigned int zone_lo, unsigned int zone_hi,
> -    unsigned int order, unsigned int memflags,
> -    struct domain *d)
> +static struct page_info *get_free_buddy(unsigned int zone_lo,
> +                                        unsigned int zone_hi,
> +                                        unsigned int order, unsigned int 
> memflags,
> +                                        struct domain *d)

Did you check whether this can be const here?

>  {
> -    unsigned int i, j, zone = 0, nodemask_retry = 0;
>      nodeid_t first_node, node = MEMF_get_node(memflags), req_node = node;
> -    unsigned long request = 1UL << order;
> +    nodemask_t nodemask = d ? d->node_affinity : node_online_map;
> +    unsigned int j, zone, nodemask_retry = 0, request = 1UL << order;

There's a type mismatch here - either you mean 1U or request's
type should be unsigned long (following the old code it should be
the latter). Otoh it's not immediately visible from the patch
whether in both functions this really still is a useful local variable
at all.

With these two taken care of
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan


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