Re: [RFC PATCH v2 8/8] mm/damon/core-kunit: test invalid damos filter commits

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incomplete implementation: missing test coverage for DAMOS_FILTER_TYPE_PROBE_HITS_WSUM invalid commits.
--

commit d95c459dddaa5a1cc3be54d53b10942b9bd9ba9f
Author: SJ Park <sj@kernel.org>

mm/damon/core-kunit: test invalid damos filter commits

This commit adds KUnit test cases to verify the validation of damos filter
arguments at commit time.

> diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> index 2b0931cf6fb32..527abc2570616 100644
> --- a/mm/damon/tests/core-kunit.h
> +++ b/mm/damon/tests/core-kunit.h
> @@ -1184,6 +1184,13 @@ static void damos_test_commit_filter(struct kunit *test)
>  			.allow = false,
>  			.sz_range = {.min = 234, .max = 345},
>  			}, false);
> +	damos_test_commit_filter_for(test, &dst,
> +			&(struct damos_filter){
> +			.type = DAMOS_FILTER_TYPE_HUGEPAGE_SIZE,
> +			.matching = false,
> +			.allow = false,
> +			.sz_range = {.min = 456, .max = 123},
> +			}, true);
>  	damos_test_commit_filter_for(test, &dst,
>  			&(struct damos_filter){
>  			.type = DAMOS_FILTER_TYPE_UNMAPPED,
> @@ -1197,6 +1204,13 @@ static void damos_test_commit_filter(struct kunit *test)
>  			.allow = false,
>  			.addr_range = {.start = 456, .end = 567},
>  			}, false);
> +	damos_test_commit_filter_for(test, &dst,
> +			&(struct damos_filter){
> +			.type = DAMOS_FILTER_TYPE_ADDR,
> +			.matching = false,
> +			.allow = false,
> +			.addr_range = {.start = 567, .end = 456},
> +			}, true);

[Severity: Medium]
Is a test case for DAMOS_FILTER_TYPE_PROBE_HITS_WSUM missing here?

Previous changes introduced range validation (max < min) for ADDR,
HUGEPAGE_SIZE, and PROBE_HITS_WSUM. While ADDR and HUGEPAGE_SIZE are being
tested for invalid commits, PROBE_HITS_WSUM seems to be omitted.

Additionally, if a test for PROBE_HITS_WSUM is added, does
damos_test_commit_filter_for() also need an update? The switch statement
verifying copied filter fields lacks a case for it and would fall through to
default, failing to verify the range arguments:

damos_test_commit_filter_for() {
	...
	switch (src->type) {
	case DAMOS_FILTER_TYPE_MEMCG:
		...
	case DAMOS_FILTER_TYPE_HUGEPAGE_SIZE:
		...
	default:
		break;
	}
}

>  	damos_test_commit_filter_for(test, &dst,
>  			&(struct damos_filter){
>  			.type = DAMOS_FILTER_TYPE_TARGET,

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