Re: [RFC PATCH v1.1 2/7] mm/damon/core: s/damon_max_nr_accesses()/damon_nr_samples_per_aggr()/

[SJ Park <sj@kernel.org>]

On Tue, 07 Jul 2026 13:58:22 +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] Multiplications by 10000 in DAMON basis-point conversion functions overflow 32-bit unsigned integers when the number of samples per aggregation interval is large.
> --
> 
> --- Patch [2]: [RFC PATCH v1.1 2/7] mm/damon/core: s/damon_max_nr_accesses()/damon_nr_samples_per_aggr()/ ---
> commit f63888911d3e9e966f4abf35c7d87cb96cc0e08b
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: s/damon_max_nr_accesses()/damon_nr_samples_per_aggr()/
> 
> This commit renames damon_max_nr_accesses() to damon_nr_samples_per_aggr()
> to better describe that it returns the number of samples DAMON checks for
> each region per aggregation interval.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index a15005db82899..177e5abc981ef 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -880,14 +880,14 @@ static unsigned int damon_age_for_new_attrs(unsigned int age,
> >  static unsigned int damon_accesses_bp_to_nr_accesses(
> >  		unsigned int accesses_bp, struct damon_attrs *attrs)
> >  {
> > -	return accesses_bp * damon_max_nr_accesses(attrs) / 10000;
> > +	return accesses_bp * damon_nr_samples_per_aggr(attrs) / 10000;
> >  }
> 
> [Severity: High]
> This is a pre-existing issue, but does this calculation overflow 32-bit
> unsigned integers when the number of samples per aggregation interval is
> large?
> 
> In damon_accesses_bp_to_nr_accesses() in mm/damon/core.c, both accesses_bp
> and the return value of damon_nr_samples_per_aggr() are 32-bit unsigned
> integers. If an administrator configures DAMON with a large ratio between
> aggr_interval and sample_interval (e.g., > 429,496, achievable via sysfs
> with a 1us sample and 500ms aggregation), the multiplication will silently
> overflow UINT_MAX, yielding much smaller thresholds than intended.

Known low priority issue.

> 
> >  /* Convert nr_accesses to access ratio in bp (per 10,000) */
> >  static unsigned int damon_nr_accesses_to_accesses_bp(
> >  		unsigned int nr_accesses, struct damon_attrs *attrs)
> >  {
> > -	return mult_frac(nr_accesses, 10000, damon_max_nr_accesses(attrs));
> > +	return mult_frac(nr_accesses, 10000, damon_nr_samples_per_aggr(attrs));
> >  }
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but can this macro expansion also
> suffer from a 32-bit overflow?
> 
> The mult_frac() macro adopts the type of its first argument, nr_accesses,
> which is an unsigned int. Its internal calculation of the remainder
> multiplied by the numerator (r * n_) evaluates as an unsigned int.
> 
> When the remainder r (which is bounded by samples_per_aggr) exceeds 429,496,
> this multiplication by 10000 will overflow 32-bit boundaries, leading to
> incorrect basis-point ratio calculations.

Known low priority issue.

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



Thanks,
SJ