Re: [PATCH v3 24/26] mm/page_alloc: always direct compact for unmapped allocs

On 7/27/26 00:22, Brendan Jackman wrote:
> This is the minimal solution for ensuring that compaction can service
> unmapped allocations. Without this, it's possible for compaction to just
> check watermarks and see plenty of free pages, without being aware of
> the direct map state, and thereby cause an ALLOC_UNMAPPED allocation to
> fail unnecessarily.
> 
> Instead, with this change, promote compact_order to pageblock order for
> unmapped allocations, much like defrag_mode. Then, check specifically in
> compaction for the presence of wholly mapped blocks that can be unmapped
> once direct compact is complete.
> 
> This all takes advantage of a major simplification: since unmapped
> blocks are currently always unmovable, this can be asymmetric. There is
> never a need to promote a !ALLOC_UNMAPPED allocation to compacting at
> pageblock_order, because compaction would be trying to generate a
> currently-unmapped block to map; that will always fail because it would
> require migrating unmapped pages, which is not supported at the moment.
> 
> Signed-off-by: Brendan Jackman <[email protected]>

Reviewed-by: Vlastimil Babka (SUSE) <[email protected]>

Nit:

> ---
>  mm/compaction.c | 22 ++++++++++++++++++----
>  mm/page_alloc.c |  9 +++++++++
>  2 files changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/compaction.c b/mm/compaction.c
> index ed12d2fc6fad3..fe1aaf293bbce 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -2531,12 +2531,25 @@ bool compaction_zonelist_suitable(struct alloc_context *ac, int order,
>  static enum compact_result
>  compaction_suit_allocation_order(struct zone *zone, unsigned int order,
>  				 int highest_zoneidx, unsigned int alloc_flags,
> -				 bool async, bool kcompactd)
> +				 bool unmapped, bool async, bool kcompactd)

Instead of the new bool parameter, can we check alloc_flags for ALLOC_UNMAPPED?

>  {
>  	unsigned long free_pages;
>  	unsigned long watermark;
>  
> -	if (kcompactd && defrag_mode)
> +	/*
> +	 * When trying to generate an unmapped block, check the counter for
> +	 * direct-mapped blocks specifically, since we'll need to unmap the
> +	 * whole block to service the allocation.
> +	 *
> +	 * Why doesn't this apply to the other way around too? (Mightn't we need
> +	 * to _map_ a whole block, to service a !ALLOC_UNMAPPED allocation?) No,
> +	 * because of a likely-temporary simplification: currently, unmapped
> +	 * blocks never contain movable pages, so compaction isn't going to free
> +	 * up one of those.
> +	 */
> +	if (unmapped)
> +		free_pages = zone_page_state(zone, NR_FREE_PAGES_BLOCKS_MAPPED);
> +	else if (kcompactd && defrag_mode)
>  		free_pages = zone_free_pages_blocks(zone);
>  	else
>  		free_pages = zone_page_state(zone, NR_FREE_PAGES);
> @@ -2599,6 +2612,7 @@ compact_zone(struct compact_control *cc, struct capture_control *capc)
>  		ret = compaction_suit_allocation_order(cc->zone, cc->order,
>  						       cc->highest_zoneidx,
>  						       cc->alloc_flags,
> +						       freetype_unmapped(cc->freetype),
>  						       cc->mode == MIGRATE_ASYNC,
>  						       !cc->direct_compaction);
>  		if (ret != COMPACT_CONTINUE)
> @@ -3084,7 +3098,7 @@ static bool kcompactd_node_suitable(pg_data_t *pgdat)
>  		ret = compaction_suit_allocation_order(zone,
>  				pgdat->kcompactd_max_order,
>  				highest_zoneidx, alloc_flags,
> -				false, true);
> +				false, false, true);
>  		if (ret == COMPACT_CONTINUE)
>  			return true;
>  	}
> @@ -3127,7 +3141,7 @@ static void kcompactd_do_work(pg_data_t *pgdat)
>  
>  		ret = compaction_suit_allocation_order(zone,
>  				cc.order, zoneid, cc.alloc_flags,
> -				false, true);
> +				false, false, true);
>  		if (ret != COMPACT_CONTINUE)
>  			continue;
>  
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index d12ce84662ab7..5f1dea7eee15b 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -827,6 +827,9 @@ compaction_capture(struct capture_control *capc, struct page *page,
>  	    capc_mt != MIGRATE_MOVABLE)
>  		return false;
>  
> +	if (freetype_flags(freetype) != freetype_flags(capc->freetype))
> +		return false;
> +
>  	if (migratetype != capc_mt)
>  		trace_mm_page_alloc_extfrag(page, capc->order, order,
>  					    capc_mt, migratetype);
> @@ -4523,6 +4526,12 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
>  	if ((alloc_flags & ALLOC_NOFRAGMENT) &&
>  	    free_to_migratetype(ac->freetype) != MIGRATE_MOVABLE)
>  		compact_order = max(order, pageblock_order);
> +	/*
> +	 * Unmapped allocations benefit from compaction even at order 0, because the
> +	 * allocator will actually grab a whole block.
> +	 */
> +	if (freetype_flags(ac->freetype) & FREETYPE_UNMAPPED)
> +		compact_order = max(order, pageblock_order);
>  
>  	if (!compact_order)
>  		return NULL;
>