Re: [PATCH v4] mm/damon/core: allow esz to be set to zero

[SJ Park <sj@kernel.org>]

On Tue, 08 Sep 2026 14:04:45 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Integer overflow in temporal goal tuner calculating `esz_bp` and `ULONG_MAX` capping severely limits DAMOS effective size quota on 32-bit systems.
> - [High] Integer overflow when calculating time quota constraints (`throughput * quota->ms`) on 32-bit systems limits time-based quota.
> --
> 
> commit 05338618157ff96b3145359ae173701a5ed4f699
> Author: Liew Rui Yan <aethernet65535@gmail.com>
> 
> mm/damon/core: allow esz to be set to zero
> 
> When the temporal quota goal tuner achieves its goal, it sets esz_bp to
> zero, but damos_set_effective_quota() previously clamped this to
> min_region_sz. This patch fixes this by keeping the effective size quota
> (esz) as minimum as possible when the goal is achieved.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index ce8c6f99106ed..3e9c9b6385d4e 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -3239,6 +3239,7 @@ static void damos_set_effective_quota(struct damon_ctx *ctx, struct damos *s)
> >  	struct damos_quota *quota = &s->quota;
> >  	unsigned long throughput;
> >  	unsigned long esz = ULONG_MAX;
> > +	unsigned long esz_time;
> >  
> >  	if (!quota->ms && list_empty(&quota->goals)) {
> >  		quota->esz = quota->sz;
> 
> [ ... ]
> 
> > @@ -3259,8 +3260,8 @@ static void damos_set_effective_quota(struct damon_ctx *ctx, struct damos *s)
> >  					1000000, quota->total_charged_ns);
> >  		else
> >  			throughput = PAGE_SIZE * 1024;
> > -		esz = min(throughput * quota->ms, esz);
> > -		esz = max(ctx->min_region_sz, esz);
> > +		esz_time = max(throughput * quota->ms, ctx->min_region_sz);
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does this inline multiplication
> wrap around on 32-bit systems?
> 
> Both throughput (calculated as bytes per millisecond) and quota->ms are
> unsigned long. On a 32-bit architecture, a realistic throughput like 2 MB/ms
> combined with a quota limit of 3000 ms yields a product of 6,000,000,000. 
> 
> Since this exceeds the 32-bit unsigned long limit of 4.29 billion, it silently
> wraps around to a significantly smaller size limit, which unintentionally bounds
> the effective quota and breaks the intended time-based logic.

We provide addr_unit for this kind of issues on 32 bit systems.  I will
separately work on this with low priority.

> 
> [Severity: High]
> This is a pre-existing issue, but I also noticed a similar overflow problem
> regarding the quota->esz_bp calculation in this same path.
> 
> When a user configures quota->sz via sysfs, it is scaled here:
> 
> mm/damon/core.c:damos_goal_tune_esz_bp_temporal() {
>     ...
> 		quota->esz_bp = quota->sz * 10000;
> 	else
> 		quota->esz_bp = ULONG_MAX;
>     ...
> }
> 
> If quota->sz is unset (0), esz_bp defaults to ULONG_MAX (4,294,967,295 on
> 32-bit systems). When this is later scaled down:
> 
> mm/damon/core.c:damos_set_effective_quota() {
>     ...
> 		esz = quota->esz_bp / 10000;
>     ...
> }
> 
> The resulting esz becomes 429,496 bytes (~419 KB). Does this artificially
> throttle DAMOS region actions on 32-bit platforms?
> 
> Additionally, if quota->sz is set larger than ~429 KB, quota->sz * 10000
> silently overflows the 32-bit unsigned long, creating a much smaller wrapped
> quota. Could we address this ULONG_MAX limitation and multiplication overflow
> on 32-bit architectures?

We provide addr_unit for this kind of issues on 32 bit systems.  I will
separately work on this with low priority.


Thanks,
SJ

[...]