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

[SeongJae Park <sj@kernel.org>]

On Mon, 04 May 2026 18:10:48 +0000 sashiko-bot@kernel.org wrote:

> 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.

No real issues for this patch.

> --
> 
> 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?

Good catch.  But not this patch's fault.  On 32bit, use of addr_unit would
mitigate the issue so that reasonable amount of progress can be made.  Also,
the impact should not be significant.

> 
> 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?

We initialize charged_from as jiffies for every charge window, on
damos_adjust_quota().  So the issue shouldn't exist.


Thanks,
SJ

[...]