Re: [RFC PATCH v1.2 5/7] mm/damon/core: s/nr_accesses_for_new_attrs/nr_samples_for_new_attrs/

[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 in the DAMON KUnit tests by leaving behind a broken, unreferenced test function.

Pre-existing issues:
- [High] Functions `damon_sample_bp_to_count` and `damon_sample_count_to_bp` silently overflow their 32-bit unsigned integer calculations when the number of samples per aggregation interval is large.
--

--- Patch [5]: [RFC PATCH v1.2 5/7] mm/damon/core: s/nr_accesses_for_new_attrs/nr_samples_for_new_attrs/ ---
commit a57666ff806c37c47b5fa8f1ce4de7e70616ac6d
Author: SJ Park <sj@kernel.org>

mm/damon/core: s/nr_accesses_for_new_attrs/nr_samples_for_new_attrs/
    
This commit renames damon_nr_accesses_for_new_attrs() to
damon_nr_samples_for_new_attrs() to clarify that it can be used for any
positive sample count, not just nr_accesses.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 718268df27fa7..800ff6b6d2473 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -890,22 +890,21 @@ static unsigned int damon_sample_count_to_bp(
>  	return mult_frac(count, 10000, damon_nr_samples_per_aggr(attrs));
>  }

[Severity: High]
This isn't a bug introduced by this patch, but renaming this function
propagates a flawed arithmetic calculation.

Can the 32-bit unsigned integer calculations in damon_sample_count_to_bp()
and damon_sample_bp_to_count() silently overflow?

If a user configures a large aggregation interval and small sample interval,
the total samples per aggregation can exceed 429,496, leading to 32-bit
arithmetic overflow during ratio conversion.

In damon_sample_count_to_bp():

    mult_frac(count, 10000, damon_nr_samples_per_aggr(attrs));

The mult_frac macro expands to perform a 32-bit multiplication of count
(or its remainder) by 10000, which overflows for counts exceeding 429,496.

Similarly, in damon_sample_bp_to_count() (just above this context):

    return bp * damon_nr_samples_per_aggr(attrs) / 10000;

The multiplication is performed in 32-bit unsigned integer arithmetic, which
overflows if the total samples per aggregation is large.

>  
> -static unsigned int damon_nr_accesses_for_new_attrs(unsigned int nr_accesses,
> +static unsigned int damon_nr_samples_for_new_attrs(unsigned int nr,
>  		struct damon_attrs *old_attrs, struct damon_attrs *new_attrs)
>  {
>  	return damon_sample_bp_to_count(
> -			damon_sample_count_to_bp(nr_accesses, old_attrs),
> -			new_attrs);
> +			damon_sample_count_to_bp(nr, old_attrs), new_attrs);
>  }
>  

[ ... ]

[Severity: Low]
Does this patch series leave behind a broken, unreferenced test function
in the DAMON KUnit tests?

When building the kernel with DAMON KUnit tests enabled, this causes a build
failure due to an implicit function declaration.

In mm/damon/tests/core-kunit.h:damon_test_max_nr_accesses():

    KUNIT_EXPECT_EQ(test, damon_max_nr_accesses(attrs), 1);

The function calls damon_max_nr_accesses(), which was renamed in an earlier
patch in this series but was left unupdated in this abandoned test function.

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