Re: [PATCH 6.1.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] Unmitigated integer overflow in the `throughput` calculation permanently breaks time quotas.
- [High] Incorrect initialization of `charged_from` completely bypasses quotas for the first 5 minutes of system boot.
--

commit 558d8bfd17c7ba00954bbbf18a4eb2783d713b59
Author: SeongJae Park <sj@kernel.org>
mm/damon/core: disallow time-quota setting zero esz

This commit addresses an issue where DAMOS time quotas could make the
effective size quota smaller than the minimum region size. By forcing
the effective size quota to be at least DAMON_MIN_REGION, it prevents
schemes from being unexpectedly deactivated.

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 ab5c351b276ce..94f8450f6e843 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -802,6 +802,7 @@ static void damos_set_effective_quota(struct damos_quota *quota)
>  	else
>  		throughput = PAGE_SIZE * 1024;
>  	esz = throughput * quota->ms;
> +	esz = max(DAMON_MIN_REGION, esz);

Can the throughput calculation overflow here?

Looking at damos_set_effective_quota() before this change, throughput is
calculated using:

    throughput = quota->total_charged_sz * 1000000 / quota->total_charged_ns;

Both total_charged_sz and total_charged_ns accumulate indefinitely over the
scheme's lifetime. The intermediate multiplication by 1000000 is performed
in unsigned long precision.

Could this overflow after a large amount of memory is processed? For
instance, it could wrap around after around 18.4 TB on 64-bit systems, or
just 4KB on 32-bit systems where unsigned long is 32-bit.

When the multiplication wraps around, the numerator becomes artificially
small, causing the computed throughput to collapse to near-zero. While
clamping esz to DAMON_MIN_REGION prevents total deactivation, does the
overflow permanently trap the scheme into processing exactly one minimal
region per charge window?

Also related to the effective size quota updates, is there a regression
during early system boot?

Looking at damos_quota_init_priv(), quota->charged_from is initialized
to 0 rather than the current jiffies.

In kdamond_apply_schemes(), the charge window only starts when:

    time_after_eq(jiffies, quota->charged_from + interval)

Because jiffies starts at INITIAL_JIFFIES (which casts to a negative long,
typically -300*HZ), does time_after_eq() fail for the first 5 minutes of
system uptime?

During this 5-minute period, damos_set_effective_quota() would never be
invoked, leaving quota->esz at its default initialized value of 0.

Because quota->esz == 0 is treated as no limit by the scheme application
checks, do early-boot schemes (like damon_reclaim) run entirely
unthrottled for the first 5 minutes, completely ignoring user-configured
CPU and size quotas?

>  
>  	if (quota->sz && quota->sz < esz)
>  		esz = quota->sz;

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