Re: [RFC PATCH v5 1/8] mm/damon/core: introduce damos_quota_goal->complement

[SJ Park <sj@kernel.org>]

On Mon, 28 Sep 2026 09:44:40 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] mm/damon: integer overflow in damos_set_psi_current_val on 32-bit systems
> --
> 
> commit 2c98b9664c0f8cdd559874386842cae96a9ea8cb
> Author: SJ Park <sj@kernel.org>
> mm/damon/core: introduce damos_quota_goal->complement
> 
> This commit introduces the complement flag to damos_quota_goal, allowing the
> system to use the complemented value of a target metric. It adds the field
> to the data structure and implements support for it during quota goal commits
> and metric value retrievals.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 733025b367457..270ce9c85a2ce 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> 
> [ ... ]
> 
> > @@ -2960,10 +2961,18 @@ static void damos_set_psi_current_val(u64 now_psi_total,
> >  		struct damos_quota_goal *goal, struct damos *s)
> >  {
> >  	u64 last_psi_total = goal->last_psi_total;
> > +	unsigned long val;
> >  
> >  	goal->last_psi_total = now_psi_total;
> >  	if (last_psi_total != U64_MAX) {
> > -		goal->current_value = now_psi_total - last_psi_total;
> > +		val = now_psi_total - last_psi_total;
> > +		if (goal->complement) {
> > +			if (val < s->quota.reset_interval * 1000)
> > +				val = s->quota.reset_interval * 1000 - val;
> 
> [Severity: Low]
> Does this multiplication risk an integer overflow on 32-bit systems?
> 
> On 32-bit systems, unsigned long is 32 bits, with a maximum value of ~4.29
> billion. If a user configures reset_interval via sysfs to a value larger
> than 4,294,967 (about 71.5 minutes), the multiplication
> s->quota.reset_interval * 1000 will overflow unsigned long.
> 
> This causes the auto-tuning goal complement metric to evaluate with a
> truncated, incorrect value. While 32-bit systems running DAMON with 71+ 
> minute intervals are extremely rare, could this cause unexpected behavior
> when long intervals are configured?

Yes, that's extremely rare.  Also sane users would use the new feature after
testing.  Not a blocker of this series in my opinion.

> 
> > +			else
> > +				val = 0;
> > +		}
> > +		goal->current_value = val;
> >  		return;
> >  	}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260928093342.16584-1-sj@kernel.org?part=1
> 


Thanks,
SJ