Re: [PATCH 1/2] mm/compaction: keep compaction deferral state per migration mode

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm: incomplete tracing of async mode in compaction_defer_reset()
--

commit 0627b5eaa00c60d552c24b2f41326741ce401351
Author: Qiliang Yuan <odys.yuan@gmail.com>

mm/compaction: keep compaction deferral state per migration mode

This commit updates compaction deferral state variables to be arrays
indexed by the sync migration mode. The deferral tracepoints are also
updated to include a sync field to indicate which state they report.

> diff --git a/mm/compaction.c b/mm/compaction.c
> index a049415512c67..7f8845d1990aa 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;
> +	int sync;
> +
> +	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;
>  	}
> -	if (order >= zone->compact_order_failed)
> -		zone->compact_order_failed = order + 1;
>  
> -	trace_mm_compaction_defer_reset(zone, order);
> +	trace_mm_compaction_defer_reset(zone, order, true);

[Severity: Medium]
Does this miss tracing the async state reset?

The code in compaction_defer_reset() updates both the async and sync state
arrays within the loop, but the trace event is emitted outside the loop with a
hardcoded true argument for the sync parameter.

This seems to silently omit the state transition for async compaction where
sync is false. Userspace tracing tools might miss the deferral reset event
for async compaction and falsely assume it remains deferred after a reset.

Should this trace event be moved inside the loop to emit for both states, or
is there a reason to only trace the sync reset?

>  }

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