Fixes for various issues in the timens selftests.
Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de --- Thomas Weißschuh (3): selftests/timens: Print TAP headers selftests/timens: Make run_tests() functions static selftests/timens: timerfd: Use correct clockid type in tclock_gettime()
tools/testing/selftests/timens/clock_nanosleep.c | 4 +++- tools/testing/selftests/timens/exec.c | 2 ++ tools/testing/selftests/timens/futex.c | 2 ++ tools/testing/selftests/timens/gettime_perf.c | 2 ++ tools/testing/selftests/timens/procfs.c | 2 ++ tools/testing/selftests/timens/timens.c | 2 ++ tools/testing/selftests/timens/timer.c | 4 +++- tools/testing/selftests/timens/timerfd.c | 6 ++++-- tools/testing/selftests/timens/vfork_exec.c | 2 ++ 9 files changed, 22 insertions(+), 4 deletions(-) --- base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8 change-id: 20250326-selftests-timens-fixes-97662bdb18b6
Best regards,
The TAP specification requires that the output begins with a header line. These headers lines are missing in the timens tests.
Print such a line.
Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de --- tools/testing/selftests/timens/clock_nanosleep.c | 2 ++ tools/testing/selftests/timens/exec.c | 2 ++ tools/testing/selftests/timens/futex.c | 2 ++ tools/testing/selftests/timens/gettime_perf.c | 2 ++ tools/testing/selftests/timens/procfs.c | 2 ++ tools/testing/selftests/timens/timens.c | 2 ++ tools/testing/selftests/timens/timer.c | 2 ++ tools/testing/selftests/timens/timerfd.c | 2 ++ tools/testing/selftests/timens/vfork_exec.c | 2 ++ 9 files changed, 18 insertions(+)
diff --git a/tools/testing/selftests/timens/clock_nanosleep.c b/tools/testing/selftests/timens/clock_nanosleep.c index 72d41b955fb2263ae74c34b446fb322d1bd50c6a..346bff3d128dfa8c8c743cc2eba74917994bb2f7 100644 --- a/tools/testing/selftests/timens/clock_nanosleep.c +++ b/tools/testing/selftests/timens/clock_nanosleep.c @@ -115,6 +115,8 @@ int main(int argc, char *argv[]) { int ret, nsfd;
+ ksft_print_header(); + nscheck();
ksft_set_plan(4); diff --git a/tools/testing/selftests/timens/exec.c b/tools/testing/selftests/timens/exec.c index d12ff955de0d8f6665fa957e81438e373af9f92c..a644162d56fdc86827b83a45f83c2597c253aa6a 100644 --- a/tools/testing/selftests/timens/exec.c +++ b/tools/testing/selftests/timens/exec.c @@ -36,6 +36,8 @@ int main(int argc, char *argv[]) return 0; }
+ ksft_print_header(); + nscheck();
ksft_set_plan(1); diff --git a/tools/testing/selftests/timens/futex.c b/tools/testing/selftests/timens/futex.c index 6b2b9264e851caed06a628589eda0bf81ff33f16..339633ae037a74d2026ca835ac8600cd0394ae01 100644 --- a/tools/testing/selftests/timens/futex.c +++ b/tools/testing/selftests/timens/futex.c @@ -66,6 +66,8 @@ int main(int argc, char *argv[]) pid_t pid; struct timespec mtime_now;
+ ksft_print_header(); + nscheck();
ksft_set_plan(2); diff --git a/tools/testing/selftests/timens/gettime_perf.c b/tools/testing/selftests/timens/gettime_perf.c index 6b13dc277724126a9d2d5e17c805217760df72ad..d6658b7b754877fbf67237d6c22787a2a89066ce 100644 --- a/tools/testing/selftests/timens/gettime_perf.c +++ b/tools/testing/selftests/timens/gettime_perf.c @@ -67,6 +67,8 @@ int main(int argc, char *argv[]) time_t offset = 10; int nsfd;
+ ksft_print_header(); + ksft_set_plan(8);
fill_function_pointers(); diff --git a/tools/testing/selftests/timens/procfs.c b/tools/testing/selftests/timens/procfs.c index 1833ca97eb247b1b45b7a86f0ca800d4a6e58c74..0a9ff90ee69a9327ed905428f1462fa189ffd830 100644 --- a/tools/testing/selftests/timens/procfs.c +++ b/tools/testing/selftests/timens/procfs.c @@ -180,6 +180,8 @@ int main(int argc, char *argv[]) { int ret = 0;
+ ksft_print_header(); + nscheck();
ksft_set_plan(2); diff --git a/tools/testing/selftests/timens/timens.c b/tools/testing/selftests/timens/timens.c index 387220791a052e6013bd0fb4162f123d85152183..a9c0534ef8f67184c34882ae163472c179f593e4 100644 --- a/tools/testing/selftests/timens/timens.c +++ b/tools/testing/selftests/timens/timens.c @@ -151,6 +151,8 @@ int main(int argc, char *argv[]) time_t offset; int ret = 0;
+ ksft_print_header(); + nscheck();
check_supported_timers(); diff --git a/tools/testing/selftests/timens/timer.c b/tools/testing/selftests/timens/timer.c index 5b939f59dfa4d64e4c2e4dc8eb6a644d44211f13..51babe63e233bdaaef35c1cafbcb5b5cc91c021e 100644 --- a/tools/testing/selftests/timens/timer.c +++ b/tools/testing/selftests/timens/timer.c @@ -75,6 +75,8 @@ int main(int argc, char *argv[]) pid_t pid; struct timespec btime_now, mtime_now;
+ ksft_print_header(); + nscheck();
check_supported_timers(); diff --git a/tools/testing/selftests/timens/timerfd.c b/tools/testing/selftests/timens/timerfd.c index a4196bbd6e33f41b6a3ae346070037aff1cf4087..e58bc8b64ce2738dccb8c2a2c88e592c237b55d5 100644 --- a/tools/testing/selftests/timens/timerfd.c +++ b/tools/testing/selftests/timens/timerfd.c @@ -82,6 +82,8 @@ int main(int argc, char *argv[]) pid_t pid; struct timespec btime_now, mtime_now;
+ ksft_print_header(); + nscheck();
check_supported_timers(); diff --git a/tools/testing/selftests/timens/vfork_exec.c b/tools/testing/selftests/timens/vfork_exec.c index 5b8907bf451dde3f3e2699f87797c155b9115aab..b957e1a651243adde41ec5cd57bbd12a7cac9eeb 100644 --- a/tools/testing/selftests/timens/vfork_exec.c +++ b/tools/testing/selftests/timens/vfork_exec.c @@ -91,6 +91,8 @@ int main(int argc, char *argv[]) return check("child after exec", &now); }
+ ksft_print_header(); + nscheck();
ksft_set_plan(4);
On 5/2/25 5:03 PM, Thomas Weißschuh wrote:
The TAP specification requires that the output begins with a header line. These headers lines are missing in the timens tests.
Print such a line.
Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de
Reviewed-by: Muhammad Usama Anjum usama.anjum@collabora.com
tools/testing/selftests/timens/clock_nanosleep.c | 2 ++ tools/testing/selftests/timens/exec.c | 2 ++ tools/testing/selftests/timens/futex.c | 2 ++ tools/testing/selftests/timens/gettime_perf.c | 2 ++ tools/testing/selftests/timens/procfs.c | 2 ++ tools/testing/selftests/timens/timens.c | 2 ++ tools/testing/selftests/timens/timer.c | 2 ++ tools/testing/selftests/timens/timerfd.c | 2 ++ tools/testing/selftests/timens/vfork_exec.c | 2 ++ 9 files changed, 18 insertions(+)
diff --git a/tools/testing/selftests/timens/clock_nanosleep.c b/tools/testing/selftests/timens/clock_nanosleep.c index 72d41b955fb2263ae74c34b446fb322d1bd50c6a..346bff3d128dfa8c8c743cc2eba74917994bb2f7 100644 --- a/tools/testing/selftests/timens/clock_nanosleep.c +++ b/tools/testing/selftests/timens/clock_nanosleep.c @@ -115,6 +115,8 @@ int main(int argc, char *argv[]) { int ret, nsfd;
- ksft_print_header();
- nscheck();
ksft_set_plan(4); diff --git a/tools/testing/selftests/timens/exec.c b/tools/testing/selftests/timens/exec.c index d12ff955de0d8f6665fa957e81438e373af9f92c..a644162d56fdc86827b83a45f83c2597c253aa6a 100644 --- a/tools/testing/selftests/timens/exec.c +++ b/tools/testing/selftests/timens/exec.c @@ -36,6 +36,8 @@ int main(int argc, char *argv[]) return 0; }
- ksft_print_header();
- nscheck();
ksft_set_plan(1); diff --git a/tools/testing/selftests/timens/futex.c b/tools/testing/selftests/timens/futex.c index 6b2b9264e851caed06a628589eda0bf81ff33f16..339633ae037a74d2026ca835ac8600cd0394ae01 100644 --- a/tools/testing/selftests/timens/futex.c +++ b/tools/testing/selftests/timens/futex.c @@ -66,6 +66,8 @@ int main(int argc, char *argv[]) pid_t pid; struct timespec mtime_now;
- ksft_print_header();
- nscheck();
ksft_set_plan(2); diff --git a/tools/testing/selftests/timens/gettime_perf.c b/tools/testing/selftests/timens/gettime_perf.c index 6b13dc277724126a9d2d5e17c805217760df72ad..d6658b7b754877fbf67237d6c22787a2a89066ce 100644 --- a/tools/testing/selftests/timens/gettime_perf.c +++ b/tools/testing/selftests/timens/gettime_perf.c @@ -67,6 +67,8 @@ int main(int argc, char *argv[]) time_t offset = 10; int nsfd;
- ksft_print_header();
- ksft_set_plan(8);
fill_function_pointers(); diff --git a/tools/testing/selftests/timens/procfs.c b/tools/testing/selftests/timens/procfs.c index 1833ca97eb247b1b45b7a86f0ca800d4a6e58c74..0a9ff90ee69a9327ed905428f1462fa189ffd830 100644 --- a/tools/testing/selftests/timens/procfs.c +++ b/tools/testing/selftests/timens/procfs.c @@ -180,6 +180,8 @@ int main(int argc, char *argv[]) { int ret = 0;
- ksft_print_header();
- nscheck();
ksft_set_plan(2); diff --git a/tools/testing/selftests/timens/timens.c b/tools/testing/selftests/timens/timens.c index 387220791a052e6013bd0fb4162f123d85152183..a9c0534ef8f67184c34882ae163472c179f593e4 100644 --- a/tools/testing/selftests/timens/timens.c +++ b/tools/testing/selftests/timens/timens.c @@ -151,6 +151,8 @@ int main(int argc, char *argv[]) time_t offset; int ret = 0;
- ksft_print_header();
- nscheck();
check_supported_timers(); diff --git a/tools/testing/selftests/timens/timer.c b/tools/testing/selftests/timens/timer.c index 5b939f59dfa4d64e4c2e4dc8eb6a644d44211f13..51babe63e233bdaaef35c1cafbcb5b5cc91c021e 100644 --- a/tools/testing/selftests/timens/timer.c +++ b/tools/testing/selftests/timens/timer.c @@ -75,6 +75,8 @@ int main(int argc, char *argv[]) pid_t pid; struct timespec btime_now, mtime_now;
- ksft_print_header();
- nscheck();
check_supported_timers(); diff --git a/tools/testing/selftests/timens/timerfd.c b/tools/testing/selftests/timens/timerfd.c index a4196bbd6e33f41b6a3ae346070037aff1cf4087..e58bc8b64ce2738dccb8c2a2c88e592c237b55d5 100644 --- a/tools/testing/selftests/timens/timerfd.c +++ b/tools/testing/selftests/timens/timerfd.c @@ -82,6 +82,8 @@ int main(int argc, char *argv[]) pid_t pid; struct timespec btime_now, mtime_now;
- ksft_print_header();
- nscheck();
check_supported_timers(); diff --git a/tools/testing/selftests/timens/vfork_exec.c b/tools/testing/selftests/timens/vfork_exec.c index 5b8907bf451dde3f3e2699f87797c155b9115aab..b957e1a651243adde41ec5cd57bbd12a7cac9eeb 100644 --- a/tools/testing/selftests/timens/vfork_exec.c +++ b/tools/testing/selftests/timens/vfork_exec.c @@ -91,6 +91,8 @@ int main(int argc, char *argv[]) return check("child after exec", &now); }
- ksft_print_header();
- nscheck();
ksft_set_plan(4);
On 5/2/25 06:03, Thomas Weißschuh wrote:
The TAP specification requires that the output begins with a header line. These headers lines are missing in the timens tests.
Print such a line.
There is no cover letter for this - so I will respond to the first patch. The TAP information is added by the kselftest wrapper if you were to run the test using ksefltest.
The following will add the TAP header or if you use make kselftest command from the main Makefile.
make -C timens run_tests
cd timens; make run_tests (will also add TAP header)
The only time you won't see the TAP headers is when you run the test from the test directory just as a command. Is this what you need to do? I would rather not see TAP headers added to invidual tests unless there is a good reason for it.
The problem with adding TAP headers is two-fold:
1. It becomes hard to read the ouptput if test is run using the framework a. make -C timens run_tests b. cd timens; make run_tests 2. These headers become bested and TAP doesn't like nested TAP headers. My undestansing is parses will have problems.
I am adding Tim Bird to the discussion.
Tim, is this still an issue? Are nested TAP headers still a problem.
Even if they aren't there is the issue of multiple TAP headers for each test when we run the test using framework.
thanks, -- Shuah
+Cc Kees
On Wed, May 07, 2025 at 03:06:16PM -0600, Shuah Khan wrote:
On 5/2/25 06:03, Thomas Weißschuh wrote:
The TAP specification requires that the output begins with a header line. These headers lines are missing in the timens tests.
Print such a line.
There is no cover letter for this - so I will respond to the first patch.
Hm, I sent one and can also see it on lore.
The TAP information is added by the kselftest wrapper if you were to run the test using ksefltest.
The following will add the TAP header or if you use make kselftest command from the main Makefile.
make -C timens run_tests
cd timens; make run_tests (will also add TAP header)
The only time you won't see the TAP headers is when you run the test from the test directory just as a command. Is this what you need to do? I would rather not see TAP headers added to invidual tests unless there is a good reason for it.
Yes, I am running each test on its own, as part of kunit based on [0]. I also looked at the vDSO selftests and those all print the TAP headers on their own. The same for all of the x86 selftests I looked at.
The problem with adding TAP headers is two-fold:
- It becomes hard to read the ouptput if test is run using the framework a. make -C timens run_tests b. cd timens; make run_tests
Isn't this what "KSFT_TAP_LEVEL" was added for in commit 10f531f67482 ("selftests: kselftest framework: add handling for TAP header level") ?
- These headers become bested and TAP doesn't like nested TAP headers. My undestansing is parses will have problems.
They are also prefixed with a "#", so they should be ignored by parsers.
I am adding Tim Bird to the discussion.
Tim, is this still an issue? Are nested TAP headers still a problem.
Even if they aren't there is the issue of multiple TAP headers for each test when we run the test using framework.
The suppression was actually there at some point and got removed by Kees in commit f41c322f17ec ("selftests: Remove KSFT_TAP_LEVEL").
[0] https://lore.kernel.org/all/20250217-kunit-kselftests-v1-0-42b4524c3b0a@linu...
On 5/9/25 09:41, Thomas Weißschuh wrote:
+Cc Kees
On Wed, May 07, 2025 at 03:06:16PM -0600, Shuah Khan wrote:
On 5/2/25 06:03, Thomas Weißschuh wrote:
The TAP specification requires that the output begins with a header line. These headers lines are missing in the timens tests.
Print such a line.
There is no cover letter for this - so I will respond to the first patch.
Hm, I sent one and can also see it on lore.
The TAP information is added by the kselftest wrapper if you were to run the test using ksefltest.
The following will add the TAP header or if you use make kselftest command from the main Makefile.
make -C timens run_tests
cd timens; make run_tests (will also add TAP header)
The only time you won't see the TAP headers is when you run the test from the test directory just as a command. Is this what you need to do? I would rather not see TAP headers added to invidual tests unless there is a good reason for it.
Yes, I am running each test on its own, as part of kunit based on [0]. I also looked at the vDSO selftests and those all print the TAP headers on their own. The same for all of the x86 selftests I looked at.
We have been a bit inconsistent with adding TAP headers to individual tests. We added them to some and then when we found out the nesting TAP headers issue, we stopped and never really cleaned up.
Since we have this kunit use-case, I am going to take this series and see who complains. I don't think parsers are in the mix at least for majority of ksefltest runs.
thanks, -- Shuah
On Fri, May 09, 2025 at 05:41:03PM +0200, Thomas Weißschuh wrote:
The suppression was actually there at some point and got removed by Kees in commit f41c322f17ec ("selftests: Remove KSFT_TAP_LEVEL").
Right, to get consistent output and to work with indentation, there's no need to track level any more.
If you want to be able to explicitly supress KTAP output, that's probably a new thing to be added. But normally it's not needed -- things should be fairly readable even with KTAP output.
These functions are never used outside their defining compilation unit and can be made static.
Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de --- tools/testing/selftests/timens/clock_nanosleep.c | 2 +- tools/testing/selftests/timens/timer.c | 2 +- tools/testing/selftests/timens/timerfd.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/timens/clock_nanosleep.c b/tools/testing/selftests/timens/clock_nanosleep.c index 346bff3d128dfa8c8c743cc2eba74917994bb2f7..5cc0010e85ff00e00c89940a82268994a1769873 100644 --- a/tools/testing/selftests/timens/clock_nanosleep.c +++ b/tools/testing/selftests/timens/clock_nanosleep.c @@ -38,7 +38,7 @@ void *call_nanosleep(void *_args) return NULL; }
-int run_test(int clockid, int abs) +static int run_test(int clockid, int abs) { struct timespec now = {}, rem; struct thread_args args = { .now = &now, .rem = &rem, .clockid = clockid}; diff --git a/tools/testing/selftests/timens/timer.c b/tools/testing/selftests/timens/timer.c index 51babe63e233bdaaef35c1cafbcb5b5cc91c021e..79543ceb2c0f97fdd638271a762c97a8b19d9a4a 100644 --- a/tools/testing/selftests/timens/timer.c +++ b/tools/testing/selftests/timens/timer.c @@ -15,7 +15,7 @@ #include "log.h" #include "timens.h"
-int run_test(int clockid, struct timespec now) +static int run_test(int clockid, struct timespec now) { struct itimerspec new_value; long long elapsed; diff --git a/tools/testing/selftests/timens/timerfd.c b/tools/testing/selftests/timens/timerfd.c index e58bc8b64ce2738dccb8c2a2c88e592c237b55d5..d6adf66bd8df064f5b76ccc265c13217d7a53f5b 100644 --- a/tools/testing/selftests/timens/timerfd.c +++ b/tools/testing/selftests/timens/timerfd.c @@ -22,7 +22,7 @@ static int tclock_gettime(clock_t clockid, struct timespec *now) return clock_gettime(clockid, now); }
-int run_test(int clockid, struct timespec now) +static int run_test(int clockid, struct timespec now) { struct itimerspec new_value; long long elapsed;
On 5/2/25 5:03 PM, Thomas Weißschuh wrote:
These functions are never used outside their defining compilation unit and can be made static.
Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de
Reviewed-by: Muhammad Usama Anjum usama.anjum@collabora.com
tools/testing/selftests/timens/clock_nanosleep.c | 2 +- tools/testing/selftests/timens/timer.c | 2 +- tools/testing/selftests/timens/timerfd.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/timens/clock_nanosleep.c b/tools/testing/selftests/timens/clock_nanosleep.c index 346bff3d128dfa8c8c743cc2eba74917994bb2f7..5cc0010e85ff00e00c89940a82268994a1769873 100644 --- a/tools/testing/selftests/timens/clock_nanosleep.c +++ b/tools/testing/selftests/timens/clock_nanosleep.c @@ -38,7 +38,7 @@ void *call_nanosleep(void *_args) return NULL; } -int run_test(int clockid, int abs) +static int run_test(int clockid, int abs) { struct timespec now = {}, rem; struct thread_args args = { .now = &now, .rem = &rem, .clockid = clockid}; diff --git a/tools/testing/selftests/timens/timer.c b/tools/testing/selftests/timens/timer.c index 51babe63e233bdaaef35c1cafbcb5b5cc91c021e..79543ceb2c0f97fdd638271a762c97a8b19d9a4a 100644 --- a/tools/testing/selftests/timens/timer.c +++ b/tools/testing/selftests/timens/timer.c @@ -15,7 +15,7 @@ #include "log.h" #include "timens.h" -int run_test(int clockid, struct timespec now) +static int run_test(int clockid, struct timespec now) { struct itimerspec new_value; long long elapsed; diff --git a/tools/testing/selftests/timens/timerfd.c b/tools/testing/selftests/timens/timerfd.c index e58bc8b64ce2738dccb8c2a2c88e592c237b55d5..d6adf66bd8df064f5b76ccc265c13217d7a53f5b 100644 --- a/tools/testing/selftests/timens/timerfd.c +++ b/tools/testing/selftests/timens/timerfd.c @@ -22,7 +22,7 @@ static int tclock_gettime(clock_t clockid, struct timespec *now) return clock_gettime(clockid, now); } -int run_test(int clockid, struct timespec now) +static int run_test(int clockid, struct timespec now) { struct itimerspec new_value; long long elapsed;
tclock_gettime() is a wrapper around clock_gettime(). The first parameter of clock_gettime() is of type "clockid_t", not "clock_t".
Use the correct type instead.
Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de --- tools/testing/selftests/timens/timerfd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/timens/timerfd.c b/tools/testing/selftests/timens/timerfd.c index d6adf66bd8df064f5b76ccc265c13217d7a53f5b..402e2e4155450d946796b8297a8be85f93cea16f 100644 --- a/tools/testing/selftests/timens/timerfd.c +++ b/tools/testing/selftests/timens/timerfd.c @@ -15,7 +15,7 @@ #include "log.h" #include "timens.h"
-static int tclock_gettime(clock_t clockid, struct timespec *now) +static int tclock_gettime(clockid_t clockid, struct timespec *now) { if (clockid == CLOCK_BOOTTIME_ALARM) clockid = CLOCK_BOOTTIME;
On 5/2/25 5:03 PM, Thomas Weißschuh wrote:
tclock_gettime() is a wrapper around clock_gettime(). The first parameter of clock_gettime() is of type "clockid_t", not "clock_t".
Use the correct type instead.
Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de
Reviewed-by: Muhammad Usama Anjum usama.anjum@collabora.com
tools/testing/selftests/timens/timerfd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/timens/timerfd.c b/tools/testing/selftests/timens/timerfd.c index d6adf66bd8df064f5b76ccc265c13217d7a53f5b..402e2e4155450d946796b8297a8be85f93cea16f 100644 --- a/tools/testing/selftests/timens/timerfd.c +++ b/tools/testing/selftests/timens/timerfd.c @@ -15,7 +15,7 @@ #include "log.h" #include "timens.h" -static int tclock_gettime(clock_t clockid, struct timespec *now) +static int tclock_gettime(clockid_t clockid, struct timespec *now) { if (clockid == CLOCK_BOOTTIME_ALARM) clockid = CLOCK_BOOTTIME;
linux-kselftest-mirror@lists.linaro.org
-
Kees Cook -
Muhammad Usama Anjum -
Shuah Khan -
Thomas Weißschuh