Re: [RFC PATCH v1.3 03/18] mm/damon/core: always update ->last_nr_accesses for intervals change

[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 integer overflow in `damon_accesses_bp_to_nr_accesses()` and `damon_nr_accesses_to_accesses_bp()` when interval ratios are large.
--

commit d0875d13ff723f48c4ed975ef9fd7e92fde65c11
Author: SeongJae Park <sj@kernel.org>

mm/damon/core: always update ->last_nr_accesses for intervals change

This commit ensures the last_nr_accesses value in a damon_region is always
updated when intervals change to prevent stale values from causing incorrect
calculations. It updates damon_update_monitoring_result() to unconditionally
scale last_nr_accesses using damon_nr_accesses_for_new_attrs().

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index addd14e4e70bf..bc40a6d10f277 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -873,6 +873,8 @@ static void damon_update_monitoring_result(struct damon_region *r,
>  		struct damon_attrs *old_attrs, struct damon_attrs *new_attrs,
>  		bool aggregating)
>  {
> +	r->last_nr_accesses = damon_nr_accesses_for_new_attrs(
> +			r->last_nr_accesses, old_attrs, new_attrs);

[Severity: High]
This is a pre-existing issue, but does this calculation in the underlying
functions risk a 32-bit integer overflow?

The new call to damon_nr_accesses_for_new_attrs() relies on
damon_accesses_bp_to_nr_accesses() and damon_nr_accesses_to_accesses_bp().

Looking at damon_accesses_bp_to_nr_accesses():

mm/damon/core.c:damon_accesses_bp_to_nr_accesses() {
    ...
    return accesses_bp * damon_max_nr_accesses(attrs) / 10000;
}

If a user configures a small sample_interval (e.g., 5ms) and a large
aggr_interval (e.g., 3600s), the ratio calculated by damon_max_nr_accesses()
is 720,000. For an accesses_bp of 10000 (100%), the multiplication yields
7,200,000,000, which exceeds the 32-bit unsigned int maximum and silently
truncates.

Similarly, in damon_nr_accesses_to_accesses_bp():

mm/damon/core.c:damon_nr_accesses_to_accesses_bp() {
    ...
    return mult_frac(nr_accesses, 10000, damon_max_nr_accesses(attrs));
}

The mult_frac macro uses the type of nr_accesses (unsigned int) for its
internal calculations. Can this cause a similar 32-bit overflow when
multiplying the remainder by 10000 if the remainder exceeds 429,496?

>  	if (!aggregating) {
>  		r->nr_accesses = damon_nr_accesses_for_new_attrs(
>  				r->nr_accesses, old_attrs, new_attrs);

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