Re: [PATCH 2/2] mm/compaction: defer failed async direct compaction

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mm: async compaction clears skip hints causing full zone rescans
- [Low] mm: missing tracepoint for async compaction deferral reset
--

commit 2e6082a2f3b4a8dcf4b7cf16ef03d8f061912e07
Author: Qiliang Yuan <odys.yuan@gmail.com>
Date:   Thu Oct 1 15:33:49 2026 +0000

    mm/compaction: defer failed async direct compaction

This patch splits the compaction deferral state into sync and async, allowing
async compaction to be deferred independently of sync compaction. It aims to
prevent unnecessary zone rescans during THP allocations by keeping async
deferral state.

> diff --git a/mm/compaction.c b/mm/compaction.c
> index a049415512c67..25f758bc54f06 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
[ ... ]
> @@ -165,24 +165,28 @@ static bool compaction_deferred(struct zone *zone, int order)
>  void compaction_defer_reset(struct zone *zone, int order,
>  		bool alloc_success)
>  {
> -	if (alloc_success) {
> -		zone->compact_considered = 0;
> -		zone->compact_defer_shift = 0;
> -	}
> -	if (order >= zone->compact_order_failed)
> -		zone->compact_order_failed = order + 1;
> +	int sync;
>  
> -	trace_mm_compaction_defer_reset(zone, order);
> +	for (sync = 0; sync < ASYNC_AND_SYNC; sync++) {
> +		if (alloc_success) {
> +			zone->compact_considered[sync] = 0;
> +			zone->compact_defer_shift[sync] = 0;
> +		}
> +		if (order >= zone->compact_order_failed[sync])
> +			zone->compact_order_failed[sync] = order + 1;
> +	}
> +
> +	trace_mm_compaction_defer_reset(zone, order, true);

[Severity: Low]
Does this tracepoint unintentionally omit the async state? The tracepoint
unconditionally reports a reset for sync compaction (true), but entirely
omits the corresponding event for the async state that was also modified
in the loop.

>  }
>  
> -/* Returns true if restarting compaction after many failures */
> +/* Returns true if restarting sync compaction after many failures */
>  static bool compaction_restarting(struct zone *zone, int order)
>  {
> -	if (order < zone->compact_order_failed)
> +	if (order < zone->compact_order_failed[true])
>  		return false;
>  
> -	return zone->compact_defer_shift == COMPACT_MAX_DEFER_SHIFT &&
> -		zone->compact_considered >= 1UL << zone->compact_defer_shift;
> +	return zone->compact_defer_shift[true] == COMPACT_MAX_DEFER_SHIFT &&
> +		zone->compact_considered[true] >= 1UL << zone->compact_defer_shift[true];
>  }
[ ... ]
> @@ -2596,7 +2600,9 @@ compact_zone(struct compact_control *cc, struct capture_control *capc)
>  
>  	/*
>  	 * Clear pageblock skip if there were failures recently and compaction
> -	 * is about to be retried after being deferred.
> +	 * is about to be retried after being deferred. Only do it when sync
> +	 * compaction restarts: async compaction relies on the skip hints, and
> +	 * clearing them on every async retry would rescan the whole zone.
>  	 */
>  	if (compaction_restarting(cc->zone, cc->order))
>  		__reset_isolation_suitable(cc->zone);

[Severity: High]
Does this code clear skip hints during async compaction, causing full zone
rescans?

The comment explicitly states: "Only do it when sync compaction restarts".
However, compaction_restarting() only evaluates the sync deferral state. If
sync compaction has been fully deferred (compact_defer_shift[true] ==
COMPACT_MAX_DEFER_SHIFT), this will return true for all callers.

Without checking if the current compaction is actually synchronous (e.g.,
via cc->mode != MIGRATE_ASYNC), wouldn't this cause async compaction to
continuously clear skip hints and rescan the entire zone on every failed
THP fault?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261001-bug-mm-thp-async-compact-defer-v1-0-0174c7923430@gmail.com?part=2