[PATCH v2] selftests/mm: Drop duplicate test_seal_mprotect_two_vma_with_gap() call

Hongfu Li posted 1 patch 1 month, 3 weeks ago
tools/testing/selftests/mm/mseal_test.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
[PATCH v2] selftests/mm: Drop duplicate test_seal_mprotect_two_vma_with_gap() call
Posted by Hongfu Li 1 month, 3 weeks ago
From: Hongfu Li <lihongfu@kylinos.cn>

mseal_test main() invokes test_seal_mprotect_two_vma_with_gap() twice.
The second run repeats all assertions with no benefit. Drop the
duplicate call.

Signed-off-by: Hongfu Li <lihongfu@kylinos.cn>
Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
Acked-by: David Hildenbrand (Arm) <david@kernel.org>
---
v2:
- update ksft_set_plan(88) to 87 to match the actual number of tests
  after removing the duplicate call.
- Add Reviewed-by and Acked-by tags.
---
 tools/testing/selftests/mm/mseal_test.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/testing/selftests/mm/mseal_test.c b/tools/testing/selftests/mm/mseal_test.c
index 93c2e13094d4..1a05e6921fed 100644
--- a/tools/testing/selftests/mm/mseal_test.c
+++ b/tools/testing/selftests/mm/mseal_test.c
@@ -1876,7 +1876,7 @@ int main(void)
 	if (!pkey_supported())
 		ksft_print_msg("PKEY not supported\n");
 
-	ksft_set_plan(88);
+	ksft_set_plan(87);
 
 	test_seal_addseal();
 	test_seal_unmapped_start();
@@ -1913,7 +1913,6 @@ int main(void)
 	test_seal_mprotect_partial_mprotect(false);
 	test_seal_mprotect_partial_mprotect(true);
 
-	test_seal_mprotect_two_vma_with_gap();
 	test_seal_mprotect_two_vma_with_gap();
 
 	test_seal_mprotect_merge(false);
-- 
2.54.0
Re: [PATCH v2] selftests/mm: Drop duplicate test_seal_mprotect_two_vma_with_gap() call
Posted by Anshuman Khandual 1 month, 3 weeks ago
On Thu, Aug 06, 2026 at 11:08:50AM +0800, Hongfu Li wrote:
> From: Hongfu Li <lihongfu@kylinos.cn>
> 
> mseal_test main() invokes test_seal_mprotect_two_vma_with_gap() twice.
> The second run repeats all assertions with no benefit. Drop the
> duplicate call.

Guess these two instances would have been introduced just following the
adjacent other test cases where one takes true and the other takes false
as arguments :)

> 
> Signed-off-by: Hongfu Li <lihongfu@kylinos.cn>
> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> Acked-by: David Hildenbrand (Arm) <david@kernel.org>

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

> ---
> v2:
> - update ksft_set_plan(88) to 87 to match the actual number of tests
>   after removing the duplicate call.
> - Add Reviewed-by and Acked-by tags.
> ---
>  tools/testing/selftests/mm/mseal_test.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/mseal_test.c b/tools/testing/selftests/mm/mseal_test.c
> index 93c2e13094d4..1a05e6921fed 100644
> --- a/tools/testing/selftests/mm/mseal_test.c
> +++ b/tools/testing/selftests/mm/mseal_test.c
> @@ -1876,7 +1876,7 @@ int main(void)
>  	if (!pkey_supported())
>  		ksft_print_msg("PKEY not supported\n");
>  
> -	ksft_set_plan(88);
> +	ksft_set_plan(87);
>  
>  	test_seal_addseal();
>  	test_seal_unmapped_start();
> @@ -1913,7 +1913,6 @@ int main(void)
>  	test_seal_mprotect_partial_mprotect(false);
>  	test_seal_mprotect_partial_mprotect(true);
>  
> -	test_seal_mprotect_two_vma_with_gap();
>  	test_seal_mprotect_two_vma_with_gap();
>  
>  	test_seal_mprotect_merge(false);
> -- 
> 2.54.0
>
Re: [PATCH v2] selftests/mm: Drop duplicate test_seal_mprotect_two_vma_with_gap() call
Posted by SJ Park 1 month, 3 weeks ago
On Thu,  6 Aug 2026 11:08:50 +0800 Hongfu Li <hongfu.li@linux.dev> wrote:

> From: Hongfu Li <lihongfu@kylinos.cn>
> 
> mseal_test main() invokes test_seal_mprotect_two_vma_with_gap() twice.
> The second run repeats all assertions with no benefit. Drop the
> duplicate call.

Good eyes, thank you for fixing this!

> 
> Signed-off-by: Hongfu Li <lihongfu@kylinos.cn>
> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> Acked-by: David Hildenbrand (Arm) <david@kernel.org>

Reviewed-by: SJ Park <sj@kernel.org>

> ---
> v2:
> - update ksft_set_plan(88) to 87 to match the actual number of tests
>   after removing the duplicate call.

I wonder if there could be an automated way for this.  Obviously not a question
to the author but just my loud thought.


Thanks,
SJ

[...]
Re: [PATCH v2] selftests/mm: Drop duplicate test_seal_mprotect_two_vma_with_gap() call
Posted by Hongfu Li 1 month, 3 weeks ago
> > ---
> > v2:
> > - update ksft_set_plan(88) to 87 to match the actual number of tests
> >   after removing the duplicate call.
> 
> I wonder if there could be an automated way for this.  Obviously not a question
> to the author but just my loud thought.

Glad you brought this up, I totally agree with your thought.

Something like the below refactor could achieve that automation:

+typedef void (*test_fn)(void);
+typedef void (*test_fn_seal)(bool seal);
+
+static const struct {
+    test_fn_seal fn;
+    bool seal;
+} tests_seal[] = {
+    { test_seal_mprotect, false },
+    { test_seal_mprotect, true },
...
+};
+
+static const test_fn tests[] = {
+    test_seal_addseal,
+    test_seal_unmapped_start,
...
+};
+
+
 int main(void)
 {
...
-       ksft_set_plan(87);
+       ksft_set_plan(ARRAY_SIZE(tests) + ARRAY_SIZE(tests_seal));
+       for (size_t i = 0; i < ARRAY_SIZE(tests); i++)
+               tests[i]();
+       for (size_t i = 0; i < ARRAY_SIZE(tests_seal); i++)
+        tests_seal[i].fn(tests_seal[i].seal);

Though I'm still unsure whether this extra complexity is really worthwhile.
Re: [PATCH v2] selftests/mm: Drop duplicate test_seal_mprotect_two_vma_with_gap() call
Posted by SJ Park 1 month, 3 weeks ago
On Fri,  7 Aug 2026 15:15:33 +0800 Hongfu Li <hongfu.li@linux.dev> wrote:

> > > ---
> > > v2:
> > > - update ksft_set_plan(88) to 87 to match the actual number of tests
> > >   after removing the duplicate call.
> > 
> > I wonder if there could be an automated way for this.  Obviously not a question
> > to the author but just my loud thought.
> 
> Glad you brought this up, I totally agree with your thought.
> 
> Something like the below refactor could achieve that automation:
> 
> +typedef void (*test_fn)(void);
> +typedef void (*test_fn_seal)(bool seal);
> +
> +static const struct {
> +    test_fn_seal fn;
> +    bool seal;
> +} tests_seal[] = {
> +    { test_seal_mprotect, false },
> +    { test_seal_mprotect, true },
> ...
> +};
> +
> +static const test_fn tests[] = {
> +    test_seal_addseal,
> +    test_seal_unmapped_start,
> ...
> +};
> +
> +
>  int main(void)
>  {
> ...
> -       ksft_set_plan(87);
> +       ksft_set_plan(ARRAY_SIZE(tests) + ARRAY_SIZE(tests_seal));
> +       for (size_t i = 0; i < ARRAY_SIZE(tests); i++)
> +               tests[i]();
> +       for (size_t i = 0; i < ARRAY_SIZE(tests_seal); i++)
> +        tests_seal[i].fn(tests_seal[i].seal);

Thank you for sharing your great idea.

> 
> Though I'm still unsure whether this extra complexity is really worthwhile.

Agreed.  I believe it is good to have this as is for now.

If we have to add such automation, I think kselftest framework is a better
place to change, than each test.


Thanks,
SJ