Re: [RFC PATCH 07/10] mm/damon/tests/core-kunit: test damon_nr_samples_per_aggr()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added `damon_test_nr_samples_per_aggr` test uses tautological assertions that will always pass, failing to detect the regressions it was intended to prevent. Additionally, the test setup overflows to zero on 32-bit architectures.
--

--- Patch [7]: [RFC PATCH 07/10] mm/damon/tests/core-kunit: test damon_nr_samples_per_aggr() ---
commit 50a7320dda40ca169bc23924caa1421599f78314
Author: SJ Park <[email protected]>

mm/damon/tests/core-kunit: test damon_nr_samples_per_aggr()

This commit adds a kunit test for damon_nr_samples_per_aggr() to ensure
that extreme interval setups are handled properly without returning zero
or random overflowed values.

> diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> index 3041b8288ec26..511f2856d343c 100644
> --- a/mm/damon/tests/core-kunit.h
> +++ b/mm/damon/tests/core-kunit.h

[ ... ]

> +static void damon_test_nr_samples_per_aggr(struct kunit *test)
> +{
> +	struct damon_attrs attrs = {
> +		.sample_interval = 0,
> +		.aggr_interval = 0,
> +	};
> +
> +	/* Zero aggregation interval doesn't cause division by zero */
> +	KUNIT_EXPECT_EQ(test, damon_nr_samples_per_aggr(&attrs), 1);
> +
> +	/* Too large aggregation interval doesn't cause overflow */
> +	attrs.aggr_interval = (unsigned long)UINT_MAX + 1;

[Severity: Medium]
On 32-bit systems where unsigned long is 32-bits wide, does this assignment
overflow to 0?

If it overflows, the test would configure an interval of 0 instead of a value
larger than UINT_MAX, failing to test the large-interval scenario.

> +	KUNIT_EXPECT_GE(test, damon_nr_samples_per_aggr(&attrs), 0);
> +	KUNIT_EXPECT_LE(test, damon_nr_samples_per_aggr(&attrs), UINT_MAX);
> +}

[Severity: Medium]
Since damon_nr_samples_per_aggr() returns an unsigned int, are these
assertions tautological?

An unsigned int is inherently always >= 0 and <= UINT_MAX. This means the
test will pass regardless of the function's return value, and it wouldn't
catch if the function unexpectedly returned 0.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7