Re: [PATCH v3 04/12] mm: hugetlb_vmemmap: generalize vmemmap page allocation

[Oscar Salvador (SUSE)]

On Wed, Nov 04, 2026 at 10:35:02AM +0800, Muchun Song wrote:
> Vmemmap page allocation is currently tied to restoring a virtual range
> and assumes an empty destination list.  These constraints prevent it from
> being reused when pages are requested independently of a vmemmap range or
> appended to an existing list.
> 
> Generalize the allocator and give it an all-or-nothing list contract so
> the restore and optimization paths can share the same allocation policy.
> This also allows HVO head-page allocation to fail gracefully under memory
> pressure without triggering the OOM killer.
> 
> Signed-off-by: Muchun Song <songmuchun@xxxxxxxxxxxxx>
> Reviewed-by: Lance Yang <lance.yang@xxxxxxxxx>
> Reviewed-by: Qi Zheng <qi.zheng@xxxxxxxxx>

Reviewed-by: Oscar Salvador <osalvador@xxxxxxx>

A small nit below:

> diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c
> index 0e5b2b182d1b..d3058e106795 100644
> --- a/mm/hugetlb_vmemmap.c
> +++ b/mm/hugetlb_vmemmap.c
...
> +static int alloc_vmemmap_page_list(struct list_head *head,
> +		unsigned int nr_pages, int nid)
>  {
> -	gfp_t gfp_mask = GFP_KERNEL | __GFP_RETRY_MAYFAIL;
> -	unsigned long nr_pages = (end - start) >> PAGE_SHIFT;
> -	int nid = page_to_nid((struct page *)start);
> +	LIST_HEAD(new_pages);
>  	struct page *page, *next;
> -	int i;
>  
> -	for (i = 0; i < nr_pages; i++) {
> -		page = alloc_pages_node(nid, gfp_mask, 0);
> +	for (unsigned int i = 0; i < nr_pages; i++) {
> +		page = alloc_pages_node(nid, GFP_KERNEL | __GFP_RETRY_MAYFAIL, 0);
>  		if (!page)
> -			goto out;
> -		list_add(&page->lru, list);
> +			goto free_pages;
> +		list_add(&page->lru, &new_pages);
>  	}
> +	list_splice(&new_pages, head);
>  	memmap_pages_add(nr_pages);
>  
>  	return 0;
> -out:
> -	list_for_each_entry_safe(page, next, list, lru)
> +free_pages:
> +	list_for_each_entry_safe(page, next, &new_pages, lru)
>  		__free_page(page);
>  	return -ENOMEM;
>  }
...
> @@ -527,15 +526,13 @@ static int __hugetlb_vmemmap_optimize_folio(const struct hstate *h,
>  	 */
>  	folio_set_hugetlb_vmemmap_optimized(folio);
>  
> -	vmemmap_head = alloc_pages_node(nid, GFP_KERNEL, 0);
> -	if (!vmemmap_head) {
> +	if (alloc_vmemmap_page_list(vmemmap_pages, VMEMMAP_OPTIMIZATION_PAGES, nid)) {
>  		ret = -ENOMEM;
>  		goto out;

alloc_vmemmap_page_list() already gives you the error code (ENOMEM), so
it would look more natural to me to use that already and not "overwrite"
it.
e.g: 

 ret = alloc_vmemmap_page_list
 if (ret)
     goto out;

but as I said, just a nit :-)
 

-- 
Oscar Salvador
SUSE Labs