Re: [RFC PATCH v1.2 4/7] mm/damon/core: s/nr_accesses_to_accesses_bp/sample_count_to_bp/

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] The patch series introduces a build failure when `CONFIG_DAMON_KUNIT_TEST=y`. An unused test function references a removed function `damon_max_nr_accesses()`.

Pre-existing issues:
- [High] 32-bit integer overflows in `damon_sample_bp_to_count()` and `damon_sample_count_to_bp()` can yield wildly incorrect sample counts when large aggregation intervals are used.
--

--- Patch [4]: [RFC PATCH v1.2 4/7] mm/damon/core: s/nr_accesses_to_accesses_bp/sample_count_to_bp/ ---
commit ea8982f99be13edf95a6b08dabc871567edc1f0c
Author: SJ Park <sj@kernel.org>

mm/damon/core: s/nr_accesses_to_accesses_bp/sample_count_to_bp/

damon_nr_accesses_to_accesses_bp() actually converts a positive sample
count to the ratio.  Rename it to better describe what it really does
and not confusing for more general uses.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index dea63083917b0..718268df27fa7 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -883,19 +883,18 @@ static unsigned int damon_sample_bp_to_count(
>  	return bp * damon_nr_samples_per_aggr(attrs) / 10000;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but could this calculation
overflow and truncate before division when the product exceeds
4,294,967,295?

If DAMON is configured with a high aggr_interval relative to
sample_interval, nr_samples can be large. If a region is heavily
accessed, the sample count increases. Using 32-bit unsigned integer
arithmetic here might cause silent truncation, leading to hot pages
being misclassified as cold, and causing aggressive memory reclamation
of the active working set.

> -/* Convert nr_accesses to access ratio in bp (per 10,000) */
> -static unsigned int damon_nr_accesses_to_accesses_bp(
> -		unsigned int nr_accesses, struct damon_attrs *attrs)
> +/* convert sample count to ratio in bp (per 10,000) */
> +static unsigned int damon_sample_count_to_bp(
> +		unsigned int count, struct damon_attrs *attrs)
>  {
> -	return mult_frac(nr_accesses, 10000, damon_nr_samples_per_aggr(attrs));
> +	return mult_frac(count, 10000, damon_nr_samples_per_aggr(attrs));
>  }

[Severity: High]
This is a pre-existing issue, but does mult_frac() also suffer from a
32-bit integer overflow?

mult_frac() expands to expressions that internally compute the remainder
multiplied by 10000 using 32-bit unsigned arithmetic. If the sample
counts and aggregation intervals are large enough (above 429,496), could
this overflow?


[Severity: Low]
Additionally, it looks like an earlier patch in this series removed
damon_max_nr_accesses(), but left references to it in
mm/damon/tests/core-kunit.h:damon_test_max_nr_accesses().

Does compiling the kernel with CONFIG_DAMON_KUNIT_TEST=y fail due to an
undeclared function error?

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