Re: [PATCH 6.12.y] mm/damon/ops-common: use nr_accesses moving sum for quota score

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Pre-existing 32-bit unsigned int multiplication overflow when calculating access rates and base points, which corrupts the moving sum and DAMON scoring.
--

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

mm/damon/ops-common: use nr_accesses moving sum for quota score

This commit updates the DAMON quota prioritization score calculation to use
the moving sum of accesses instead of the potentially not fully aggregated
access count. This change aims to resolve suboptimum DAMOS performance under
certain setups.

Link: https://lore.kernel.org/20260719161136.90191-1-sj@kernel.org

> diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
> index 4b5638bbe0614..dea662a6747ce 100644
> --- a/mm/damon/ops-common.c
> +++ b/mm/damon/ops-common.c
> @@ -80,8 +80,9 @@ int damon_hot_score(struct damon_ctx *c, struct damon_region *r,
>  	unsigned int age_weight = s->quota.weight_age;
>  	int hotness;
>  
> -	freq_subscore = r->nr_accesses * DAMON_MAX_SUBSCORE /
> -		damon_max_nr_accesses(&c->attrs);
> +	freq_subscore = mult_frac(r->nr_accesses_bp / 10000,
> +			DAMON_MAX_SUBSCORE,
> +			damon_max_nr_accesses(&c->attrs));

[Severity: High]
This isn't a bug introduced by this patch, but changing the score calculation
to rely on nr_accesses_bp makes it susceptible to a pre-existing 32-bit
unsigned integer multiplication overflow.

When DAMON is configured with an aggr_interval / sample_interval ratio
greater than 429,496 (for example, a 1ms sample interval and a 10-minute
aggregation interval, which gives 600,000), last_nr_accesses can exceed
429,496.

This causes a silent overflow when updating the moving sum in
damon_update_region_access_rate():

mm/damon/core.c:damon_update_region_access_rate() {
	...
	r->nr_accesses_bp = damon_moving_sum(r->nr_accesses_bp,
			r->last_nr_accesses * 10000, len_window,
			accessed ? 10000 : 0);
	...
}

A similar overflow occurs in damon_nr_accesses_to_accesses_bp():

mm/damon/core.c:damon_nr_accesses_to_accesses_bp() {
	...
	return nr_accesses * 10000 / damon_max_nr_accesses(attrs);
}

Because this patch modifies damon_hot_score() to calculate freq_subscore
using the corrupted nr_accesses_bp, will DAMON calculate garbage hotness
scores for memory regions? 

Previously, it used r->nr_accesses * DAMON_MAX_SUBSCORE, which wouldn't
overflow until a ratio of > 42,949,672. Could this change cause incorrect
functional behavior, such as wrongly reclaiming hot memory or failing to
reclaim cold memory on valid long-running configurations?

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