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

[SJ Park <sj@kernel.org>]

On Mon, 06 Jul 2026 15:02:05 +0000 sashiko-bot@kernel.org wrote:

> 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?

No.  damon_has_probe_weights() always returns false at this moment, so no
issue.

> 
> >  
> >  		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


Thanks,
SJ

[...]