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

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

[SJ Park]

Since commit 42f994b71404 ("mm/damon/core: implement scheme-specific apply
interval"), DAMOS scheme can be applied at any time.  At that time,
nr_accesses may not be fully aggregated.  But the quota prioritization
score is calculated using the not fully aggregated count.  As a result,
the performance of DAMOS could be degraded.  Fix by using
damon_nr_accesses_mvsum() instead.

The user impact of the issue is suboptimum DAMOS performance under certain
setups.  Nonetheless, the bug was there from the beginning of the setup
availability.  In other words, the suboptimum performance is the baseline
of the setup and hence it didn't cause regression.  Also the extent of the
suboptimality was not big enough to be found from users and testers.
Still, this is a clear bug that is better to be fixed, and can be easily
fixed.

Link: https://lore.kernel.org/20260719161136.90191-1-sj@kernel.org
Fixes: 42f994b71404 ("mm/damon/core: implement scheme-specific apply interval")
Signed-off-by: SJ Park <sj@kernel.org>
Cc: <stable@vger.kernel.org> # 6.7.x
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
(cherry picked from commit 6c25083f7ae7e2660d766169e5b4d3e96010503f)
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/ops-common.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

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));
 
 	age_in_sec = (unsigned long)r->age * c->attrs.aggr_interval / 1000000;
 	for (age_in_log = 0; age_in_log < DAMON_MAX_AGE_IN_LOG && age_in_sec;
-- 
2.47.3

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

[sashiko-bot]

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

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

[Sasha Levin]

> Since commit 42f994b71404 ("mm/damon/core: implement scheme-specific apply
> interval"), DAMOS scheme can be applied at any time.  At that time,
> nr_accesses may not be fully aggregated.  But the quota prioritization
> score is calculated using the not fully aggregated count.

Queued for 6.12, thanks.

-- 
Thanks,
Sasha