Re: [PATCH v2 3/3] mm: vmscan: tell direct reclaim the allocation is ALLOC_NOFRAGMENT

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

On 11/2/26 10:39, Kiryl Shutsemau (Meta) wrote:
> Direct reclaim for a costly order stops as soon as the allocation could
> succeed.  compaction_ready() skips a zone and should_continue_reclaim()
> stops reclaiming when zone_watermark_ok() passes, and zone_watermark_ok()
> accepts a free page of any migratetype.
> 
> With defrag_mode, the allocation behind the reclaim is ALLOC_NOFRAGMENT
> and cannot take the free pages left in unmovable and reclaimable
> pageblocks.  Reclaim stops on them anyway, having freed little.
> 
> In Dev Jain's 64K mTHP benchmark, with the high-order watermark check
> already honouring ALLOC_NOFRAGMENT, an order-4 direct reclaim frees 16
> pages on average with defrag_mode and 45 without.  64K faults get a bit
> over half as many folios as without defrag_mode (medians of 15 runs):
> 
>                               defrag_mode=0  defrag_mode=1  defrag_mode=1
>                                                             + this patch
>     64K faults allocated              20750          11538         19759
>     64K faults falling back to 4K       725           9945          1664
> 
> Pass ALLOC_NOFRAGMENT from the allocation down to try_to_free_pages().
> Both checks then ask __zone_watermark_ok_mt() for a free page the
> allocation can take, and an order-4 direct reclaim frees 40 pages on
> average.
> 
> 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>

I'd perhaps pass whole alloc_flags to try_to_free_pages(), but other than
that LGTM.

> ---
>  mm/internal.h   |  3 ++-
>  mm/page_alloc.c |  8 +++++---
>  mm/vmscan.c     | 24 +++++++++++++++++++-----
>  3 files changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/mm/internal.h b/mm/internal.h
> index 8e58e2db5e09..b43947818f9a 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -65,7 +65,8 @@ void folio_mark_lazyfree(struct folio *folio);
>  /* mm/vmscan.c */
>  unsigned long zone_reclaimable_pages(struct zone *zone);
>  unsigned long try_to_free_pages(struct zonelist *zonelist, int order,
> -				gfp_t gfp_mask, const nodemask_t *mask);
> +				gfp_t gfp_mask, const nodemask_t *mask,
> +				bool nofragment);
>  unsigned long lruvec_lru_size(struct lruvec *lruvec, enum lru_list lru,
>  			      int zone_idx);
>  
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index d53fb2c67ae1..08f9bdcd3b9f 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -4451,7 +4451,7 @@ static unsigned int check_retry_zonelist(unsigned int seq)
>  
>  /* Perform direct synchronous page reclaim */
>  static unsigned long
> -__perform_reclaim(gfp_t gfp_mask, unsigned int order,
> +__perform_reclaim(gfp_t gfp_mask, unsigned int order, unsigned int alloc_flags,
>  					const struct alloc_context *ac)
>  {
>  	unsigned int noreclaim_flag;
> @@ -4465,7 +4465,8 @@ __perform_reclaim(gfp_t gfp_mask, unsigned int order,
>  	noreclaim_flag = memalloc_noreclaim_save();
>  
>  	progress = try_to_free_pages(ac->zonelist, order, gfp_mask,
> -								ac->nodemask);
> +				     ac->nodemask,
> +				     alloc_flags & ALLOC_NOFRAGMENT);
>  
>  	memalloc_noreclaim_restore(noreclaim_flag);
>  	fs_reclaim_release(gfp_mask);
> @@ -4487,7 +4488,8 @@ __alloc_pages_direct_reclaim(gfp_t gfp_mask, unsigned int order,
>  	int reclaim_order = ac->reclaim_compact_order;
>  
>  	psi_memstall_enter(&pflags);
> -	*did_some_progress = __perform_reclaim(gfp_mask, reclaim_order, ac);
> +	*did_some_progress = __perform_reclaim(gfp_mask, reclaim_order,
> +					       alloc_flags, ac);
>  	if (unlikely(!(*did_some_progress)))
>  		goto out;
>  
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 9959ca158ce8..54a16dbb84dd 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -142,6 +142,9 @@ struct scan_control {
>  	/* One of the zones is ready for compaction */
>  	unsigned int compaction_ready:1;
>  
> +	/* The allocation cannot fall back to other migratetypes' blocks */
> +	unsigned int nofragment:1;
> +
>  	/* There is easily reclaimable cold cache in the current node */
>  	unsigned int cache_trim_mode:1;
>  
> @@ -6316,6 +6319,17 @@ static bool in_reclaim_compaction(struct scan_control *sc)
>  	return false;
>  }
>  
> +static bool allocation_can_succeed(struct zone *zone, struct scan_control *sc,
> +				   unsigned long mark)
> +{
> +	unsigned int alloc_flags = sc->nofragment ? ALLOC_NOFRAGMENT : 0;
> +	long free_pages = zone_page_state(zone, NR_FREE_PAGES);
> +	int migratetype = gfp_migratetype(sc->gfp_mask);
> +
> +	return __zone_watermark_ok_mt(zone, sc->order, mark, sc->reclaim_idx,
> +				      alloc_flags, free_pages, migratetype);
> +}
> +
>  /*
>   * Reclaim/compaction is used for high-order allocation requests. It reclaims
>   * order-0 pages before compacting the zone. should_continue_reclaim() returns
> @@ -6354,8 +6368,7 @@ static inline bool should_continue_reclaim(struct pglist_data *pgdat,
>  		unsigned long watermark = min_wmark_pages(zone);
>  
>  		/* Allocation can already succeed, nothing to do */
> -		if (zone_watermark_ok(zone, sc->order, watermark,
> -				      sc->reclaim_idx, 0))
> +		if (allocation_can_succeed(zone, sc, watermark))
>  			return false;
>  
>  		if (compaction_suitable(zone, sc->order, watermark,
> @@ -6580,8 +6593,7 @@ static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
>  		return false;
>  
>  	/* Allocation can already succeed, nothing to do */
> -	if (zone_watermark_ok(zone, sc->order, min_wmark_pages(zone),
> -			      sc->reclaim_idx, 0))
> +	if (allocation_can_succeed(zone, sc, min_wmark_pages(zone)))
>  		return true;
>  
>  	/*
> @@ -6965,7 +6977,8 @@ static bool throttle_direct_reclaim(gfp_t gfp_mask, struct zonelist *zonelist,
>  }
>  
>  unsigned long try_to_free_pages(struct zonelist *zonelist, int order,
> -				gfp_t gfp_mask, const nodemask_t *nodemask)
> +				gfp_t gfp_mask, const nodemask_t *nodemask,
> +				bool nofragment)
>  {
>  	unsigned long nr_reclaimed;
>  	struct scan_control sc = {
> @@ -6978,6 +6991,7 @@ unsigned long try_to_free_pages(struct zonelist *zonelist, int order,
>  		.may_writepage = 1,
>  		.may_unmap = 1,
>  		.may_swap = 1,
> +		.nofragment = nofragment,
>  	};
>  
>  	/*