Re: [PATCH 01/18] mm/damon/core: introduce damon_nr_accesses_mvsum()

[SJ Park <sj@kernel.org>]

On Tue, 30 Jun 2026 04:19:34 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Calculation of `left_window_bp` and moving sums in `damon_nr_accesses_mvsum()` and `damon_mvsum()` suffers from an integer overflow on 32-bit architectures when `window_len > 429,496`.
> --
> 
> commit 55e8315faa6caec232a35b71d630c4dd8cbb568f
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: introduce damon_nr_accesses_mvsum()
> 
> Introduce a new DAMON core function, damon_nr_accesses_mvsum(). It returns a
> pseudo moving sum value of a given region's nr_accesses for the last
> aggregation interval.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 027250e43c66f..fe4c470726d0b 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -208,6 +208,68 @@ static struct damon_probe *damon_nth_probe(int n, struct damon_ctx *ctx)
> >  	return NULL;
> >  }
> >  
> > +/*
> > + * damon_mvsum() - Returns pseudo moving sum value for a time window.
> 
> [ ... ]
> 
> > +static unsigned long damon_mvsum(unsigned long current_nr,
> > +		unsigned long last_nr, unsigned long left_window_bp)
> > +{
> > +	return current_nr + mult_frac(last_nr, left_window_bp, 10000);
> 
> [Severity: Medium]
> Could this mult_frac() calculation suffer from an integer overflow on 32-bit
> architectures?
> 
> If a user configures DAMON with a large aggr_interval and a small
> sample_interval such that window_len exceeds 429,496, last_nr could be
> equally large. Since unsigned long is 32 bits on those systems, can the
> internal multiplication inside mult_frac() silently overflow and result in
> incorrect moving sums?

Known low priority issue.  Not a blocker for this patch.

> 
> > +}
> > +
> > +/**
> > + * damon_nr_accesses_mvsum() - Returns moving sum access frequency score.
> 
> [ ... ]
> 
> > +unsigned int damon_nr_accesses_mvsum(struct damon_region *r,
> > +		struct damon_ctx *ctx)
> > +{
> > +	unsigned long sample_interval, aggr_interval;
> > +	unsigned long window_len, left_window, left_window_bp;
> > +
> > +	sample_interval = ctx->attrs.sample_interval ? : 1;
> > +	aggr_interval = ctx->attrs.aggr_interval ? : 1;
> > +	window_len = aggr_interval / sample_interval;
> > +	if (time_after_eq(ctx->passed_sample_intervals,
> > +				ctx->next_aggregation_sis))
> > +		left_window = 0;
> > +	else
> > +		left_window = ctx->next_aggregation_sis -
> > +			ctx->passed_sample_intervals;
> > +	left_window_bp = mult_frac(left_window, 10000, window_len);
> 
> [Severity: Medium]
> Does this calculation also overflow on 32-bit systems?
> 
> The mult_frac() macro calculates (left_window % window_len) * 10000. If
> left_window is greater than 429,496, multiplying by 10000 will exceed the
> maximum 32-bit unsigned long limit (4,294,967,295), causing a truncated
> left_window_bp result.
> 
> Should these calculations use 64-bit arithmetic, such as div64_ul(), to
> safely support large intervals on 32-bit systems?

Known low priority issue.  Not a blocker for this patch.


Thanks,
SJ

[...]