Re: [RFC PATCH v1.1 09/16] mm/damon/core: set samples in apply_probes() if probe weights are set

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Region merging loop bound `max_thres` is incorrectly scaled when probe weights are used, causing premature loop termination and permanent violation of `max_nr_regions` limits.
--

commit cc6e72a3b37a62b24190b0e42f141fba6caab8fc
Author: SJ Park <sj@kernel.org>

mm/damon/core: set samples in apply_probes() if probe weights are set

When probe weights are set, the prepare_access_checks() ops callback is
not invoked, so sampling addresses of regions are unset.  Ask
apply_probes() to set those together in the case.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index a2f768bd07223..5311aa689c8e7 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -3727,7 +3727,8 @@ static int kdamond_fn(void *data)
>  		if (!access_check_disabled && ctx->ops.check_accesses)
>  			max_nr_accesses = ctx->ops.check_accesses(ctx);
>  		if (ctx->ops.apply_probes)
> -			ctx->ops.apply_probes(ctx, false, false);
> +			ctx->ops.apply_probes(ctx, access_check_disabled,
> +					false);

[Severity: High]
Does enabling probe weights here introduce a regression with the region
merging loop bound in kdamond_merge_regions()?

Later in the series, a commit ("mm/damon/core: get merge threshold from
probe hits when weights are set") derives the merge threshold from weighted
sums when probe weights are used. However, the loop bound max_thres in
kdamond_merge_regions() remains tied to the raw access frequency:

mm/damon/core.c:kdamond_merge_regions() {
	...
	max_thres = c->attrs.aggr_interval /
		(c->attrs.sample_interval ?  c->attrs.sample_interval : 1);
	do {
		nr_regions = 0;
		damon_for_each_target(t, c) {
			damon_merge_regions_of(t, threshold, sz_limit, c);
			nr_regions += damon_nr_regions(t);
		}
		threshold = max(1, threshold * 2);
	} while (nr_regions > c->attrs.max_nr_regions &&
			threshold / 2 < max_thres);
}

If a large weighted sum is used as the initial threshold, it seems the loop
will exit immediately on the first iteration without merging enough regions
because threshold / 2 will already exceed the raw max_thres.

Can this permanently violate the max_nr_regions limits?

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

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