Re: [PATCH 05/12] xen: introduce reserve_heap_pages

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.04.2020 03:02, Stefano Stabellini wrote:
> Introduce a function named reserve_heap_pages (similar to
> alloc_heap_pages) that allocates a requested memory range. Call
> __alloc_heap_pages for the implementation.
> 
> Change __alloc_heap_pages so that the original page doesn't get
> modified, giving back unneeded memory top to bottom rather than bottom
> to top.

While it may be less of a problem within a zone, doing so is
against our general "return high pages first" policy.

> @@ -1073,7 +1073,42 @@ static struct page_info *alloc_heap_pages(
>          return NULL;
>      }
>  
> -    __alloc_heap_pages(&pg, order, memflags, d);
> +    __alloc_heap_pages(pg, order, memflags, d);
> +    return pg;
> +}
> +
> +static struct page_info *reserve_heap_pages(struct domain *d,
> +                                            paddr_t start,
> +                                            unsigned int order,
> +                                            unsigned int memflags)
> +{
> +    nodeid_t node;
> +    unsigned int zone;
> +    struct page_info *pg;
> +
> +    if ( unlikely(order > MAX_ORDER) )
> +        return NULL;
> +
> +    spin_lock(&heap_lock);
> +
> +    /*
> +     * Claimed memory is considered unavailable unless the request
> +     * is made by a domain with sufficient unclaimed pages.
> +     */
> +    if ( (outstanding_claims + (1UL << order) > total_avail_pages) &&
> +          ((memflags & MEMF_no_refcount) ||
> +           !d || d->outstanding_pages < (1UL << order)) )
> +    {
> +        spin_unlock(&heap_lock);
> +        return NULL;
> +    }

Where would such a claim come from? Given the purpose I'd assume
the function (as well as reserve_domheap_pages()) can actually be
__init. With that I'd then also be okay with them getting built
unconditionally, i.e. even on x86.

> +    pg = maddr_to_page(start);
> +    node = phys_to_nid(start);
> +    zone = page_to_zone(pg);
> +    page_list_del(pg, &heap(node, zone, order));
> +
> +    __alloc_heap_pages(pg, order, memflags, d);

I agree with Julien in not seeing how this can be safe / correct.

Jan