Re: [PATCH 6.12.y] mm/damon/core: disallow time-quota setting zero esz

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The patch masks a severe pre-existing integer overflow in the time quota throughput calculation, which permanently throttles the scheme instead of fixing the root cause.
- [Medium] The patch forces the effective size quota to be at least DAMON_MIN_REGION when a time quota is configured, breaking the ability of quota goals to intentionally pause the scheme.
--

commit 648433abd4b11fd3f4a77018e7e47105abac1ce7
Author: SeongJae Park <sj@kernel.org>

mm/damon/core: disallow time-quota setting zero esz

When the throughput of a DAMOS scheme is very slow, the DAMOS time quota can
make the effective size quota smaller than the minimum region size, preventing
actions from being applied. This commit fixes the issue by setting the time
quota-modified effective size quota to at least DAMON_MIN_REGION.

Link: https://lore.kernel.org/20260407003153.79589-1-sj@kernel.org
Link: https://lore.kernel.org/20260405192504.110014-1-sj@kernel.org [1]

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index ed2b750231810..69f8244324b36 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -1577,6 +1577,7 @@ static void damos_set_effective_quota(struct damos_quota *quota)
>  			esz = min(throughput * quota->ms, esz);
>  		else
>  			esz = throughput * quota->ms;
> +		esz = max(DAMON_MIN_REGION, esz);

Does this cause a regression by breaking the ability of quota goals to
intentionally pause the scheme?

If a dynamic quota goal determines that actions should be paused and requests
an esz smaller than DAMON_MIN_REGION, this max() macro forcibly overrides it
and bumps the quota back to DAMON_MIN_REGION.

This causes the scheme to process at least 1 page per charge window instead
of fully pausing.

Could the max() boundary be applied exclusively to the throughput-derived quota
before it is min()'ed with the goal-derived quota?

Additionally, does this code mask a pre-existing integer overflow regression
in the throughput calculation?

Looking earlier in damos_set_effective_quota():

	if (quota->total_charged_ns)
		throughput = quota->total_charged_sz * 1000000 /
			quota->total_charged_ns;

Because total_charged_sz and total_charged_ns are continuously accumulated
over the scheme's lifetime, total_charged_sz * 1000000 will overflow an
unsigned long on 64-bit systems after processing around 18.4 TB of memory.

When the numerator overflows, it wraps modulo 2^64 while total_charged_ns
continues to grow. This causes the calculated throughput to collapse to
near-zero.

Also, on 32-bit systems, total_charged_ns stores nanoseconds and overflows
a 32-bit unsigned long after 4.29 seconds of accumulated action time, causing
erratic throughput calculations.

By clamping the output to DAMON_MIN_REGION here, the patch seems to permanently
throttle the scheme to process exactly 1 page per charge window once the
overflow occurs, rather than fixing the underlying throughput collapse.

>  	}
>  
>  	if (quota->sz && quota->sz < esz)

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