Re: [PATCH v2 2/3] mm: page_alloc: make the high-order watermark check honour ALLOC_NOFRAGMENT

["Vlastimil Babka (SUSE)" <vbabka@kernel.org>]

On 11/2/26 10:39, Kiryl Shutsemau (Meta) wrote:
> With defrag_mode, every slowpath allocation carries ALLOC_NOFRAGMENT.  A
> request below pageblock_order may take a page of its own migratetype or
> claim a whole free block, but not steal from another migratetype's block.
> The high-order part of the watermark check still accepts a free page of
> any migratetype.
> 
> Under memory pressure, movable allocations take free movable pages as
> soon as reclaim frees them.  Free space in unmovable and reclaimable
> pageblocks is off-limits to them under ALLOC_NOFRAGMENT, and builds up.
> The free pages of order 4 and up end up mostly in those pageblocks.
> 
> A 64K mTHP fault cannot take them, but direct compaction's up-front check
> counts them and returns COMPACT_SUCCESS without compacting anything.  The
> allocation then fails.  Because compaction "succeeded", it is never
> deferred, and every fault stalls in it again before falling back to small
> pages.
> 
> Dev Jain's benchmark faults 64K mTHPs under swap pressure, in a 7G VM
> with only the 64K size enabled.  With defrag_mode, those faults stall in
> compaction 18 times as often and get about a third of the folios
> (medians of 15 runs):
> 
>                               defrag_mode=0  defrag_mode=1  defrag_mode=1
>                                                             + this patch
>     compact_stall                      1690          30669          2607
>     compact_fail                        621          30531           906
>     64K faults allocated              20750           7391         11538
>     64K faults falling back to 4K       725          13832          9945
> 
> Make __zone_watermark_ok_mt() skip other migratetypes' free lists below
> pageblock_order for an ALLOC_NOFRAGMENT request.  Give
> __compact_finished() the same rule for its fallback test, so that
> compaction, once it runs, stops on a page the request can take.
> 
> zone_watermark_fast() calls __zone_watermark_ok_mt() too.  With
> defrag_mode, an allocation now wakes kswapd and starts reclaim or
> compaction as soon as the free lists hold nothing it can take.  For the
> orders the per-cpu lists serve, that is earlier than before: the check
> does not count their pages, so the allocation no longer uses up the local
> list first.  A request that may not wake kswapd or reclaim, such as an
> mTHP fault with THP defrag=never, falls back to a smaller folio instead.
> 
> Fixes: e3aa7df331bc ("mm: page_alloc: defrag_mode")
> Reported-by: Dev Jain <dev.jain@arm.com>
> Closes: https://lore.kernel.org/all/4aa14b41-9b89-494c-8825-55fffd5a541c@arm.com/
> Tested-by: Dev Jain <dev.jain@arm.com>
> Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>

This seems fine enough.

> ---
>  mm/compaction.c |  5 +++++
>  mm/page_alloc.c | 21 ++++++++++++++++++---
>  2 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 6c553d0e90f8..7b2267bbf115 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -2365,6 +2365,11 @@ static enum compact_result __compact_finished(struct compact_control *cc)
>  			!free_area_empty(area, MIGRATE_CMA))
>  			return COMPACT_SUCCESS;
>  #endif
> +		/* Under ALLOC_NOFRAGMENT it may only claim a whole block */
> +		if ((cc->alloc_flags & ALLOC_NOFRAGMENT) &&
> +		    order < pageblock_order)
> +			continue;
> +
>  		/*
>  		 * Job done if allocation would steal freepages from
>  		 * other migratetype buddy lists.
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 7730660bdd09..d53fb2c67ae1 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3586,6 +3586,11 @@ static inline long __zone_watermark_unusable_free(struct zone *z,
>   * will return true of the order-0 watermark is reached and there is at least
>   * one free page of a suitable size. Checking now avoids taking the zone lock
>   * to check in the allocation paths if no pages are free.
> + *
> + * Under ALLOC_NOFRAGMENT, a request below pageblock_order can take another
> + * migratetype's page only by claiming the whole block, so only free pages of
> + * @migratetype count below pageblock_order.  Without ALLOC_NOFRAGMENT,
> + * @migratetype is not used.
>   */
>  bool __zone_watermark_ok_mt(struct zone *z, unsigned int order,
>  			    unsigned long mark, int highest_zoneidx,
> @@ -3648,6 +3653,9 @@ bool __zone_watermark_ok_mt(struct zone *z, unsigned int order,
>  			continue;
>  
>  		for (mt = 0; mt < MIGRATE_PCPTYPES; mt++) {
> +			if ((alloc_flags & ALLOC_NOFRAGMENT) &&
> +			    mt != migratetype && o < pageblock_order)
> +				continue;
>  			if (!free_area_empty(area, mt))
>  				return true;
>  		}
> @@ -3677,8 +3685,12 @@ bool __zone_watermark_ok(struct zone *z, unsigned int order, unsigned long mark,
>  			 int highest_zoneidx, unsigned int alloc_flags,
>  			 long free_pages)
>  {
> +	/* Serving an ALLOC_NOFRAGMENT request needs __zone_watermark_ok_mt() */
> +	VM_WARN_ON_ONCE(order && order < pageblock_order &&
> +			(alloc_flags & ALLOC_NOFRAGMENT));
>  	return __zone_watermark_ok_mt(z, order, mark, highest_zoneidx,
> -				      alloc_flags, free_pages, MIGRATE_MOVABLE);
> +				      alloc_flags & ~ALLOC_NOFRAGMENT,
> +				      free_pages, MIGRATE_MOVABLE);
>  }
>  
>  bool zone_watermark_ok(struct zone *z, unsigned int order, unsigned long mark,
> @@ -4685,10 +4697,13 @@ should_reclaim_retry(gfp_t gfp_mask, unsigned order,
>  
>  		/*
>  		 * Would the allocation succeed if we reclaimed all
> -		 * reclaimable pages?
> +		 * reclaimable pages?  Count free pages of every migratetype:
> +		 * reclaim and compaction work across migratetypes to free a
> +		 * whole pageblock, which the allocation can then claim.
>  		 */
>  		wmark = __zone_watermark_ok(zone, order, min_wmark,
> -				ac->highest_zoneidx, alloc_flags, available);
> +				ac->highest_zoneidx,
> +				alloc_flags & ~ALLOC_NOFRAGMENT, available);
>  		trace_reclaim_retry_zone(z, order, reclaimable,
>  				available, min_wmark, *no_progress_loops, wmark);
>  		if (wmark) {