Re: [RFC PATCH 11/16] mm/damon/core: get merge threshold from probe hits when weights are set

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The merge threshold limit (`max_thres`) in `kdamond_merge_regions` is not scaled for probe weights, causing premature termination of the region merging loop and failing to enforce the user-defined `max_nr_regions` limit.

Pre-existing issues:
- [High] When probe weights are configured for operations that do not implement `apply_probes` (e.g., `vaddr`), the merge threshold becomes 0 and regions are improperly and aggressively merged.
--

commit 23bf451408faae1b0a1f1eb841dc6f4b60077861
Author: SJ Park <sj@kernel.org>
Subject: mm/damon/core: get merge threshold from probe hits when weights are set

When probe weights are set, DAMON merges regions based on their probe hits
weighted sum, but the merge threshold is calculated based on access frequency.
Update it to retrieve the maximum probe hits weighted sum in the snapshot from
the apply_probes() ops callback, and generate the threshold based on it.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index bd6d836f1db5e..7a070cfb7cd1b 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -3683,9 +3684,18 @@ static int kdamond_fn(void *data)
>  
>  		if (!access_check_disabled && ctx->ops.check_accesses)
>  			max_merge_score = ctx->ops.check_accesses(ctx);
> -		if (ctx->ops.apply_probes)
> -			ctx->ops.apply_probes(ctx, access_check_disabled,
> -					false);
> +		if (ctx->ops.apply_probes) {
> +			if (time_after_eq(ctx->passed_sample_intervals,
> +						next_aggregation_sis) &&
> +					access_check_disabled)
> +				get_max_wsum = true;
> +			else
> +				get_max_wsum = false;
> +			max_wsum = ctx->ops.apply_probes(ctx,
> +					access_check_disabled, get_max_wsum);
> +			if (get_max_wsum)
> +				max_merge_score = max_wsum;
> +		}

[Severity: High]
This is a pre-existing issue, but what happens if a user configures probe
weights for operations that do not implement apply_probes (like vaddr)?

When damon_has_probe_weights() returns true, access checks are disabled. If
the operations lack an apply_probes callback, max_merge_score will remain 0.

Will this cause all regions to be improperly and unconditionally merged into
maximum-sized chunks, resulting in a loss of monitoring resolution?

[Severity: High]
Does kdamond_merge_regions() properly handle this new weight-based score limit?

Just after this block, max_merge_score is passed into kdamond_merge_regions():

mm/damon/core.c:kdamond_fn() {
    ...
		if (time_after_eq(ctx->passed_sample_intervals,
					next_aggregation_sis)) {
			kdamond_merge_regions(ctx,
					max_merge_score / 10,
					sz_limit);
    ...
}

Because max_thres inside kdamond_merge_regions() is hardcoded to the maximum
possible frequency of unweighted accesses, rather than dynamically accounting
for probe weights, the scaling loop termination condition:

mm/damon/core.c:kdamond_merge_regions() {
	...
	} while (nr_regions > c->attrs.max_nr_regions &&
			threshold / 2 < max_thres);
	...
}

appears to instantly fail when the initial threshold is already large due to
probe weights.

Will this cause an early abort of the fallback loop, leaving regions unmerged
and failing to enforce the max_nr_regions limit?

>  
>  		if (time_after_eq(ctx->passed_sample_intervals,
>  					next_aggregation_sis)) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260705205743.98656-1-sj@kernel.org?part=11