Re: [Xen-devel] [PATCH v3 1/9] mm: Separate free page chunk merging into its own routine

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 14.04.17 at 17:37, <boris.ostrovsky@xxxxxxxxxx> wrote:
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -919,11 +919,50 @@ static int reserve_offlined_page(struct page_info *head)
>      return count;
>  }
>  
> +/* Returns new buddy head. */
> +static struct page_info *
> +merge_and_free_buddy(struct page_info *pg, unsigned int node,
> +                     unsigned int zone, unsigned int order)
> +{
> +    ASSERT(spin_is_locked(&heap_lock));
> +
> +    /* Merge chunks as far as possible. */
> +    while ( order < MAX_ORDER )
> +    {
> +        unsigned long mask = 1UL << order;
> +        struct page_info *buddy;
> +
> +        if ( (page_to_mfn(pg) & mask) )

Stray inner parentheses.

> +            buddy = pg - mask; /* Merge with predecessor block. */
> +        else
> +            buddy = pg + mask; /* Merge with successor block. */
> +
> +        if ( !mfn_valid(_mfn(page_to_mfn(buddy))) ||
> +             !page_state_is(buddy, free) ||
> +             (PFN_ORDER(buddy) != order) ||
> +             (phys_to_nid(page_to_maddr(buddy)) != node) )

I think generated code would benefit from pulling out the page->mfn
translation, using the result also in place of page_to_maddr().

With at least the cosmetic issue above taken care of,
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan


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