Re: [RFC PATCH 1/6] mm/damon/core: s/damon_max_nr_accesses()/damon_nr_samples_per_aggr()/

[SJ Park <sj@kernel.org>]

On Tue, 07 Jul 2026 04:48:40 +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] Multiplication overflow in DAMON access rate calculations due to 32-bit arithmetic.
> --
> 
> --- Patch [1]: [RFC PATCH 1/6] mm/damon/core: s/damon_max_nr_accesses()/damon_nr_samples_per_aggr()/ ---
> commit 928ef68b58766df0da13e507ad449f7ba9d5c79d
> 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 390e00b3685ef..e710e2789085e 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -880,7 +880,7 @@ 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 isn't a bug introduced by this patch, but can this multiplication in
> damon_accesses_bp_to_nr_accesses() overflow 32-bit arithmetic?
> 
> If the sample interval is configured to a small value and the aggregation
> interval to a large value, damon_nr_samples_per_aggr() can return a large
> number. Multiplying accesses_bp by this large number wraps around modulo
> 2^32 since both operands are 32-bit unsigned integers, resulting in an
> incorrectly reduced access count.

Known low priority issue.

> 
> >  }
> >  
> >  /*
> > @@ -893,7 +893,7 @@ static unsigned int damon_accesses_bp_to_nr_accesses(
> >  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 is a pre-existing issue, but does the use of mult_frac() in
> damon_nr_accesses_to_accesses_bp() also suffer from a 32-bit overflow?
> 
> When nr_accesses is sufficiently large, the internal calculation in
> mult_frac() overflows 32-bit arithmetic.

Known low priority issue.

> 
> >  }
> >  
> 
> [ ... ]
> 
> > diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
> > index e8c389ad3226f..7e077084cb03a 100644
> > --- a/mm/damon/lru_sort.c
> > +++ b/mm/damon/lru_sort.c
> > @@ -303,7 +303,7 @@ static int damon_lru_sort_apply_parameters(void)
> >  		goto out;
> >  
> >  	err = -ENOMEM;
> > -	hot_thres = damon_max_nr_accesses(&attrs) *
> > +	hot_thres = damon_nr_samples_per_aggr(&attrs) *
> >  		hot_thres_access_freq / 1000;
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does this calculation in
> damon_lru_sort_apply_parameters() overflow on 32-bit architectures?
> 
> Multiplying the sample count by hot_thres_access_freq overflows 32-bit
> arithmetic on 32-bit systems when the sample count is large.

Known low priority issue.

> 
> >  	hot_scheme = damon_lru_sort_new_hot_scheme(hot_thres);
> >  	if (!hot_scheme)
> 
> [ ... ]
> 
> > diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
> > index 6bdd1cfd3863a..89c694f6e4fa8 100644
> > --- a/mm/damon/ops-common.c
> > +++ b/mm/damon/ops-common.c
> > @@ -112,7 +112,7 @@ int damon_hot_score(struct damon_ctx *c, struct damon_region *r,
> >  	int hotness;
> >  
> >  	freq_subscore = r->nr_accesses * DAMON_MAX_SUBSCORE /
> > -		damon_max_nr_accesses(&c->attrs);
> > +		damon_nr_samples_per_aggr(&c->attrs);
> 
> [Severity: High]
> This is a pre-existing issue, but could this calculation in
> damon_hot_score() result in a 32-bit overflow?
> 
> Multiplying r->nr_accesses by DAMON_MAX_SUBSCORE (which is 100) overflows
> 32-bit arithmetic if the number of samples is near UINT_MAX. This might
> cause hot memory to be incorrectly identified as cold.

Known low priority issue.

> 
> >  
> >  	age_in_sec = (unsigned long)r->age * c->attrs.aggr_interval / 1000000;
> >  	if (age_in_sec)
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260707043828.97900-1-sj@kernel.org?part=1


Thanks,
SJ

[...]