mbox series

[0/7] selftests: vDSO: Some cleanups and fixes

Message ID 20250502-selftests-vdso-fixes-v1-0-fb5d640a4f78@linutronix.de
Headers show
Series selftests: vDSO: Some cleanups and fixes | expand

Message

Thomas Weißschuh May 2, 2025, 12:40 p.m. UTC
Fixes and cleanups for various issues in the vDSO selftests.

Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
Thomas Weißschuh (7):
      selftests: vDSO: chacha: Correctly skip test if necessary
      selftests: vDSO: clock_getres: Drop unused include of err.h
      selftests: vDSO: vdso_test_correctness: Fix -Wold-style-definitions
      selftests: vDSO: vdso_test_getrandom: Drop unused include of linux/compiler.h
      selftests: vDSO: vdso_test_getrandom: Drop some dead code
      selftests: vDSO: vdso_test_getrandom: Always print TAP header
      selftests: vDSO: vdso_config: Avoid -Wunused-variables

 tools/testing/selftests/vDSO/vdso_config.h            |  2 ++
 tools/testing/selftests/vDSO/vdso_test_chacha.c       |  3 ++-
 tools/testing/selftests/vDSO/vdso_test_clock_getres.c |  1 -
 tools/testing/selftests/vDSO/vdso_test_correctness.c  |  2 +-
 tools/testing/selftests/vDSO/vdso_test_getrandom.c    | 18 +++++-------------
 5 files changed, 10 insertions(+), 16 deletions(-)
---
base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
change-id: 20250423-selftests-vdso-fixes-d2ce74142359

Best regards,

Comments

Muhammad Usama Anjum May 2, 2025, 1:42 p.m. UTC | #1
On 5/2/25 5:40 PM, Thomas Weißschuh wrote:
> According to kselftest.h ksft_exit_skip() is not meant to be called when
> a plan has already been printed.
Agreed

> 
> Use the recommended function ksft_test_result_skip().
> 
> This fixes a bug, where the TAP output would be invalid when skipping:
> 
> 	TAP version 13
> 	1..1
> 	ok 2 # SKIP Not implemented on architecture
> 
> The SKIP line should start with "ok 1" as the plan only contains one test.
> 
> Fixes: 3b5992eaf730 ("selftests: vDSO: unconditionally build chacha test")
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>

> 
> ---
> I'm not sure if this is not a general bug in ksft_exit_skip().
> First ksft_xskip is incremented then read back through ksft_test_num() and
> then that result is incremented again.
> 
> In any case, using the correct function is better.
> ---
>  tools/testing/selftests/vDSO/vdso_test_chacha.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/vDSO/vdso_test_chacha.c b/tools/testing/selftests/vDSO/vdso_test_chacha.c
> index 8757f738b0b1a76a48c83c5e5df79925a30c1bc7..0aad682b12c8836efabb49a65a47cf87466891a3 100644
> --- a/tools/testing/selftests/vDSO/vdso_test_chacha.c
> +++ b/tools/testing/selftests/vDSO/vdso_test_chacha.c
> @@ -76,7 +76,8 @@ static void reference_chacha20_blocks(uint8_t *dst_bytes, const uint32_t *key, u
>  
>  void __weak __arch_chacha20_blocks_nostack(uint8_t *dst_bytes, const uint32_t *key, uint32_t *counter, size_t nblocks)
>  {
> -	ksft_exit_skip("Not implemented on architecture\n");
> +	ksft_test_result_skip("Not implemented on architecture\n");
> +	ksft_finished();
>  }
>  
>  int main(int argc, char *argv[])
>
Muhammad Usama Anjum May 2, 2025, 1:44 p.m. UTC | #2
On 5/2/25 5:40 PM, Thomas Weißschuh wrote:
> Functions definitions without any argument list produce a warning with
> -Wold-style-definition:
> 
> vdso_test_correctness.c:111:13: warning: old-style function definition [-Wold-style-definition]
>   111 | static void fill_function_pointers()
>       |             ^~~~~~~~~~~~~~~~~~~~~~
This warning doesn't appear on my side. Are you using extra compiler
flags? If yes, please add them to the Makefile.

> 
> Explicitly use an empty argument list.
> 
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> ---
>  tools/testing/selftests/vDSO/vdso_test_correctness.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/vDSO/vdso_test_correctness.c b/tools/testing/selftests/vDSO/vdso_test_correctness.c
> index 5fb97ad67eeaf17b6cfa4f82783c57894f03e5c5..da651cf53c6ca4242085de109c7fc57bd807297c 100644
> --- a/tools/testing/selftests/vDSO/vdso_test_correctness.c
> +++ b/tools/testing/selftests/vDSO/vdso_test_correctness.c
> @@ -108,7 +108,7 @@ static void *vsyscall_getcpu(void)
>  }
>  
>  
> -static void fill_function_pointers()
> +static void fill_function_pointers(void)
>  {
>  	void *vdso = dlopen("linux-vdso.so.1",
>  			    RTLD_LAZY | RTLD_LOCAL | RTLD_NOLOAD);
>
Muhammad Usama Anjum May 2, 2025, 1:48 p.m. UTC | #3
On 5/2/25 5:40 PM, Thomas Weißschuh wrote:
> The TAP specification requires that the output begins with a header line.
> If vgetrandom_init() fails and skips the test, that header line is missing.
> 
> Call vgetrandom_init() after ksft_print_header().
> 
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>

> ---
>  tools/testing/selftests/vDSO/vdso_test_getrandom.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/vDSO/vdso_test_getrandom.c b/tools/testing/selftests/vDSO/vdso_test_getrandom.c
> index b0e0d664508a38d6dde9df0a61ec8198ee928a17..01892d8e65d754d0353f7df2b63910d5be8cd1bc 100644
> --- a/tools/testing/selftests/vDSO/vdso_test_getrandom.c
> +++ b/tools/testing/selftests/vDSO/vdso_test_getrandom.c
> @@ -232,6 +232,7 @@ static void kselftest(void)
>  	pid_t child;
>  
>  	ksft_print_header();
> +	vgetrandom_init();
>  	ksft_set_plan(2);
>  
>  	for (size_t i = 0; i < 1000; ++i) {
> @@ -285,8 +286,6 @@ static void usage(const char *argv0)
>  
>  int main(int argc, char *argv[])
>  {
> -	vgetrandom_init();
> -
>  	if (argc == 1) {
>  		kselftest();
>  		return 0;
> @@ -296,6 +295,9 @@ int main(int argc, char *argv[])
>  		usage(argv[0]);
>  		return 1;
>  	}
> +
> +	vgetrandom_init();
> +
>  	if (!strcmp(argv[1], "bench-single"))
>  		bench_single();
>  	else if (!strcmp(argv[1], "bench-multi"))
>