Re: [PATCH v4 7/8] mm/page_alloc: retry alloc_contig_pages() when the zonelist is rebuilt

[Vlastimil Babka (SUSE)]

On 10/28/26 22:19, Gregory Price (Meta) wrote:
> Onlining the first block of a zone rebuilds every zonelist in place,
> shifting the slots after the new zone.
> alloc_contig_frozen_pages_noprof() approves a slot by the caller's gfp
> and nodemask, then loads the zone from it:
> 
>   CPU 0                             CPU 1
>   alloc_contig_frozen_pages_noprof(GFP_HIGHUSER | __GFP_THISNODE)
>     slot 0 (NORMAL1) approved
>                                     online_pages()
>                                       build_all_zonelists()
>                                         [NORMAL1]
>                                         -> [MOVABLE1, NORMAL1]
>     zone = zonelist_zone(slot 0)    /* MOVABLE1 */
>     alloc_contig_frozen_range(MOVABLE1 pfns)
> 
> A gigantic hugetlb page can then land in ZONE_MOVABLE, and a nodemask
> caller can get a range from another node.  Gigantic pages are kept out
> of ZONE_MOVABLE since commit 79b63f12abcb ("mm, hugetlb: do not allocate
> non-migrateable gigantic pages from movable zones").
> 
> Check zonelist_update_seq before using each zone, and restart the walk
> if it changed, as the hugetlb pool walk does.  Check again before
> escalating to the pass that may migrate hugetlb pages, so a pass a
> rebuild cut short is redone rather than escalated.
> 
> With kprobes holding the walk across the online, the base kernel put the
> 1G page in ZONE_MOVABLE in 3 of 3 runs, and this change put it in
> ZONE_NORMAL in 3 of 3.  Direct GFP_KERNEL calls with nodemask {1} and
> {2} got a ZONE_MOVABLE range and a node 1 range on base, 3 of 3 each,
> and neither with this change.
> 
> Fixes: 5e27a2df03b8 ("mm/page_alloc: add alloc_contig_pages()")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Gregory Price (Meta) <gourry@xxxxxxxxxx>

Reviewed-by: Vlastimil Babka (SUSE) <vbabka@xxxxxxxxxx>

> ---
>  mm/page_alloc.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 70830689048a..7d06c6c4011a 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -7593,6 +7593,7 @@ struct page *alloc_contig_frozen_pages_noprof(unsigned long nr_pages,
>  		gfp_t gfp_mask, int nid, nodemask_t *nodemask)
>  {
>  	unsigned long ret, pfn, flags;
> +	unsigned int zonelist_cookie;
>  	struct zonelist *zonelist;
>  	struct zone *zone;
>  	struct zoneref *z;
> @@ -7601,8 +7602,13 @@ struct page *alloc_contig_frozen_pages_noprof(unsigned long nr_pages,
>  
>  retry:
>  	zonelist = node_zonelist(nid, gfp_mask);
> +	zonelist_cookie = zonelist_iter_begin();
>  	for_each_zone_zonelist_nodemask(zone, z, zonelist,
>  					gfp_zone(gfp_mask), nodemask) {
> +		/* A rebuild may have shifted an ineligible zone into this slot. */
> +		if (unlikely(check_retry_zonelist(zonelist_cookie)))
> +			goto retry;
> +
>  		spin_lock_irqsave(&zone->lock, flags);
>  
>  		pfn = ALIGN(zone->zone_start_pfn, nr_pages);
> @@ -7637,6 +7643,9 @@ struct page *alloc_contig_frozen_pages_noprof(unsigned long nr_pages,
>  	 * second pass, we will try moving HugeTLB pages when those pages are
>  	 * smaller than the requested contiguous region size.
>  	 */
> +	/* Redo a pass that a rebuild cut short before escalating. */
> +	if (unlikely(check_retry_zonelist(zonelist_cookie)))
> +		goto retry;
>  	if (skip_hugetlb && skipped_hugetlb) {
>  		skip_hugetlb = false;
>  		goto retry;