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

[SJ Park <sj@kernel.org>]

On Sun, 05 Jul 2026 21:12:08 +0000 sashiko-bot@kernel.org wrote:

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

Correct, and that's no problem.  Users who wrongly configured DAMON get poor
output.  As long as no critical consequences like kernel crash or memory leaks
happen, I believe this is fine.

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

Yes, and the later patch in this series will fix it.  Also, to make it no real
issue even before the later patch, we set thee mode detection to say no.  So
the issue cannot happen in the real world.

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


Thanks,
SJ