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

[sashiko-bot@kernel.org]

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?

> +		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