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

[SJ Park <sj@kernel.org>]

On Sat,  5 Sep 2026 22:14:09 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:

> On Sat, 05 Sep 2026 12:58:25 +0000 sashiko-bot@kernel.org wrote:
> 
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > 
> > Pre-existing issues:
> > - [High] 32-bit integer overflows in time quota and throughput calculations lead to severe miscalculations on 32-bit architectures.
> > --
> > 
> > commit dc1c35769ce920ee1796367097f9876b2f24a643
> > Author: Liew Rui Yan <aethernet65535@gmail.com>
> > 
> > mm/damon/core: allow esz to be set to zero
> > 
> > This commit modifies damos_set_effective_quota() to allow the effective
> > size to be set to zero when the goal has been achieved. It changes the
> > order of max and min calculations when quota->ms is set to ensure
> > that a zero esz_bp results in a minimal effective size instead of being
> > clamped to min_region_sz.
> > 
> > > 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
> > 
> > [ ... ]
> > 
> > > @@ -3259,8 +3260,8 @@ static void damos_set_effective_quota(struct damon_ctx *ctx, struct damos *s)
> > >  					1000000, quota->total_charged_ns);
> > 
> > [Severity: High]
> > This is a pre-existing issue, but does mult_frac() silently overflow on
> > 32-bit architectures here in damos_set_effective_quota()?
> > 
> > Since quota->total_charged_ns is represented as an unsigned long, it natively
> > overflows after 4.29 seconds on 32-bit systems. Furthermore, when
> > calculating throughput via mult_frac(), the remainder calculation
> > (sz % ns) * 1000000 can silently overflow a 32-bit unsigned long, which
> > causes throughput estimation to be completely incorrect and often near zero.
> > 
> > >  		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 is also a pre-existing issue, but can throughput * quota->ms overflow 32
> > bits for quotas over a few seconds here in damos_set_effective_quota()?
> > 
> > If this wraps around on 32-bit architectures, it improperly clamps the
> > effective size to ctx->min_region_sz, nullifying the time quota feature.
> > 
> > > +		esz = min(esz_time, esz);
> > >  	}
> > >  
> > >  	if (quota->sz && quota->sz < esz)
> > 
> > -- 
> > Sashiko AI review · https://sashiko.dev/#/patchset/20260905124706.3398-1-aethernet65535@gmail.com?part=1
> 
> Thank you for pointing those out!  I will fix those separately.

These are known low priority issue that I'm working on.  Let's not duplicate
works.


Thanks,
SJ

[...]