The timer selftests are quite useful for me when enabling timers on new SoCs, e.g. like now with the CMT timer on a Renesas R-Car S4-8. During development, I needed these fixes and additions to make full use of the tests. I think they make all sense upstream, so here they are.
Patches are based on v5.19-rc1. Looking forward to comments.
Happy hacking,
Wolfram
Wolfram Sang (9): selftests: timers: valid-adjtimex: build fix for newer toolchains selftests: timers: fix declarations of main() selftests: timers: nanosleep: adapt to kselftest framework selftests: timers: inconsistency-check: adapt to kselftest framework selftests: timers: clocksource-switch: fix passing errors from child selftests: timers: clocksource-switch: sort includes selftests: timers: clocksource-switch: add command line switch to skip sanity check selftests: timers: clocksource-switch: add 'runtime' command line parameter selftests: timers: clocksource-switch: adapt to kselftest framework
tools/testing/selftests/timers/adjtick.c | 2 +- tools/testing/selftests/timers/change_skew.c | 2 +- .../selftests/timers/clocksource-switch.c | 70 ++++++++++++------- .../selftests/timers/inconsistency-check.c | 32 +++++---- tools/testing/selftests/timers/nanosleep.c | 18 +++-- tools/testing/selftests/timers/raw_skew.c | 2 +- .../selftests/timers/skew_consistency.c | 2 +- .../testing/selftests/timers/valid-adjtimex.c | 2 +- 8 files changed, 80 insertions(+), 50 deletions(-)
Toolchains with an include file 'sys/timex.h' based on 3.18 will have a 'clock_adjtime' definition added, so it can't be static in the code:
valid-adjtimex.c:43:12: error: static declaration of ‘clock_adjtime’ follows non-static declaration
Fixes: e03a58c320e1 ("kselftests: timers: Add adjtimex SETOFFSET validity tests") Signed-off-by: Wolfram Sang wsa+renesas@sang-engineering.com --- tools/testing/selftests/timers/valid-adjtimex.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/timers/valid-adjtimex.c b/tools/testing/selftests/timers/valid-adjtimex.c index 5397de708d3c..48b9a803235a 100644 --- a/tools/testing/selftests/timers/valid-adjtimex.c +++ b/tools/testing/selftests/timers/valid-adjtimex.c @@ -40,7 +40,7 @@ #define ADJ_SETOFFSET 0x0100
#include <sys/syscall.h> -static int clock_adjtime(clockid_t id, struct timex *tx) +int clock_adjtime(clockid_t id, struct timex *tx) { return syscall(__NR_clock_adjtime, id, tx); }
Mixing up argc/argv went unnoticed because they were not used. Still, this is worth fixing.
Signed-off-by: Wolfram Sang wsa+renesas@sang-engineering.com --- tools/testing/selftests/timers/adjtick.c | 2 +- tools/testing/selftests/timers/change_skew.c | 2 +- tools/testing/selftests/timers/raw_skew.c | 2 +- tools/testing/selftests/timers/skew_consistency.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/timers/adjtick.c b/tools/testing/selftests/timers/adjtick.c index 54d8d87f36b3..47e05fdc32c5 100644 --- a/tools/testing/selftests/timers/adjtick.c +++ b/tools/testing/selftests/timers/adjtick.c @@ -165,7 +165,7 @@ int check_tick_adj(long tickval) return 0; }
-int main(int argv, char **argc) +int main(int argc, char **argv) { struct timespec raw; long tick, max, interval, err; diff --git a/tools/testing/selftests/timers/change_skew.c b/tools/testing/selftests/timers/change_skew.c index c4eab7124990..992a77f2a74c 100644 --- a/tools/testing/selftests/timers/change_skew.c +++ b/tools/testing/selftests/timers/change_skew.c @@ -55,7 +55,7 @@ int change_skew_test(int ppm) }
-int main(int argv, char **argc) +int main(int argc, char **argv) { struct timex tx; int i, ret; diff --git a/tools/testing/selftests/timers/raw_skew.c b/tools/testing/selftests/timers/raw_skew.c index b41d8dd0c40c..5beceeed0d11 100644 --- a/tools/testing/selftests/timers/raw_skew.c +++ b/tools/testing/selftests/timers/raw_skew.c @@ -89,7 +89,7 @@ void get_monotonic_and_raw(struct timespec *mon, struct timespec *raw) } }
-int main(int argv, char **argc) +int main(int argc, char **argv) { struct timespec mon, raw, start, end; long long delta1, delta2, interval, eppm, ppm; diff --git a/tools/testing/selftests/timers/skew_consistency.c b/tools/testing/selftests/timers/skew_consistency.c index 8066be9aff11..63913f75b384 100644 --- a/tools/testing/selftests/timers/skew_consistency.c +++ b/tools/testing/selftests/timers/skew_consistency.c @@ -38,7 +38,7 @@
#define NSEC_PER_SEC 1000000000LL
-int main(int argv, char **argc) +int main(int argc, char **argv) { struct timex tx; int ret, ppm;
So we have proper counters at the end of a test, e.g.: # Totals: pass:4 fail:0 xfail:0 xpass:0 skip:8 error:0
Signed-off-by: Wolfram Sang wsa+renesas@sang-engineering.com --- tools/testing/selftests/timers/nanosleep.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/timers/nanosleep.c b/tools/testing/selftests/timers/nanosleep.c index 71b5441c2fd9..df1d03516e7b 100644 --- a/tools/testing/selftests/timers/nanosleep.c +++ b/tools/testing/selftests/timers/nanosleep.c @@ -133,33 +133,37 @@ int main(int argc, char **argv) long long length; int clockid, ret;
+ ksft_print_header(); + ksft_set_plan(NR_CLOCKIDS); + for (clockid = CLOCK_REALTIME; clockid < NR_CLOCKIDS; clockid++) {
/* Skip cputime clockids since nanosleep won't increment cputime */ if (clockid == CLOCK_PROCESS_CPUTIME_ID || clockid == CLOCK_THREAD_CPUTIME_ID || - clockid == CLOCK_HWSPECIFIC) + clockid == CLOCK_HWSPECIFIC) { + ksft_test_result_skip("%-31s\n", clockstring(clockid)); continue; + }
- printf("Nanosleep %-31s ", clockstring(clockid)); fflush(stdout);
length = 10; while (length <= (NSEC_PER_SEC * 10)) { ret = nanosleep_test(clockid, length); if (ret == UNSUPPORTED) { - printf("[UNSUPPORTED]\n"); + ksft_test_result_skip("%-31s\n", clockstring(clockid)); goto next; } if (ret < 0) { - printf("[FAILED]\n"); - return ksft_exit_fail(); + ksft_test_result_fail("%-31s\n", clockstring(clockid)); + ksft_exit_fail(); } length *= 100; } - printf("[OK]\n"); + ksft_test_result_pass("%-31s\n", clockstring(clockid)); next: ret = 0; } - return ksft_exit_pass(); + ksft_exit_pass(); }
So we have proper counters at the end of a test, e.g.: # Totals: pass:11 fail:0 xfail:0 xpass:0 skip:1 error:0
Signed-off-by: Wolfram Sang wsa+renesas@sang-engineering.com --- .../selftests/timers/inconsistency-check.c | 32 +++++++++++-------- 1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/tools/testing/selftests/timers/inconsistency-check.c b/tools/testing/selftests/timers/inconsistency-check.c index e6756d9c60a7..36a49fba6c9b 100644 --- a/tools/testing/selftests/timers/inconsistency-check.c +++ b/tools/testing/selftests/timers/inconsistency-check.c @@ -122,30 +122,28 @@ int consistency_test(int clock_type, unsigned long seconds) if (inconsistent >= 0) { unsigned long long delta;
- printf("%s\n", start_str); + ksft_print_msg("%s\n", start_str); for (i = 0; i < CALLS_PER_LOOP; i++) { if (i == inconsistent) - printf("--------------------\n"); - printf("%lu:%lu\n", list[i].tv_sec, + ksft_print_msg("--------------------\n"); + ksft_print_msg("%lu:%lu\n", list[i].tv_sec, list[i].tv_nsec); if (i == inconsistent + 1) - printf("--------------------\n"); + ksft_print_msg("--------------------\n"); } delta = list[inconsistent].tv_sec * NSEC_PER_SEC; delta += list[inconsistent].tv_nsec; delta -= list[inconsistent+1].tv_sec * NSEC_PER_SEC; delta -= list[inconsistent+1].tv_nsec; - printf("Delta: %llu ns\n", delta); + ksft_print_msg("Delta: %llu ns\n", delta); fflush(0); /* timestamp inconsistency*/ t = time(0); - printf("%s\n", ctime(&t)); - printf("[FAILED]\n"); + ksft_print_msg("%s\n", ctime(&t)); return -1; } now = list[0].tv_sec; } - printf("[OK]\n"); return 0; }
@@ -178,16 +176,22 @@ int main(int argc, char *argv[])
setbuf(stdout, NULL);
+ ksft_print_header(); + ksft_set_plan(maxclocks - userclock); + for (clockid = userclock; clockid < maxclocks; clockid++) {
- if (clockid == CLOCK_HWSPECIFIC) + if (clockid == CLOCK_HWSPECIFIC || clock_gettime(clockid, &ts)) { + ksft_test_result_skip("%-31s\n", clockstring(clockid)); continue; + }
- if (!clock_gettime(clockid, &ts)) { - printf("Consistent %-30s ", clockstring(clockid)); - if (consistency_test(clockid, runtime)) - return ksft_exit_fail(); + if (consistency_test(clockid, runtime)) { + ksft_test_result_fail("%-31s\n", clockstring(clockid)); + ksft_exit_fail(); + } else { + ksft_test_result_pass("%-31s\n", clockstring(clockid)); } } - return ksft_exit_pass(); + ksft_exit_pass(); }
The return value from system() is a waitpid-style integer. Do not return it directly because with the implicit masking in exit() it will always return 0. Access it with apropriate macros to really pass on errors.
Fixes: 7290ce1423c3 ("selftests/timers: Add clocksource-switch test from timetest suite") Signed-off-by: Wolfram Sang wsa+renesas@sang-engineering.com --- tools/testing/selftests/timers/clocksource-switch.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/timers/clocksource-switch.c b/tools/testing/selftests/timers/clocksource-switch.c index ef8eb3604595..b57f0a9be490 100644 --- a/tools/testing/selftests/timers/clocksource-switch.c +++ b/tools/testing/selftests/timers/clocksource-switch.c @@ -110,10 +110,10 @@ int run_tests(int secs)
sprintf(buf, "./inconsistency-check -t %i", secs); ret = system(buf); - if (ret) - return ret; + if (WIFEXITED(ret) && WEXITSTATUS(ret)) + return WEXITSTATUS(ret); ret = system("./nanosleep"); - return ret; + return WIFEXITED(ret) ? WEXITSTATUS(ret) : 0; }
It is easier to check if you need to add an include if the existing ones are sorted.
Signed-off-by: Wolfram Sang wsa+renesas@sang-engineering.com --- tools/testing/selftests/timers/clocksource-switch.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/timers/clocksource-switch.c b/tools/testing/selftests/timers/clocksource-switch.c index b57f0a9be490..ed5b71f5b37c 100644 --- a/tools/testing/selftests/timers/clocksource-switch.c +++ b/tools/testing/selftests/timers/clocksource-switch.c @@ -23,17 +23,17 @@ */
+#include <fcntl.h> #include <stdio.h> -#include <unistd.h> #include <stdlib.h> +#include <string.h> +#include <sys/stat.h> #include <sys/time.h> #include <sys/timex.h> -#include <time.h> #include <sys/types.h> -#include <sys/stat.h> -#include <fcntl.h> -#include <string.h> #include <sys/wait.h> +#include <time.h> +#include <unistd.h> #include "../kselftest.h"
The sanity check takes a while. If you do repeated checks when debugging, this is time consuming. Add a parameter to skip it.
Signed-off-by: Wolfram Sang wsa+renesas@sang-engineering.com --- .../selftests/timers/clocksource-switch.c | 40 +++++++++++++------ 1 file changed, 28 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/timers/clocksource-switch.c b/tools/testing/selftests/timers/clocksource-switch.c index ed5b71f5b37c..5256e6215980 100644 --- a/tools/testing/selftests/timers/clocksource-switch.c +++ b/tools/testing/selftests/timers/clocksource-switch.c @@ -119,12 +119,26 @@ int run_tests(int secs)
char clocksource_list[10][30];
-int main(int argv, char **argc) +int main(int argc, char **argv) { char orig_clk[512]; - int count, i, status; + int count, i, status, opt; + int do_sanity_check = 1; pid_t pid;
+ /* Process arguments */ + while ((opt = getopt(argc, argv, "s")) != -1) { + switch (opt) { + case 's': + do_sanity_check = 0; + break; + default: + printf("Usage: %s [-s]\n", argv[0]); + printf(" -s: skip sanity checks\n"); + exit(-1); + } + } + get_cur_clocksource(orig_clk, 512);
count = get_clocksources(clocksource_list); @@ -135,19 +149,21 @@ int main(int argv, char **argc) }
/* Check everything is sane before we start switching asynchronously */ - for (i = 0; i < count; i++) { - printf("Validating clocksource %s\n", clocksource_list[i]); - if (change_clocksource(clocksource_list[i])) { - status = -1; - goto out; - } - if (run_tests(5)) { - status = -1; - goto out; + if (do_sanity_check) { + for (i = 0; i < count; i++) { + printf("Validating clocksource %s\n", + clocksource_list[i]); + if (change_clocksource(clocksource_list[i])) { + status = -1; + goto out; + } + if (run_tests(5)) { + status = -1; + goto out; + } } }
- printf("Running Asynchronous Switching Tests...\n"); pid = fork(); if (!pid)
So the user can decide how long the test should run.
Signed-off-by: Wolfram Sang wsa+renesas@sang-engineering.com --- tools/testing/selftests/timers/clocksource-switch.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/timers/clocksource-switch.c b/tools/testing/selftests/timers/clocksource-switch.c index 5256e6215980..a1d0d33738b6 100644 --- a/tools/testing/selftests/timers/clocksource-switch.c +++ b/tools/testing/selftests/timers/clocksource-switch.c @@ -124,14 +124,18 @@ int main(int argc, char **argv) char orig_clk[512]; int count, i, status, opt; int do_sanity_check = 1; + int runtime = 60; pid_t pid;
/* Process arguments */ - while ((opt = getopt(argc, argv, "s")) != -1) { + while ((opt = getopt(argc, argv, "st:")) != -1) { switch (opt) { case 's': do_sanity_check = 0; break; + case 't': + runtime = atoi(optarg); + break; default: printf("Usage: %s [-s]\n", argv[0]); printf(" -s: skip sanity checks\n"); @@ -167,7 +171,7 @@ int main(int argc, char **argv) printf("Running Asynchronous Switching Tests...\n"); pid = fork(); if (!pid) - return run_tests(60); + return run_tests(runtime);
while (pid != waitpid(pid, &status, WNOHANG)) for (i = 0; i < count; i++)
On Wed, Jul 13, 2022 at 1:46 PM Wolfram Sang wsa+renesas@sang-engineering.com wrote:
So the user can decide how long the test should run.
Signed-off-by: Wolfram Sang wsa+renesas@sang-engineering.com
tools/testing/selftests/timers/clocksource-switch.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/timers/clocksource-switch.c b/tools/testing/selftests/timers/clocksource-switch.c index 5256e6215980..a1d0d33738b6 100644 --- a/tools/testing/selftests/timers/clocksource-switch.c +++ b/tools/testing/selftests/timers/clocksource-switch.c @@ -124,14 +124,18 @@ int main(int argc, char **argv) char orig_clk[512]; int count, i, status, opt; int do_sanity_check = 1;
int runtime = 60; pid_t pid; /* Process arguments */
while ((opt = getopt(argc, argv, "s")) != -1) {
while ((opt = getopt(argc, argv, "st:")) != -1) { switch (opt) { case 's': do_sanity_check = 0; break;case 't':runtime = atoi(optarg);break; default: printf("Usage: %s [-s]\n", argv[0]); printf(" -s: skip sanity checks\n");
Ah, one minor nit: Should the -t option get documented here in the usage output?
thanks again! -john
Hello!
On 7/13/22 11:46 PM, Wolfram Sang wrote:
So the user can decide how long the test should run.
Signed-off-by: Wolfram Sang wsa+renesas@sang-engineering.com
tools/testing/selftests/timers/clocksource-switch.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/timers/clocksource-switch.c b/tools/testing/selftests/timers/clocksource-switch.c index 5256e6215980..a1d0d33738b6 100644 --- a/tools/testing/selftests/timers/clocksource-switch.c +++ b/tools/testing/selftests/timers/clocksource-switch.c
[...]
- while ((opt = getopt(argc, argv, "s")) != -1) {
- while ((opt = getopt(argc, argv, "st:")) != -1) { switch (opt) { case 's': do_sanity_check = 0; break;
case 't':runtime = atoi(optarg);break;
Hm, you probably forgot to update the usage msg?
[...]
MBR, Sergey
So we have proper counters at the end of a test. We also print the kselftest header at the end of the test, so we don't mix with the output of the child process. There is only this one test anyhow.
Signed-off-by: Wolfram Sang wsa+renesas@sang-engineering.com --- tools/testing/selftests/timers/clocksource-switch.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/timers/clocksource-switch.c b/tools/testing/selftests/timers/clocksource-switch.c index a1d0d33738b6..e2c0e4485ea8 100644 --- a/tools/testing/selftests/timers/clocksource-switch.c +++ b/tools/testing/selftests/timers/clocksource-switch.c @@ -182,7 +182,9 @@ int main(int argc, char **argv) out: change_clocksource(orig_clk);
- if (status) - return ksft_exit_fail(); - return ksft_exit_pass(); + /* Print at the end to not mix output with child process */ + ksft_print_header(); + ksft_set_plan(1); + ksft_test_result(!status, "clocksource-switch\n"); + ksft_exit(!status); }
On Wed, Jul 13, 2022 at 1:46 PM Wolfram Sang wsa+renesas@sang-engineering.com wrote:
The timer selftests are quite useful for me when enabling timers on new SoCs, e.g. like now with the CMT timer on a Renesas R-Car S4-8. During development, I needed these fixes and additions to make full use of the tests. I think they make all sense upstream, so here they are.
Patches are based on v5.19-rc1. Looking forward to comments.
Hey! Thanks so much I really appreciate the effort to make and send out these cleanups. From the initial skim it all looks great (though, some are slightly embarrassing :), and I think some of the extra config args will be quite nice for others to use as well.
Acked-by: John Stultz jstultz@google.com
Thanks so much for submitting these. -john
On 7/13/22 3:02 PM, John Stultz wrote:
On Wed, Jul 13, 2022 at 1:46 PM Wolfram Sang wsa+renesas@sang-engineering.com wrote:
The timer selftests are quite useful for me when enabling timers on new SoCs, e.g. like now with the CMT timer on a Renesas R-Car S4-8. During development, I needed these fixes and additions to make full use of the tests. I think they make all sense upstream, so here they are.
Patches are based on v5.19-rc1. Looking forward to comments.
Hey! Thanks so much I really appreciate the effort to make and send out these cleanups. From the initial skim it all looks great (though, some are slightly embarrassing :), and I think some of the extra config args will be quite nice for others to use as well.
Acked-by: John Stultz jstultz@google.com
Thanks so much for submitting these. -john
Thank you both. I can queue these for 5.20-rc1
Wolfram, are you going to send v2 to address John's comment on 8/9?
thanks, -- Shuah
Hey John,
Acked-by: John Stultz jstultz@google.com
Thanks so much for submitting these.
Glad you like this series. I will wait some more to see if there are further review comments. But surely, I will add the missing parameter to the help output and add your tags to v2.
Thank you for doing these tools :)
Wolfram
linux-kselftest-mirror@lists.linaro.org
-
John Stultz -
Sergei Shtylyov -
Shuah Khan -
Wolfram Sang