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,
According to kselftest.h ksft_exit_skip() is not meant to be called when a plan has already been printed.
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
--- 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[])
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[])
Nothing from err.h is used.
Drop the include.
Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de --- tools/testing/selftests/vDSO/vdso_test_clock_getres.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/tools/testing/selftests/vDSO/vdso_test_clock_getres.c b/tools/testing/selftests/vDSO/vdso_test_clock_getres.c index 38d46a8bf7cba7a9b4a9b13b5eb17aa207972bd0..b5d5f59f725a703c357dfca91bfe170aaaeb42fa 100644 --- a/tools/testing/selftests/vDSO/vdso_test_clock_getres.c +++ b/tools/testing/selftests/vDSO/vdso_test_clock_getres.c @@ -13,7 +13,6 @@
#define _GNU_SOURCE #include <elf.h> -#include <err.h> #include <fcntl.h> #include <stdint.h> #include <stdio.h>
On 5/2/25 5:40 PM, Thomas Weißschuh wrote:
Nothing from err.h is used.
Drop the include.
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_clock_getres.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/tools/testing/selftests/vDSO/vdso_test_clock_getres.c b/tools/testing/selftests/vDSO/vdso_test_clock_getres.c index 38d46a8bf7cba7a9b4a9b13b5eb17aa207972bd0..b5d5f59f725a703c357dfca91bfe170aaaeb42fa 100644 --- a/tools/testing/selftests/vDSO/vdso_test_clock_getres.c +++ b/tools/testing/selftests/vDSO/vdso_test_clock_getres.c @@ -13,7 +13,6 @@ #define _GNU_SOURCE #include <elf.h> -#include <err.h> #include <fcntl.h> #include <stdint.h> #include <stdio.h>
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() | ^~~~~~~~~~~~~~~~~~~~~~
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);
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);
The header is unused. Furthermore this is not a real UAPI header, but only exists in tools/include/. This prevents building the selftest against real UAPI headers.
Drop the include.
Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de --- tools/testing/selftests/vDSO/vdso_test_getrandom.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/tools/testing/selftests/vDSO/vdso_test_getrandom.c b/tools/testing/selftests/vDSO/vdso_test_getrandom.c index 95057f7567db22226d9cb09a667a56e387a33a46..f36e50f372f935e6d4da3175c81e210653bdce1d 100644 --- a/tools/testing/selftests/vDSO/vdso_test_getrandom.c +++ b/tools/testing/selftests/vDSO/vdso_test_getrandom.c @@ -21,7 +21,6 @@ #include <sys/wait.h> #include <sys/types.h> #include <linux/random.h> -#include <linux/compiler.h> #include <linux/ptrace.h>
#include "../kselftest.h"
vgetrandom_put_state() and the variable ret in kselftest() are never used.
Drop the dead code.
Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de --- tools/testing/selftests/vDSO/vdso_test_getrandom.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/tools/testing/selftests/vDSO/vdso_test_getrandom.c b/tools/testing/selftests/vDSO/vdso_test_getrandom.c index f36e50f372f935e6d4da3175c81e210653bdce1d..b0e0d664508a38d6dde9df0a61ec8198ee928a17 100644 --- a/tools/testing/selftests/vDSO/vdso_test_getrandom.c +++ b/tools/testing/selftests/vDSO/vdso_test_getrandom.c @@ -100,15 +100,6 @@ static void *vgetrandom_get_state(void) return state; }
-static void vgetrandom_put_state(void *state) -{ - if (!state) - return; - pthread_mutex_lock(&vgrnd.lock); - vgrnd.states[vgrnd.len++] = state; - pthread_mutex_unlock(&vgrnd.lock); -} - static void vgetrandom_init(void) { const char *version = versions[VDSO_VERSION]; @@ -264,7 +255,7 @@ static void kselftest(void) } for (;;) { struct ptrace_syscall_info info = { 0 }; - int status, ret; + int status; ksft_assert(waitpid(child, &status, 0) >= 0); if (WIFEXITED(status)) { ksft_assert(WEXITSTATUS(status) == 0);
On 5/2/25 5:40 PM, Thomas Weißschuh wrote:
vgetrandom_put_state() and the variable ret in kselftest() are never used.
Drop the dead code.
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 | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/tools/testing/selftests/vDSO/vdso_test_getrandom.c b/tools/testing/selftests/vDSO/vdso_test_getrandom.c index f36e50f372f935e6d4da3175c81e210653bdce1d..b0e0d664508a38d6dde9df0a61ec8198ee928a17 100644 --- a/tools/testing/selftests/vDSO/vdso_test_getrandom.c +++ b/tools/testing/selftests/vDSO/vdso_test_getrandom.c @@ -100,15 +100,6 @@ static void *vgetrandom_get_state(void) return state; } -static void vgetrandom_put_state(void *state) -{
- if (!state)
return;- pthread_mutex_lock(&vgrnd.lock);
- vgrnd.states[vgrnd.len++] = state;
- pthread_mutex_unlock(&vgrnd.lock);
-}
static void vgetrandom_init(void) { const char *version = versions[VDSO_VERSION]; @@ -264,7 +255,7 @@ static void kselftest(void) } for (;;) { struct ptrace_syscall_info info = { 0 };
int status, ret;
int status;
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 --- 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"))
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"))
Not all users of this header make use of all its variables. For example vdso_test_correctness.c does not use "versions":
In file included from vdso_test_correctness.c:22: vdso_config.h:61:20: warning: ‘versions’ defined but not used [-Wunused-variable] 61 | static const char *versions[7] = { | ^~~~~~~~
Avoid those warnings through attribute((unused)).
Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de --- tools/testing/selftests/vDSO/vdso_config.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/tools/testing/selftests/vDSO/vdso_config.h b/tools/testing/selftests/vDSO/vdso_config.h index 722260f9756198956f0dfccced907284b6851e76..5fdd0f36233742bc47ae79f23d2cfae5a0f56dee 100644 --- a/tools/testing/selftests/vDSO/vdso_config.h +++ b/tools/testing/selftests/vDSO/vdso_config.h @@ -58,6 +58,7 @@ #define VDSO_NAMES 1 #endif
+__attribute__((unused)) static const char *versions[7] = { "LINUX_2.6", "LINUX_2.6.15", @@ -68,6 +69,7 @@ static const char *versions[7] = { "LINUX_5.10" };
+__attribute__((unused)) static const char *names[2][7] = { { "__kernel_gettimeofday",
On 5/2/25 5:40 PM, Thomas Weißschuh wrote:
Not all users of this header make use of all its variables. For example vdso_test_correctness.c does not use "versions":
In file included from vdso_test_correctness.c:22: vdso_config.h:61:20: warning: ‘versions’ defined but not used [-Wunused-variable] 61 | static const char *versions[7] = { | ^~~~~~~~
Avoid those warnings through attribute((unused)).
Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de
Reviewed-by: Muhammad Usama Anjum usama.anjum@collabora.com
tools/testing/selftests/vDSO/vdso_config.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/tools/testing/selftests/vDSO/vdso_config.h b/tools/testing/selftests/vDSO/vdso_config.h index 722260f9756198956f0dfccced907284b6851e76..5fdd0f36233742bc47ae79f23d2cfae5a0f56dee 100644 --- a/tools/testing/selftests/vDSO/vdso_config.h +++ b/tools/testing/selftests/vDSO/vdso_config.h @@ -58,6 +58,7 @@ #define VDSO_NAMES 1 #endif +__attribute__((unused)) static const char *versions[7] = { "LINUX_2.6", "LINUX_2.6.15", @@ -68,6 +69,7 @@ static const char *versions[7] = { "LINUX_5.10" }; +__attribute__((unused)) static const char *names[2][7] = { { "__kernel_gettimeofday",
linux-kselftest-mirror@lists.linaro.org
-
Muhammad Usama Anjum -
Thomas Weißschuh