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

[sashiko-bot@kernel.org]

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.

> -#ifdef CONFIG_DAMON_DEBUG_SANITY
> -static void damon_verify_merge_regions_of(struct damon_region *r)
> -{
> -	WARN_ONCE(r->nr_accesses != r->nr_accesses_bp / 10000,
> -			"nr_accesses (%u) != nr_accesses_bp (%u)\n",
> -			r->nr_accesses, r->nr_accesses_bp);
> -}
> -#else
> -static void damon_verify_merge_regions_of(struct damon_region *r)
> -{
> -}
> -#endif
> -
> -
>  /*
>   * Merge adjacent regions having similar access frequencies

[ ... ]

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