Re: [RFC PATCH 5/6] mm/damon/core: update probe hits for new parameter commit

[SJ Park <sj@kernel.org>]

On Tue, 07 Jul 2026 04:51:51 +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 arithmetic overflow in interval scaling functions corrupts core metrics.
> --
> 
> --- Patch [5]: [RFC PATCH 5/6] mm/damon/core: update probe hits for new parameter commit ---
> commit 7e26d9c01ec3d36d75d519b11f48ef36f78ec868
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: update probe hits for new parameter commit
> 
> Users can update DAMON parameters at runtime. If the samples and/or
> aggregation intervals are updated, monitoring results depending on the
> intervals should also be updated. This commit ensures probe_hits are
> properly updated to reflect new intervals, similar to age and nr_accesses.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index cda318806b65b..9a3ff0fe55a61 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> [ ... ]
> > @@ -898,10 +898,32 @@ static unsigned int damon_nr_samples_for_new_attrs(unsigned int nr_accesses,
> >  			new_attrs);
> >  }
> >  
> > +static void damon_update_probe_hits(struct damon_region *r,
> > +		struct damon_attrs *old_attrs, struct damon_attrs *new_attrs,
> > +		bool aggregating, struct damon_ctx *ctx)
> > +{
> > +	struct damon_probe *p;
> > +	int i = 0;
> > +
> > +	damon_for_each_probe(p, ctx) {
> > +		r->last_probe_hits[i] = damon_nr_samples_for_new_attrs(
> > +				r->last_probe_hits[i], old_attrs, new_attrs);
> 
> [Severity: High]
> This is a pre-existing issue, but can the scaling logic called here
> overflow and corrupt the core metrics?
> 
> Looking at the underlying functions called by
> damon_nr_samples_for_new_attrs():
> 
> mm/damon/core.c:damon_sample_bp_to_count() {
>     return bp * damon_nr_samples_per_aggr(attrs) / 10000;
> }
> 
> and
> 
> mm/damon/core.c:damon_sample_count_to_bp() {
>     return mult_frac(count, 10000, damon_nr_samples_per_aggr(attrs));
> }
> 
> If a user configures a large aggregation to sampling interval ratio
> (e.g., 1 second aggregation and 1 microsecond sampling), the ratio can
> exceed 429K. 
> 
> When damon_sample_count_to_bp() scales a count of 500,000, the internal
> multiplication in mult_frac() (count * 10000) evaluates to 5,000,000,000,
> which overflows the 32-bit unsigned integer maximum and yields an incorrect
> value.
> 
> Similarly, damon_sample_bp_to_count() performs 32-bit multiplication
> which can overflow if bp * samples exceeds the 32-bit limit.
> 
> Should these scaling calculations cast to 64-bit to prevent silent
> wrap-around and wildly inaccurate statistics?

Known low priority issue.

> 
> > +		if (!aggregating)
> > +			r->probe_hits[i] = damon_nr_samples_for_new_attrs(
> > +					r->probe_hits[i], old_attrs,
> > +					new_attrs);
> > +		else
> > +			r->probe_hits[i] = 0;
> > +		i++;
> > +	}
> > +}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260707043828.97900-1-sj@kernel.org?part=5


Thanks,
SJ

[...]