Re: [RFC PATCH v1.2 09/17] mm/damon/core: remove damon_verify_merge_regions_of()

[SeongJae Park <sj@kernel.org>]

On Sun, 21 Jun 2026 18:09:54 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Missing size-weighted averaging for `last_nr_accesses` upon region merge causes erroneous age resets and corrupted metrics.
> --
> 
> commit 6d81112ae07520e7dd1deaeaf23912d17d4d8a57
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/core: remove damon_verify_merge_regions_of()
> 
> This commit removes the damon_verify_merge_regions_of() validation check.
> The nr_accesses_bp field is no longer used for a real purpose, making this
> validation unnecessary.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index ee19238d62f5f..94d8ec2965451 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -3114,20 +3114,6 @@ static void damon_merge_two_regions(struct damon_target *t,
> >  	damon_destroy_region(r, t);
> >  }
> 
> [Severity: High]
> This is a pre-existing issue, but does damon_merge_two_regions() need to
> calculate a size-weighted average for last_nr_accesses alongside nr_accesses
> and age?
> 
> Currently, when two adjacent regions are combined, last_nr_accesses appears
> to be omitted from the averaging logic, causing the newly merged region to
> strictly inherit the left region's history.
> 
> Because kdamond_merge_regions() iterates its merge loop until the region
> limit is satisfied, a region merged in the first iteration will be evaluated
> again in the second iteration:
> 
> mm/damon/core.c:damon_merge_regions_of() {
>         ...
>         if (abs(r->nr_accesses - r->last_nr_accesses) > thres)
>                 r->age = 0;
>         ...
> }
> 
> Since r->nr_accesses was size-averaged but r->last_nr_accesses was not,
> this comparison evaluates against mismatched data. Could this frequently
> cause it to erroneously reset r->age to 0?
> 
> If so, this seems like it might break DAMON schemes that rely on region age
> (such as pageout schemes meant to reclaim cold memory) by continuously
> resetting the region's age. It might also skew policy frequency scores in
> damon_nr_accesses_mvsum() if damon_set_attrs() is called from a user context
> before the aggregation interval resets.

Good finding, but not necessarily a blocker of this series.  I will separately
work on this.


Thanks,
SJ

[...]