* [LTP] [PATCH 0/4] Introduce a concept of test runtime cap
@ 2021-06-09 11:46 Cyril Hrubis
2021-06-09 11:46 ` [LTP] [PATCH 1/4] lib: tst_supported_fs_types: Add tst_fs_max_types() Cyril Hrubis
` (4 more replies)
0 siblings, 5 replies; 14+ messages in thread
From: Cyril Hrubis @ 2021-06-09 11:46 UTC (permalink / raw)
To: ltp
Recently we had a problem with tests that use .all_filesystems and
tst_timeout_remaining().
The problem is that the tst_timeout_remaining() function wasn't aware of
the loop over fork_testrun() that executes a testrun for each supported
filesystem, which caused such test, e.g. writev03, to run for
unpredictable and long periods.
This is an attempt how to solve this properly by uncoupling timeouts
from test runtime so that test never runs for longer than its timeout.
Cyril Hrubis (4):
lib: tst_supported_fs_types: Add tst_fs_max_types()
lib: tst_test: Move timeout scaling out of fork_testrun()
lib: Introduce concept of max_test_runtime
syscalls/writev03: Adjust test runtime
include/tst_fs.h | 5 ++
include/tst_fuzzy_sync.h | 4 +-
include/tst_test.h | 7 +-
lib/newlib_tests/.gitignore | 3 +-
.../{test18.c => test_runtime01.c} | 7 +-
lib/newlib_tests/test_runtime02.c | 31 +++++++
lib/tst_supported_fs_types.c | 5 ++
lib/tst_test.c | 87 ++++++++++++++++---
testcases/kernel/crypto/af_alg02.c | 2 +-
testcases/kernel/crypto/pcrypt_aead01.c | 2 +-
testcases/kernel/mem/mtest01/mtest01.c | 6 +-
testcases/kernel/mem/mtest06/mmap1.c | 13 ++-
.../kernel/syscalls/move_pages/move_pages12.c | 4 +-
testcases/kernel/syscalls/writev/writev03.c | 1 +
14 files changed, 141 insertions(+), 36 deletions(-)
rename lib/newlib_tests/{test18.c => test_runtime01.c} (59%)
create mode 100644 lib/newlib_tests/test_runtime02.c
--
2.31.1
^ permalink raw reply [flat|nested] 14+ messages in thread* [LTP] [PATCH 1/4] lib: tst_supported_fs_types: Add tst_fs_max_types() 2021-06-09 11:46 [LTP] [PATCH 0/4] Introduce a concept of test runtime cap Cyril Hrubis @ 2021-06-09 11:46 ` Cyril Hrubis 2021-06-09 11:46 ` [LTP] [PATCH 2/4] lib: tst_test: Move timeout scaling out of fork_testrun() Cyril Hrubis ` (3 subsequent siblings) 4 siblings, 0 replies; 14+ messages in thread From: Cyril Hrubis @ 2021-06-09 11:46 UTC (permalink / raw) To: ltp The tst_fs_max_types() returns an upper bound on the number of supported filesystems. Signed-off-by: Cyril Hrubis <chrubis@suse.cz> --- include/tst_fs.h | 5 +++++ lib/tst_supported_fs_types.c | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/include/tst_fs.h b/include/tst_fs.h index 36d4b46f0..d1ed78cb5 100644 --- a/include/tst_fs.h +++ b/include/tst_fs.h @@ -195,6 +195,11 @@ const char **tst_get_supported_fs_types(const char *const *skiplist); */ int tst_fs_in_skiplist(const char *fs_type, const char *const *skiplist); +/* + * Returns maximal number of supported filesystem types. + */ +unsigned int tst_fs_max_types(void); + /* * Check whether device supports FS quotas. Negative return value means that * quotas appear to be broken. diff --git a/lib/tst_supported_fs_types.c b/lib/tst_supported_fs_types.c index 592a526ae..1de0f30b2 100644 --- a/lib/tst_supported_fs_types.c +++ b/lib/tst_supported_fs_types.c @@ -29,6 +29,11 @@ static const char *const fs_type_whitelist[] = { static const char *fs_types[ARRAY_SIZE(fs_type_whitelist)]; +unsigned int tst_fs_max_types(void) +{ + return ARRAY_SIZE(fs_type_whitelist) - 1; +} + static int has_mkfs(const char *fs_type) { char buf[128]; -- 2.31.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [LTP] [PATCH 2/4] lib: tst_test: Move timeout scaling out of fork_testrun() 2021-06-09 11:46 [LTP] [PATCH 0/4] Introduce a concept of test runtime cap Cyril Hrubis 2021-06-09 11:46 ` [LTP] [PATCH 1/4] lib: tst_supported_fs_types: Add tst_fs_max_types() Cyril Hrubis @ 2021-06-09 11:46 ` Cyril Hrubis 2021-06-09 11:46 ` [LTP] [PATCH 3/4] lib: Introduce concept of max_test_runtime Cyril Hrubis ` (2 subsequent siblings) 4 siblings, 0 replies; 14+ messages in thread From: Cyril Hrubis @ 2021-06-09 11:46 UTC (permalink / raw) To: ltp There is no point in setting the results->timeout in each iteration of the tests (either for .all_filesystems or .test_variants) and it's cleaner to do this once at the start of the testrun. This also fixes a case where .all_filesystems or .test_variants would call tst_set_timeout() in a test setup() that would be changed back on second and subsequent runs because we called the tst_set_timeout() incorrectly at the start of the fork_testrun(). Signed-off-by: Cyril Hrubis <chrubis@suse.cz> --- lib/tst_test.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/lib/tst_test.c b/lib/tst_test.c index 36a4809c7..7c9061d6d 100644 --- a/lib/tst_test.c +++ b/lib/tst_test.c @@ -1296,7 +1296,7 @@ unsigned int tst_multiply_timeout(unsigned int timeout) return timeout * timeout_mul; } -void tst_set_timeout(int timeout) +static void set_timeout(int timeout) { if (timeout == -1) { tst_res(TINFO, "Timeout per run is disabled"); @@ -1311,24 +1311,22 @@ void tst_set_timeout(int timeout) tst_res(TINFO, "Timeout per run is %uh %02um %02us", results->timeout/3600, (results->timeout%3600)/60, results->timeout % 60); +} - if (getpid() == lib_pid) - alarm(results->timeout); - else - heartbeat(); +void tst_set_timeout(int timeout) +{ + set_timeout(timeout); + heartbeat(); } static int fork_testrun(void) { int status; - if (tst_test->timeout) - tst_set_timeout(tst_test->timeout); - else - tst_set_timeout(300); - SAFE_SIGNAL(SIGINT, sigint_handler); + alarm(results->timeout); + test_pid = fork(); if (test_pid < 0) tst_brk(TBROK | TERRNO, "fork()"); @@ -1417,6 +1415,11 @@ void tst_run_tcases(int argc, char *argv[], struct tst_test *self) SAFE_SIGNAL(SIGALRM, alarm_handler); SAFE_SIGNAL(SIGUSR1, heartbeat_handler); + if (tst_test->timeout) + set_timeout(tst_test->timeout); + else + set_timeout(300); + if (tst_test->test_variants) test_variants = tst_test->test_variants; -- 2.31.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [LTP] [PATCH 3/4] lib: Introduce concept of max_test_runtime 2021-06-09 11:46 [LTP] [PATCH 0/4] Introduce a concept of test runtime cap Cyril Hrubis 2021-06-09 11:46 ` [LTP] [PATCH 1/4] lib: tst_supported_fs_types: Add tst_fs_max_types() Cyril Hrubis 2021-06-09 11:46 ` [LTP] [PATCH 2/4] lib: tst_test: Move timeout scaling out of fork_testrun() Cyril Hrubis @ 2021-06-09 11:46 ` Cyril Hrubis 2021-06-09 13:24 ` [LTP] [Automated-testing] " Petr Vorel 2021-06-09 14:44 ` [LTP] " Richard Palethorpe 2021-06-09 11:46 ` [LTP] [PATCH 4/4] syscalls/writev03: Adjust test runtime Cyril Hrubis 2021-06-09 14:54 ` [LTP] [PATCH 0/4] Introduce a concept of test runtime cap Richard Palethorpe 4 siblings, 2 replies; 14+ messages in thread From: Cyril Hrubis @ 2021-06-09 11:46 UTC (permalink / raw) To: ltp This is an attempt on how to handle a cap on a test runtime correctly it consists of several pieces namely: * The idea of test maximal runtime is uncoupled from test timeout - the maximal runtime is simply a cap for how long should an instance of a test run, it's mainly used by CVE reproducers that attempt to trigger a race until they run out of time, such test may exit sooner but must not run longer than the cap - the tst_timeout_remaining() is replaced with tst_remaining_runtime() which accounts correctly for .test_variants and .all_filesystems * The default value for a test max_runtime is computed from test timeout - we scale the timeout down so that the there is some room for test to properly exit once test runtime was exhausted, this is our base for a test max_runtime - the scaled value is then divided, if needed, so that we end up a correct maximal runtime for an instance of a test, i.e. we have max runtime for an instance fork_testrun() that is inside of .test_variants and .all_filesystems loops - this also allows us to controll the test max runtime by setting a test timeout * The maximal runtime, per whole test, can be passed down to the test - If LTP_MAX_TEST_RUNTIME is set in test environment it's used as a base for max_runtime instead of the scaled down timeout, it's still divided into pieces so that we have correct runtime cap for an fork_testrun() instance - We also make sure that test timeout is adjusted, if needed, to accomodate for the new test runtime cap, i.e. if upscaled runtime is greater than timeout, the test timeout is adjusted Signed-off-by: Cyril Hrubis <chrubis@suse.cz> --- include/tst_fuzzy_sync.h | 4 +- include/tst_test.h | 7 +- lib/newlib_tests/.gitignore | 3 +- .../{test18.c => test_runtime01.c} | 7 +- lib/newlib_tests/test_runtime02.c | 31 +++++++++ lib/tst_test.c | 64 ++++++++++++++++++- testcases/kernel/crypto/af_alg02.c | 2 +- testcases/kernel/crypto/pcrypt_aead01.c | 2 +- testcases/kernel/mem/mtest01/mtest01.c | 6 +- testcases/kernel/mem/mtest06/mmap1.c | 13 ++-- .../kernel/syscalls/move_pages/move_pages12.c | 4 +- 11 files changed, 117 insertions(+), 26 deletions(-) rename lib/newlib_tests/{test18.c => test_runtime01.c} (59%) create mode 100644 lib/newlib_tests/test_runtime02.c diff --git a/include/tst_fuzzy_sync.h b/include/tst_fuzzy_sync.h index 8f97bb8f6..93adbb909 100644 --- a/include/tst_fuzzy_sync.h +++ b/include/tst_fuzzy_sync.h @@ -319,7 +319,7 @@ static void tst_fzsync_pair_reset(struct tst_fzsync_pair *pair, SAFE_PTHREAD_CREATE(&pair->thread_b, 0, tst_fzsync_thread_wrapper, &wrap_run_b); } - pair->exec_time_start = (float)tst_timeout_remaining(); + pair->exec_time_start = (float)tst_remaining_runtime(); } /** @@ -663,7 +663,7 @@ static inline void tst_fzsync_wait_b(struct tst_fzsync_pair *pair) static inline int tst_fzsync_run_a(struct tst_fzsync_pair *pair) { int exit = 0; - float rem_p = 1 - tst_timeout_remaining() / pair->exec_time_start; + float rem_p = 1 - tst_remaining_runtime() / pair->exec_time_start; if ((pair->exec_time_p * SAMPLING_SLICE < rem_p) && (pair->sampling > 0)) { diff --git a/include/tst_test.h b/include/tst_test.h index 6ad355506..491fedc3e 100644 --- a/include/tst_test.h +++ b/include/tst_test.h @@ -290,7 +290,12 @@ const char *tst_strsig(int sig); */ const char *tst_strstatus(int status); -unsigned int tst_timeout_remaining(void); +/* + * Returns remaining test runtime. Test that runs for more than a few seconds + * should check if they should exit by calling this function regularly. + */ +unsigned int tst_remaining_runtime(void); + unsigned int tst_multiply_timeout(unsigned int timeout); void tst_set_timeout(int timeout); diff --git a/lib/newlib_tests/.gitignore b/lib/newlib_tests/.gitignore index b95ead2c2..464d98aed 100644 --- a/lib/newlib_tests/.gitignore +++ b/lib/newlib_tests/.gitignore @@ -23,7 +23,6 @@ tst_safe_fileops tst_res_hexd tst_strstatus test17 -test18 test19 test20 test22 @@ -43,3 +42,5 @@ test_macros02 test_macros03 tst_fuzzy_sync01 tst_fuzzy_sync02 +test_runtime01 +test_runtime02 diff --git a/lib/newlib_tests/test18.c b/lib/newlib_tests/test_runtime01.c similarity index 59% rename from lib/newlib_tests/test18.c rename to lib/newlib_tests/test_runtime01.c index 026435d7d..56f5ac44e 100644 --- a/lib/newlib_tests/test18.c +++ b/lib/newlib_tests/test_runtime01.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-or-later /* - * Copyright (c) 2018, Linux Test Project + * Copyright (c) 2021, Linux Test Project */ #include <stdlib.h> @@ -9,11 +9,10 @@ static void run(void) { - do { + while (tst_remaining_runtime()) sleep(1); - } while (tst_timeout_remaining() >= 4); - tst_res(TPASS, "Timeout remaining: %d", tst_timeout_remaining()); + tst_res(TPASS, "Timeout remaining: %d", tst_remaining_runtime()); } static struct tst_test test = { diff --git a/lib/newlib_tests/test_runtime02.c b/lib/newlib_tests/test_runtime02.c new file mode 100644 index 000000000..12e4813ef --- /dev/null +++ b/lib/newlib_tests/test_runtime02.c @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2021, Linux Test Project + */ +/* + * This test is set up so that the timeout is not long enough to guarantee + * enough runtime for two iterations, i.e. the timeout without offset and after + * scaling is too small and the tests ends up with TBROK. + * + * You can fix this by exporting LTP_MAX_TEST_RUNTIME=10 before executing the + * test, in that case the runtime would be divided between iterations and timeout + * adjusted so that it provides enough safeguards for the test to finish. + */ + +#include <stdlib.h> +#include <unistd.h> +#include "tst_test.h" + +static void run(void) +{ + while (tst_remaining_runtime()) + sleep(1); + + tst_res(TPASS, "Timeout remaining: %d", tst_remaining_runtime()); +} + +static struct tst_test test = { + .test_all = run, + .timeout = 5, + .test_variants = 2 +}; diff --git a/lib/tst_test.c b/lib/tst_test.c index 7c9061d6d..23b52583a 100644 --- a/lib/tst_test.c +++ b/lib/tst_test.c @@ -62,6 +62,7 @@ struct results { int warnings; int broken; unsigned int timeout; + unsigned int max_runtime; }; static struct results *results; @@ -1255,17 +1256,74 @@ static void sigint_handler(int sig LTP_ATTRIBUTE_UNUSED) } } -unsigned int tst_timeout_remaining(void) +#define RUNTIME_TIMEOUT_OFFSET 5 +#define RUNTIME_TIMEOUT_SCALE 0.9 + +static unsigned int timeout_to_runtime(void) +{ + if (results->timeout <= RUNTIME_TIMEOUT_OFFSET) { + tst_res(TWARN, "Timeout too short for runtime offset %i!", + RUNTIME_TIMEOUT_OFFSET); + return 1; + } + + return (results->timeout - RUNTIME_TIMEOUT_OFFSET) * RUNTIME_TIMEOUT_SCALE; +} + +static unsigned int runtime_to_timeout(unsigned int runtime) +{ + return runtime / RUNTIME_TIMEOUT_SCALE + RUNTIME_TIMEOUT_OFFSET; +} + +static unsigned int divide_runtime(unsigned int runtime) +{ + if (tst_test->test_variants) + runtime = 1.00 * runtime / tst_test->test_variants; + + if (tst_test->all_filesystems) + runtime = 1.00 * runtime / tst_fs_max_types(); + + return runtime; +} + +unsigned int tst_remaining_runtime(void) { static struct timespec now; unsigned int elapsed; + if (!results->max_runtime) { + const char *runtime = getenv("LTP_MAX_TEST_RUNTIME"); + + if (runtime) { + results->max_runtime = atoi(runtime); + } else { + results->max_runtime = timeout_to_runtime(); + } + + if (!results->max_runtime) + tst_brk(TBROK, "Test runtime too small!"); + + + if (runtime_to_timeout(results->max_runtime) > results->timeout) { + results->timeout = runtime_to_timeout(results->max_runtime); + tst_res(TINFO, "runtime > timeout, adjusting test timeout to %u", results->timeout); + heartbeat(); + } + + results->max_runtime = divide_runtime(results->max_runtime); + + if (!results->max_runtime) + tst_brk(TBROK, "Test runtime too small!"); + + tst_res(TINFO, "Max runtime per iteration %us", results->max_runtime); + } + if (tst_clock_gettime(CLOCK_MONOTONIC, &now)) tst_res(TWARN | TERRNO, "tst_clock_gettime() failed"); elapsed = (tst_timespec_diff_ms(now, tst_start_time) + 500) / 1000; - if (results->timeout > elapsed) - return results->timeout - elapsed; + if (results->max_runtime > elapsed) + return results->max_runtime - elapsed; return 0; } diff --git a/testcases/kernel/crypto/af_alg02.c b/testcases/kernel/crypto/af_alg02.c index 31d30777c..26f184854 100644 --- a/testcases/kernel/crypto/af_alg02.c +++ b/testcases/kernel/crypto/af_alg02.c @@ -61,7 +61,7 @@ static void run(void) TST_CHECKPOINT_WAIT(0); while (pthread_kill(thr, 0) != ESRCH) { - if (tst_timeout_remaining() <= 10) { + if (!tst_remaining_runtime()) { pthread_cancel(thr); tst_brk(TBROK, "Timed out while reading from request socket."); diff --git a/testcases/kernel/crypto/pcrypt_aead01.c b/testcases/kernel/crypto/pcrypt_aead01.c index 0609af9f6..7fe6aed14 100644 --- a/testcases/kernel/crypto/pcrypt_aead01.c +++ b/testcases/kernel/crypto/pcrypt_aead01.c @@ -55,7 +55,7 @@ void run(void) if (TST_RET) tst_brk(TBROK | TRERRNO, "del_alg"); - if (tst_timeout_remaining() < 10) { + if (!tst_remaining_runtime()) { tst_res(TINFO, "Time limit reached, stopping@" "%d iterations", i); break; diff --git a/testcases/kernel/mem/mtest01/mtest01.c b/testcases/kernel/mem/mtest01/mtest01.c index 9676ea4b5..b205722e3 100644 --- a/testcases/kernel/mem/mtest01/mtest01.c +++ b/testcases/kernel/mem/mtest01/mtest01.c @@ -155,10 +155,10 @@ static void child_loop_alloc(unsigned long long alloc_bytes) } if (dowrite) tst_res(TINFO, "... [t=%d] %lu bytes allocated and used in child %d", - tst_timeout_remaining(), bytecount, getpid()); + tst_remaining_runtime(), bytecount, getpid()); else tst_res(TINFO, "... [t=%d] %lu bytes allocated only in child %d", - tst_timeout_remaining(), bytecount, getpid()); + tst_remaining_runtime(), bytecount, getpid()); kill(getppid(), SIGRTMIN); raise(SIGSTOP); @@ -195,7 +195,7 @@ static void mem_test(void) /* wait in the loop for all children finish allocating */ while (children_done < pid_cntr) { - if (tst_timeout_remaining() < STOP_THRESHOLD) { + if (!tst_remaining_runtime()) { tst_res(TWARN, "the remaininig time is not enough for testing"); diff --git a/testcases/kernel/mem/mtest06/mmap1.c b/testcases/kernel/mem/mtest06/mmap1.c index 10c47c35c..699c20988 100644 --- a/testcases/kernel/mem/mtest06/mmap1.c +++ b/testcases/kernel/mem/mtest06/mmap1.c @@ -35,9 +35,6 @@ #define GIGABYTE (1L*1024*1024*1024) #define TEST_FILENAME "ashfile" -/* seconds remaining before reaching timeout */ -#define STOP_THRESHOLD 10 - #define PROGRESS_SEC 3 static int file_size = 1024; @@ -224,8 +221,8 @@ static void run(void) pthread_t thid[2]; int start, last_update; - start = last_update = tst_timeout_remaining(); - while (tst_timeout_remaining() > STOP_THRESHOLD) { + start = last_update = tst_remaining_runtime(); + while (tst_remaining_runtime()) { int fd = mkfile(file_size); tst_atomic_store(0, &mapcnt); @@ -240,11 +237,11 @@ static void run(void) close(fd); - if (last_update - tst_timeout_remaining() >= PROGRESS_SEC) { - last_update = tst_timeout_remaining(); + if (last_update - tst_remaining_runtime() >= PROGRESS_SEC) { + last_update = tst_remaining_runtime(); tst_res(TINFO, "[%03d] mapped: %lu, sigsegv hit: %lu, " "threads spawned: %lu", - start - tst_timeout_remaining(), + start - tst_remaining_runtime(), map_count, mapped_sigsegv_count, threads_spawned); tst_res(TINFO, " repeated_reads: %ld, " diff --git a/testcases/kernel/syscalls/move_pages/move_pages12.c b/testcases/kernel/syscalls/move_pages/move_pages12.c index 220130f4b..fa45c41a5 100644 --- a/testcases/kernel/syscalls/move_pages/move_pages12.c +++ b/testcases/kernel/syscalls/move_pages/move_pages12.c @@ -153,7 +153,7 @@ static void do_test(unsigned int n) void *ptr; pid_t cpid = -1; int status; - unsigned int twenty_percent = (tst_timeout_remaining() / 5); + unsigned int twenty_percent = (tst_remaining_runtime() / 5); addr = SAFE_MMAP(NULL, tcases[n].tpages * hpsz, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0); @@ -198,7 +198,7 @@ static void do_test(unsigned int n) SAFE_MUNMAP(addr, tcases[n].tpages * hpsz); - if (tst_timeout_remaining() < twenty_percent) + if (tst_remaining_runtime() < twenty_percent) break; } -- 2.31.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [LTP] [Automated-testing] [PATCH 3/4] lib: Introduce concept of max_test_runtime 2021-06-09 11:46 ` [LTP] [PATCH 3/4] lib: Introduce concept of max_test_runtime Cyril Hrubis @ 2021-06-09 13:24 ` Petr Vorel 2021-06-09 13:32 ` Cyril Hrubis 2021-06-09 14:44 ` [LTP] " Richard Palethorpe 1 sibling, 1 reply; 14+ messages in thread From: Petr Vorel @ 2021-06-09 13:24 UTC (permalink / raw) To: ltp Hi Cyril, > This is an attempt on how to handle a cap on a test runtime correctly it > consists of several pieces namely: > * The idea of test maximal runtime is uncoupled from test timeout > - the maximal runtime is simply a cap for how long should an instance > of a test run, it's mainly used by CVE reproducers that attempt to > trigger a race until they run out of time, such test may exit sooner > but must not run longer than the cap > - the tst_timeout_remaining() is replaced with tst_remaining_runtime() > which accounts correctly for .test_variants and .all_filesystems > * The default value for a test max_runtime is computed from test timeout > - we scale the timeout down so that the there is some room for test to > properly exit once test runtime was exhausted, this is our base for > a test max_runtime > - the scaled value is then divided, if needed, so that we end up a > correct maximal runtime for an instance of a test, i.e. we have > max runtime for an instance fork_testrun() that is inside of > .test_variants and .all_filesystems loops Now "Max runtime per iteration" can vary, right? I.e. on .all_filesystems runtime for each filesystems depends on number of filesystems? E.g. writev03.c with setup .timeout = 600 on 2 filesystems is 5 min (300s), but with all 9 filesystems is about 1 min. We should document that author should expect max number of filesystems. What happen with these values in the (long) future, when LTP support new filesystem (or drop some)? This was a reason for me to define in the test value for "Max runtime per iteration", not whole run. > - this also allows us to controll the test max runtime by setting a > test timeout > * The maximal runtime, per whole test, can be passed down to the test > - If LTP_MAX_TEST_RUNTIME is set in test environment it's used as a > base for max_runtime instead of the scaled down timeout, it's still > divided into pieces so that we have correct runtime cap for an > fork_testrun() instance LTP_MAX_TEST_RUNTIME should go to doc/user-guide.txt. I suppose you waiting for a feedback before writing docs. > - We also make sure that test timeout is adjusted, if needed, to > accomodate for the new test runtime cap, i.e. if upscaled runtime is > greater than timeout, the test timeout is adjusted > Signed-off-by: Cyril Hrubis <chrubis@suse.cz> > --- > include/tst_fuzzy_sync.h | 4 +- > include/tst_test.h | 7 +- > lib/newlib_tests/.gitignore | 3 +- > .../{test18.c => test_runtime01.c} | 7 +- > lib/newlib_tests/test_runtime02.c | 31 +++++++++ > lib/tst_test.c | 64 ++++++++++++++++++- > testcases/kernel/crypto/af_alg02.c | 2 +- > testcases/kernel/crypto/pcrypt_aead01.c | 2 +- > testcases/kernel/mem/mtest01/mtest01.c | 6 +- > testcases/kernel/mem/mtest06/mmap1.c | 13 ++-- > .../kernel/syscalls/move_pages/move_pages12.c | 4 +- > 11 files changed, 117 insertions(+), 26 deletions(-) > rename lib/newlib_tests/{test18.c => test_runtime01.c} (59%) +1 for test description instead of plain number. ... > +++ b/lib/newlib_tests/test_runtime01.c ... > static void run(void) > { > - do { > + while (tst_remaining_runtime()) > sleep(1); > - } while (tst_timeout_remaining() >= 4); > - tst_res(TPASS, "Timeout remaining: %d", tst_timeout_remaining()); > + tst_res(TPASS, "Timeout remaining: %d", tst_remaining_runtime()); There is a warning: tst_test.c:1369: TINFO: Timeout per run is 0h 00m 05s tst_test.c:1265: TWARN: Timeout too short for runtime offset 5! tst_test.c:1309: TINFO: runtime > timeout, adjusting test timeout to 6 tst_test.c:1318: TINFO: Max runtime per iteration 1s test_runtime01.c:15: TPASS: Timeout remaining: 0 Maybe test should use value without warning (i.e. 7). Or is the warning intended to be the test output? .timeout = 6 fails: tst_test.c:1369: TINFO: Timeout per run is 0h 00m 06s tst_test.c:1304: TBROK: Test runtime too small! .timeout >= 7 is ok: tst_test.c:1369: TINFO: Timeout per run is 0h 00m 07s tst_test.c:1318: TINFO: Max runtime per iteration 1s test_runtime01.c:15: TPASS: Timeout remaining: 0 ... > diff --git a/lib/tst_test.c b/lib/tst_test.c > index 7c9061d6d..23b52583a 100644 > --- a/lib/tst_test.c > +++ b/lib/tst_test.c > -unsigned int tst_timeout_remaining(void) > +#define RUNTIME_TIMEOUT_OFFSET 5 => maybe define it to 6 to allow running with .timeout = 6? > +#define RUNTIME_TIMEOUT_SCALE 0.9 > + > +static unsigned int timeout_to_runtime(void) > +{ > + if (results->timeout <= RUNTIME_TIMEOUT_OFFSET) { > + tst_res(TWARN, "Timeout too short for runtime offset %i!", > + RUNTIME_TIMEOUT_OFFSET); > + return 1; > + } > + > + return (results->timeout - RUNTIME_TIMEOUT_OFFSET) * RUNTIME_TIMEOUT_SCALE; > +} > + > +static unsigned int runtime_to_timeout(unsigned int runtime) > +{ > + return runtime / RUNTIME_TIMEOUT_SCALE + RUNTIME_TIMEOUT_OFFSET; > +} ... Also test_runtime02.c fails, is that intended? tst_test.c:1374: TINFO: Timeout per run is 0h 00m 05s tst_test.c:1265 timeout_to_runtime(): results->timeout: 5 tst_test.c:1266 timeout_to_runtime(): RUNTIME_TIMEOUT_OFFSET: 5 tst_test.c:1268: TWARN: Timeout too short for runtime offset 5! tst_test.c:1314: TINFO: runtime > timeout, adjusting test timeout to 6 tst_test.c:1321: TBROK: Test runtime too small! Kind regards, Petr ^ permalink raw reply [flat|nested] 14+ messages in thread
* [LTP] [Automated-testing] [PATCH 3/4] lib: Introduce concept of max_test_runtime 2021-06-09 13:24 ` [LTP] [Automated-testing] " Petr Vorel @ 2021-06-09 13:32 ` Cyril Hrubis 2021-06-09 14:05 ` Petr Vorel 2021-06-11 15:07 ` Martin Doucha 0 siblings, 2 replies; 14+ messages in thread From: Cyril Hrubis @ 2021-06-09 13:32 UTC (permalink / raw) To: ltp Hi! > > - the scaled value is then divided, if needed, so that we end up a > > correct maximal runtime for an instance of a test, i.e. we have > > max runtime for an instance fork_testrun() that is inside of > > .test_variants and .all_filesystems loops > Now "Max runtime per iteration" can vary, right? I.e. on .all_filesystems > runtime for each filesystems depends on number of filesystems? E.g. writev03.c > with setup .timeout = 600 on 2 filesystems is 5 min (300s), but with all 9 > filesystems is about 1 min. We should document that author should expect max > number of filesystems. What happen with these values in the (long) future, when > LTP support new filesystem (or drop some)? This was a reason for me to define in > the test value for "Max runtime per iteration", not whole run. That's one of the downsides of this approach. The reason why I choose this approach is that you can set upper cap for the whole test run and not only for a single filesystem/variant. Also this way the test timeout corresponds to the maximal test runtime. Another option would be to redefine the timeout to be timeout per a fork_testrun() instance, which would make the approach slightly easier in some places, however that would mean either changing default test timeout to much smaller value and annotating all long running tests. Hmm, I guess that annotating all long running tests and changing default timeout may be a good idea regardless this approach. > > - this also allows us to controll the test max runtime by setting a > > test timeout > > > * The maximal runtime, per whole test, can be passed down to the test > > > - If LTP_MAX_TEST_RUNTIME is set in test environment it's used as a > > base for max_runtime instead of the scaled down timeout, it's still > > divided into pieces so that we have correct runtime cap for an > > fork_testrun() instance > LTP_MAX_TEST_RUNTIME should go to doc/user-guide.txt. I suppose you waiting for > a feedback before writing docs. Yes I do not consider this to be finished patchset and I do expect that it would need some changes. > > - We also make sure that test timeout is adjusted, if needed, to > > accomodate for the new test runtime cap, i.e. if upscaled runtime is > > greater than timeout, the test timeout is adjusted > > > Signed-off-by: Cyril Hrubis <chrubis@suse.cz> > > --- > > include/tst_fuzzy_sync.h | 4 +- > > include/tst_test.h | 7 +- > > lib/newlib_tests/.gitignore | 3 +- > > .../{test18.c => test_runtime01.c} | 7 +- > > lib/newlib_tests/test_runtime02.c | 31 +++++++++ > > lib/tst_test.c | 64 ++++++++++++++++++- > > testcases/kernel/crypto/af_alg02.c | 2 +- > > testcases/kernel/crypto/pcrypt_aead01.c | 2 +- > > testcases/kernel/mem/mtest01/mtest01.c | 6 +- > > testcases/kernel/mem/mtest06/mmap1.c | 13 ++-- > > .../kernel/syscalls/move_pages/move_pages12.c | 4 +- > > 11 files changed, 117 insertions(+), 26 deletions(-) > > rename lib/newlib_tests/{test18.c => test_runtime01.c} (59%) > +1 for test description instead of plain number. > > ... > > +++ b/lib/newlib_tests/test_runtime01.c > ... > > static void run(void) > > { > > - do { > > + while (tst_remaining_runtime()) > > sleep(1); > > - } while (tst_timeout_remaining() >= 4); > > > - tst_res(TPASS, "Timeout remaining: %d", tst_timeout_remaining()); > > + tst_res(TPASS, "Timeout remaining: %d", tst_remaining_runtime()); > > There is a warning: > tst_test.c:1369: TINFO: Timeout per run is 0h 00m 05s > tst_test.c:1265: TWARN: Timeout too short for runtime offset 5! > tst_test.c:1309: TINFO: runtime > timeout, adjusting test timeout to 6 > tst_test.c:1318: TINFO: Max runtime per iteration 1s > test_runtime01.c:15: TPASS: Timeout remaining: 0 This is expected. > Maybe test should use value without warning (i.e. 7). > Or is the warning intended to be the test output? > > .timeout = 6 fails: > > tst_test.c:1369: TINFO: Timeout per run is 0h 00m 06s > tst_test.c:1304: TBROK: Test runtime too small! This is one of the corner cases that probably needs to be handled differently. > .timeout >= 7 is ok: > tst_test.c:1369: TINFO: Timeout per run is 0h 00m 07s > tst_test.c:1318: TINFO: Max runtime per iteration 1s > test_runtime01.c:15: TPASS: Timeout remaining: 0 > > ... > > diff --git a/lib/tst_test.c b/lib/tst_test.c > > index 7c9061d6d..23b52583a 100644 > > --- a/lib/tst_test.c > > +++ b/lib/tst_test.c > > > -unsigned int tst_timeout_remaining(void) > > +#define RUNTIME_TIMEOUT_OFFSET 5 > => maybe define it to 6 to allow running with .timeout = 6? > > > +#define RUNTIME_TIMEOUT_SCALE 0.9 > > + > > +static unsigned int timeout_to_runtime(void) > > +{ > > + if (results->timeout <= RUNTIME_TIMEOUT_OFFSET) { > > + tst_res(TWARN, "Timeout too short for runtime offset %i!", > > + RUNTIME_TIMEOUT_OFFSET); > > + return 1; > > + } > > + > > + return (results->timeout - RUNTIME_TIMEOUT_OFFSET) * RUNTIME_TIMEOUT_SCALE; > > +} > > + > > +static unsigned int runtime_to_timeout(unsigned int runtime) > > +{ > > + return runtime / RUNTIME_TIMEOUT_SCALE + RUNTIME_TIMEOUT_OFFSET; > > +} > ... > > Also test_runtime02.c fails, is that intended? > tst_test.c:1374: TINFO: Timeout per run is 0h 00m 05s > tst_test.c:1265 timeout_to_runtime(): results->timeout: 5 > tst_test.c:1266 timeout_to_runtime(): RUNTIME_TIMEOUT_OFFSET: 5 > tst_test.c:1268: TWARN: Timeout too short for runtime offset 5! > tst_test.c:1314: TINFO: runtime > timeout, adjusting test timeout to 6 > tst_test.c:1321: TBROK: Test runtime too small! Yes, this is also supposed to fail, it's written in the test comment as well... -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 14+ messages in thread
* [LTP] [Automated-testing] [PATCH 3/4] lib: Introduce concept of max_test_runtime 2021-06-09 13:32 ` Cyril Hrubis @ 2021-06-09 14:05 ` Petr Vorel 2021-06-09 13:43 ` Cyril Hrubis 2021-06-11 15:07 ` Martin Doucha 1 sibling, 1 reply; 14+ messages in thread From: Petr Vorel @ 2021-06-09 14:05 UTC (permalink / raw) To: ltp Hi Cyril, > Hi! > > > - the scaled value is then divided, if needed, so that we end up a > > > correct maximal runtime for an instance of a test, i.e. we have > > > max runtime for an instance fork_testrun() that is inside of > > > .test_variants and .all_filesystems loops > > Now "Max runtime per iteration" can vary, right? I.e. on .all_filesystems > > runtime for each filesystems depends on number of filesystems? E.g. writev03.c > > with setup .timeout = 600 on 2 filesystems is 5 min (300s), but with all 9 > > filesystems is about 1 min. We should document that author should expect max > > number of filesystems. What happen with these values in the (long) future, when > > LTP support new filesystem (or drop some)? This was a reason for me to define in > > the test value for "Max runtime per iteration", not whole run. > That's one of the downsides of this approach. > The reason why I choose this approach is that you can set upper cap for > the whole test run and not only for a single filesystem/variant. > Also this way the test timeout corresponds to the maximal test runtime. > Another option would be to redefine the timeout to be timeout per a > fork_testrun() instance, which would make the approach slightly easier > in some places, however that would mean either changing default test > timeout to much smaller value and annotating all long running tests. IMHO slightly better approach to me. > Hmm, I guess that annotating all long running tests and changing default > timeout may be a good idea regardless this approach. +1 > > > - this also allows us to controll the test max runtime by setting a > > > test timeout > > > * The maximal runtime, per whole test, can be passed down to the test > > > - If LTP_MAX_TEST_RUNTIME is set in test environment it's used as a > > > base for max_runtime instead of the scaled down timeout, it's still > > > divided into pieces so that we have correct runtime cap for an > > > fork_testrun() instance > > LTP_MAX_TEST_RUNTIME should go to doc/user-guide.txt. I suppose you waiting for > > a feedback before writing docs. > Yes I do not consider this to be finished patchset and I do expect that > it would need some changes. Sure. > > > - We also make sure that test timeout is adjusted, if needed, to > > > accomodate for the new test runtime cap, i.e. if upscaled runtime is > > > greater than timeout, the test timeout is adjusted > > > Signed-off-by: Cyril Hrubis <chrubis@suse.cz> > > > --- > > > include/tst_fuzzy_sync.h | 4 +- > > > include/tst_test.h | 7 +- > > > lib/newlib_tests/.gitignore | 3 +- > > > .../{test18.c => test_runtime01.c} | 7 +- > > > lib/newlib_tests/test_runtime02.c | 31 +++++++++ > > > lib/tst_test.c | 64 ++++++++++++++++++- > > > testcases/kernel/crypto/af_alg02.c | 2 +- > > > testcases/kernel/crypto/pcrypt_aead01.c | 2 +- > > > testcases/kernel/mem/mtest01/mtest01.c | 6 +- > > > testcases/kernel/mem/mtest06/mmap1.c | 13 ++-- > > > .../kernel/syscalls/move_pages/move_pages12.c | 4 +- > > > 11 files changed, 117 insertions(+), 26 deletions(-) > > > rename lib/newlib_tests/{test18.c => test_runtime01.c} (59%) > > +1 for test description instead of plain number. > > ... > > > +++ b/lib/newlib_tests/test_runtime01.c > > ... > > > static void run(void) > > > { > > > - do { > > > + while (tst_remaining_runtime()) > > > sleep(1); > > > - } while (tst_timeout_remaining() >= 4); > > > - tst_res(TPASS, "Timeout remaining: %d", tst_timeout_remaining()); > > > + tst_res(TPASS, "Timeout remaining: %d", tst_remaining_runtime()); > > There is a warning: > > tst_test.c:1369: TINFO: Timeout per run is 0h 00m 05s > > tst_test.c:1265: TWARN: Timeout too short for runtime offset 5! > > tst_test.c:1309: TINFO: runtime > timeout, adjusting test timeout to 6 > > tst_test.c:1318: TINFO: Max runtime per iteration 1s > > test_runtime01.c:15: TPASS: Timeout remaining: 0 > This is expected. > > Maybe test should use value without warning (i.e. 7). > > Or is the warning intended to be the test output? > > .timeout = 6 fails: > > tst_test.c:1369: TINFO: Timeout per run is 0h 00m 06s > > tst_test.c:1304: TBROK: Test runtime too small! > This is one of the corner cases that probably needs to be handled > differently. +1 ... > > Also test_runtime02.c fails, is that intended? > > tst_test.c:1374: TINFO: Timeout per run is 0h 00m 05s > > tst_test.c:1265 timeout_to_runtime(): results->timeout: 5 > > tst_test.c:1266 timeout_to_runtime(): RUNTIME_TIMEOUT_OFFSET: 5 > > tst_test.c:1268: TWARN: Timeout too short for runtime offset 5! > > tst_test.c:1314: TINFO: runtime > timeout, adjusting test timeout to 6 > > tst_test.c:1321: TBROK: Test runtime too small! > Yes, this is also supposed to fail, it's written in the test comment as > well... I'm sorry to overlook this. Hope I'll finish test-c-run soon, so that we can continue with expected test output for API tests. Kind regards, Petr ^ permalink raw reply [flat|nested] 14+ messages in thread
* [LTP] [Automated-testing] [PATCH 3/4] lib: Introduce concept of max_test_runtime 2021-06-09 14:05 ` Petr Vorel @ 2021-06-09 13:43 ` Cyril Hrubis 0 siblings, 0 replies; 14+ messages in thread From: Cyril Hrubis @ 2021-06-09 13:43 UTC (permalink / raw) To: ltp Hi! > > Another option would be to redefine the timeout to be timeout per a > > fork_testrun() instance, which would make the approach slightly easier > > in some places, however that would mean either changing default test > > timeout to much smaller value and annotating all long running tests. > IMHO slightly better approach to me. > > > Hmm, I guess that annotating all long running tests and changing default > > timeout may be a good idea regardless this approach. > +1 I can send a v2 if this approach ends up being prefered... -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 14+ messages in thread
* [LTP] [Automated-testing] [PATCH 3/4] lib: Introduce concept of max_test_runtime 2021-06-09 13:32 ` Cyril Hrubis 2021-06-09 14:05 ` Petr Vorel @ 2021-06-11 15:07 ` Martin Doucha 2021-06-13 19:44 ` Petr Vorel 1 sibling, 1 reply; 14+ messages in thread From: Martin Doucha @ 2021-06-11 15:07 UTC (permalink / raw) To: ltp On 09. 06. 21 15:32, Cyril Hrubis wrote: > Hi! >>> - the scaled value is then divided, if needed, so that we end up a >>> correct maximal runtime for an instance of a test, i.e. we have >>> max runtime for an instance fork_testrun() that is inside of >>> .test_variants and .all_filesystems loops >> Now "Max runtime per iteration" can vary, right? I.e. on .all_filesystems >> runtime for each filesystems depends on number of filesystems? E.g. writev03.c >> with setup .timeout = 600 on 2 filesystems is 5 min (300s), but with all 9 >> filesystems is about 1 min. We should document that author should expect max >> number of filesystems. What happen with these values in the (long) future, when >> LTP support new filesystem (or drop some)? This was a reason for me to define in >> the test value for "Max runtime per iteration", not whole run. > > That's one of the downsides of this approach. > > The reason why I choose this approach is that you can set upper cap for > the whole test run and not only for a single filesystem/variant. > > Also this way the test timeout corresponds to the maximal test runtime. > > Another option would be to redefine the timeout to be timeout per a > fork_testrun() instance, which would make the approach slightly easier > in some places, however that would mean either changing default test > timeout to much smaller value and annotating all long running tests. > > Hmm, I guess that annotating all long running tests and changing default > timeout may be a good idea regardless this approach. Some fuzzysync tests have long run time by design because running too few loops on broken systems will not trigger the bug. Limiting maximum program execution time may be useful for quick smoke tests but it's not usable for real test runs where we want reliable reproducibility. I'd prefer adding a command line option to tst_test (e.g. -m) that would just print test metadata, including total timeout of all fork_testrun() subtests, and exit. Static metadata is not a sufficient solution for this because the same test binary may have different runtimes on different system configurations, for example because the list of available filesystems may change arbitrarily between test runs. It'd be great if test runners other than runltp-ng could get a straighforward timeout number without reimplementing a calculation that may change in future versions of LTP. -- Martin Doucha mdoucha@suse.cz QA Engineer for Software Maintenance SUSE LINUX, s.r.o. CORSO IIa Krizikova 148/34 186 00 Prague 8 Czech Republic ^ permalink raw reply [flat|nested] 14+ messages in thread
* [LTP] [Automated-testing] [PATCH 3/4] lib: Introduce concept of max_test_runtime 2021-06-11 15:07 ` Martin Doucha @ 2021-06-13 19:44 ` Petr Vorel 2021-06-14 8:02 ` Richard Palethorpe 0 siblings, 1 reply; 14+ messages in thread From: Petr Vorel @ 2021-06-13 19:44 UTC (permalink / raw) To: ltp Hi all, > On 09. 06. 21 15:32, Cyril Hrubis wrote: > > Hi! > >>> - the scaled value is then divided, if needed, so that we end up a > >>> correct maximal runtime for an instance of a test, i.e. we have > >>> max runtime for an instance fork_testrun() that is inside of > >>> .test_variants and .all_filesystems loops > >> Now "Max runtime per iteration" can vary, right? I.e. on .all_filesystems > >> runtime for each filesystems depends on number of filesystems? E.g. writev03.c > >> with setup .timeout = 600 on 2 filesystems is 5 min (300s), but with all 9 > >> filesystems is about 1 min. We should document that author should expect max > >> number of filesystems. What happen with these values in the (long) future, when > >> LTP support new filesystem (or drop some)? This was a reason for me to define in > >> the test value for "Max runtime per iteration", not whole run. > > That's one of the downsides of this approach. > > The reason why I choose this approach is that you can set upper cap for > > the whole test run and not only for a single filesystem/variant. > > Also this way the test timeout corresponds to the maximal test runtime. > > Another option would be to redefine the timeout to be timeout per a > > fork_testrun() instance, which would make the approach slightly easier > > in some places, however that would mean either changing default test > > timeout to much smaller value and annotating all long running tests. > > Hmm, I guess that annotating all long running tests and changing default > > timeout may be a good idea regardless this approach. > Some fuzzysync tests have long run time by design because running too > few loops on broken systems will not trigger the bug. Limiting maximum > program execution time may be useful for quick smoke tests but it's not > usable for real test runs where we want reliable reproducibility. Interesting. > I'd prefer adding a command line option to tst_test (e.g. -m) that would > just print test metadata, including total timeout of all fork_testrun() > subtests, and exit. Static metadata is not a sufficient solution for FYI I suggested this some time ago with private chat with Cyril, he mentioned that there were some problems with it. IMHO it'd be great to implement it. > this because the same test binary may have different runtimes on > different system configurations, for example because the list of > available filesystems may change arbitrarily between test runs. It'd be > great if test runners other than runltp-ng could get a straighforward > timeout number without reimplementing a calculation that may change in > future versions of LTP. +1 Kind regards, Petr ^ permalink raw reply [flat|nested] 14+ messages in thread
* [LTP] [Automated-testing] [PATCH 3/4] lib: Introduce concept of max_test_runtime 2021-06-13 19:44 ` Petr Vorel @ 2021-06-14 8:02 ` Richard Palethorpe 0 siblings, 0 replies; 14+ messages in thread From: Richard Palethorpe @ 2021-06-14 8:02 UTC (permalink / raw) To: ltp Hello, Petr Vorel <pvorel@suse.cz> writes: > Hi all, > >> On 09. 06. 21 15:32, Cyril Hrubis wrote: >> > Hi! >> >>> - the scaled value is then divided, if needed, so that we end up a >> >>> correct maximal runtime for an instance of a test, i.e. we have >> >>> max runtime for an instance fork_testrun() that is inside of >> >>> .test_variants and .all_filesystems loops >> >> Now "Max runtime per iteration" can vary, right? I.e. on .all_filesystems >> >> runtime for each filesystems depends on number of filesystems? E.g. writev03.c >> >> with setup .timeout = 600 on 2 filesystems is 5 min (300s), but with all 9 >> >> filesystems is about 1 min. We should document that author should expect max >> >> number of filesystems. What happen with these values in the (long) future, when >> >> LTP support new filesystem (or drop some)? This was a reason for me to define in >> >> the test value for "Max runtime per iteration", not whole run. > >> > That's one of the downsides of this approach. > >> > The reason why I choose this approach is that you can set upper cap for >> > the whole test run and not only for a single filesystem/variant. > >> > Also this way the test timeout corresponds to the maximal test runtime. > >> > Another option would be to redefine the timeout to be timeout per a >> > fork_testrun() instance, which would make the approach slightly easier >> > in some places, however that would mean either changing default test >> > timeout to much smaller value and annotating all long running tests. > >> > Hmm, I guess that annotating all long running tests and changing default >> > timeout may be a good idea regardless this approach. > >> Some fuzzysync tests have long run time by design because running too >> few loops on broken systems will not trigger the bug. Limiting maximum >> program execution time may be useful for quick smoke tests but it's not >> usable for real test runs where we want reliable reproducibility. > Interesting. > >> I'd prefer adding a command line option to tst_test (e.g. -m) that would >> just print test metadata, including total timeout of all fork_testrun() >> subtests, and exit. Static metadata is not a sufficient solution for > FYI I suggested this some time ago with private chat with Cyril, he mentioned > that there were some problems with it. IMHO it'd be great to implement > it. Yes, it has been debated before. It may be an issue when cross compiling. Also verifying whether a test should really produce TCONF. I don't think it can be the primary way of extracting meta data. OTOH, it really makes sense for the test to report some info to the test runner. Including expected runtime and what environment it can see. The test runner can compare this data with its expectations. For example, if the test reports there is X NUMA nodes, but the runner thinks there should be Y NUMA nodes. This can help to verify people's configuration. > >> this because the same test binary may have different runtimes on >> different system configurations, for example because the list of >> available filesystems may change arbitrarily between test runs. It'd be >> great if test runners other than runltp-ng could get a straighforward >> timeout number without reimplementing a calculation that may change in >> future versions of LTP. Other possibilities are that a test takes much longer to run on single core or larger page size. I have also theorised before that fuzzysync could measure the first few loops and tune the timeouts based on that. I don't think it is necessary, but that can change. -- Thank you, Richard. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [LTP] [PATCH 3/4] lib: Introduce concept of max_test_runtime 2021-06-09 11:46 ` [LTP] [PATCH 3/4] lib: Introduce concept of max_test_runtime Cyril Hrubis 2021-06-09 13:24 ` [LTP] [Automated-testing] " Petr Vorel @ 2021-06-09 14:44 ` Richard Palethorpe 1 sibling, 0 replies; 14+ messages in thread From: Richard Palethorpe @ 2021-06-09 14:44 UTC (permalink / raw) To: ltp Hello Cyril, Cyril Hrubis <chrubis@suse.cz> writes: > This is an attempt on how to handle a cap on a test runtime correctly it > consists of several pieces namely: > > * The idea of test maximal runtime is uncoupled from test timeout > > - the maximal runtime is simply a cap for how long should an instance > of a test run, it's mainly used by CVE reproducers that attempt to > trigger a race until they run out of time, such test may exit sooner > but must not run longer than the cap > > - the tst_timeout_remaining() is replaced with tst_remaining_runtime() > which accounts correctly for .test_variants and .all_filesystems > > * The default value for a test max_runtime is computed from test timeout > > - we scale the timeout down so that the there is some room for test to > properly exit once test runtime was exhausted, this is our base for > a test max_runtime > > - the scaled value is then divided, if needed, so that we end up a > correct maximal runtime for an instance of a test, i.e. we have > max runtime for an instance fork_testrun() that is inside of > .test_variants and .all_filesystems loops > > - this also allows us to controll the test max runtime by setting a > test timeout > > * The maximal runtime, per whole test, can be passed down to the test > > - If LTP_MAX_TEST_RUNTIME is set in test environment it's used as a > base for max_runtime instead of the scaled down timeout, it's still > divided into pieces so that we have correct runtime cap for an > fork_testrun() instance > > - We also make sure that test timeout is adjusted, if needed, to > accomodate for the new test runtime cap, i.e. if upscaled runtime is > greater than timeout, the test timeout is adjusted > > Signed-off-by: Cyril Hrubis <chrubis@suse.cz> > --- > include/tst_fuzzy_sync.h | 4 +- > include/tst_test.h | 7 +- > lib/newlib_tests/.gitignore | 3 +- > .../{test18.c => test_runtime01.c} | 7 +- > lib/newlib_tests/test_runtime02.c | 31 +++++++++ > lib/tst_test.c | 64 ++++++++++++++++++- > testcases/kernel/crypto/af_alg02.c | 2 +- > testcases/kernel/crypto/pcrypt_aead01.c | 2 +- > testcases/kernel/mem/mtest01/mtest01.c | 6 +- > testcases/kernel/mem/mtest06/mmap1.c | 13 ++-- > .../kernel/syscalls/move_pages/move_pages12.c | 4 +- > 11 files changed, 117 insertions(+), 26 deletions(-) > rename lib/newlib_tests/{test18.c => test_runtime01.c} (59%) > create mode 100644 lib/newlib_tests/test_runtime02.c > > diff --git a/include/tst_fuzzy_sync.h b/include/tst_fuzzy_sync.h > index 8f97bb8f6..93adbb909 100644 > --- a/include/tst_fuzzy_sync.h > +++ b/include/tst_fuzzy_sync.h > @@ -319,7 +319,7 @@ static void tst_fzsync_pair_reset(struct tst_fzsync_pair *pair, > SAFE_PTHREAD_CREATE(&pair->thread_b, 0, tst_fzsync_thread_wrapper, &wrap_run_b); > } > > - pair->exec_time_start = (float)tst_timeout_remaining(); > + pair->exec_time_start = (float)tst_remaining_runtime(); > } > > /** > @@ -663,7 +663,7 @@ static inline void tst_fzsync_wait_b(struct tst_fzsync_pair *pair) > static inline int tst_fzsync_run_a(struct tst_fzsync_pair *pair) > { > int exit = 0; > - float rem_p = 1 - tst_timeout_remaining() / pair->exec_time_start; > + float rem_p = 1 - tst_remaining_runtime() / pair->exec_time_start; > > if ((pair->exec_time_p * SAMPLING_SLICE < rem_p) > && (pair->sampling > 0)) { > diff --git a/include/tst_test.h b/include/tst_test.h > index 6ad355506..491fedc3e 100644 > --- a/include/tst_test.h > +++ b/include/tst_test.h > @@ -290,7 +290,12 @@ const char *tst_strsig(int sig); > */ > const char *tst_strstatus(int status); > > -unsigned int tst_timeout_remaining(void); > +/* > + * Returns remaining test runtime. Test that runs for more than a few seconds > + * should check if they should exit by calling this function regularly. > + */ > +unsigned int tst_remaining_runtime(void); > + > unsigned int tst_multiply_timeout(unsigned int timeout); > void tst_set_timeout(int timeout); > > diff --git a/lib/newlib_tests/.gitignore b/lib/newlib_tests/.gitignore > index b95ead2c2..464d98aed 100644 > --- a/lib/newlib_tests/.gitignore > +++ b/lib/newlib_tests/.gitignore > @@ -23,7 +23,6 @@ tst_safe_fileops > tst_res_hexd > tst_strstatus > test17 > -test18 > test19 > test20 > test22 > @@ -43,3 +42,5 @@ test_macros02 > test_macros03 > tst_fuzzy_sync01 > tst_fuzzy_sync02 > +test_runtime01 > +test_runtime02 > diff --git a/lib/newlib_tests/test18.c b/lib/newlib_tests/test_runtime01.c > similarity index 59% > rename from lib/newlib_tests/test18.c > rename to lib/newlib_tests/test_runtime01.c > index 026435d7d..56f5ac44e 100644 > --- a/lib/newlib_tests/test18.c > +++ b/lib/newlib_tests/test_runtime01.c > @@ -1,6 +1,6 @@ > // SPDX-License-Identifier: GPL-2.0-or-later > /* > - * Copyright (c) 2018, Linux Test Project > + * Copyright (c) 2021, Linux Test Project > */ > > #include <stdlib.h> > @@ -9,11 +9,10 @@ > > static void run(void) > { > - do { > + while (tst_remaining_runtime()) > sleep(1); > - } while (tst_timeout_remaining() >= 4); > > - tst_res(TPASS, "Timeout remaining: %d", tst_timeout_remaining()); > + tst_res(TPASS, "Timeout remaining: %d", tst_remaining_runtime()); > } > > static struct tst_test test = { > diff --git a/lib/newlib_tests/test_runtime02.c b/lib/newlib_tests/test_runtime02.c > new file mode 100644 > index 000000000..12e4813ef > --- /dev/null > +++ b/lib/newlib_tests/test_runtime02.c > @@ -0,0 +1,31 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (c) 2021, Linux Test Project > + */ > +/* > + * This test is set up so that the timeout is not long enough to guarantee > + * enough runtime for two iterations, i.e. the timeout without offset and after > + * scaling is too small and the tests ends up with TBROK. > + * > + * You can fix this by exporting LTP_MAX_TEST_RUNTIME=10 before executing the > + * test, in that case the runtime would be divided between iterations and timeout > + * adjusted so that it provides enough safeguards for the test to finish. > + */ > + > +#include <stdlib.h> > +#include <unistd.h> > +#include "tst_test.h" > + > +static void run(void) > +{ > + while (tst_remaining_runtime()) > + sleep(1); > + > + tst_res(TPASS, "Timeout remaining: %d", tst_remaining_runtime()); > +} > + > +static struct tst_test test = { > + .test_all = run, > + .timeout = 5, > + .test_variants = 2 > +}; > diff --git a/lib/tst_test.c b/lib/tst_test.c > index 7c9061d6d..23b52583a 100644 > --- a/lib/tst_test.c > +++ b/lib/tst_test.c > @@ -62,6 +62,7 @@ struct results { > int warnings; > int broken; > unsigned int timeout; > + unsigned int max_runtime; > }; > > static struct results *results; > @@ -1255,17 +1256,74 @@ static void sigint_handler(int sig LTP_ATTRIBUTE_UNUSED) > } > } > > -unsigned int tst_timeout_remaining(void) > +#define RUNTIME_TIMEOUT_OFFSET 5 > +#define RUNTIME_TIMEOUT_SCALE 0.9 > + > +static unsigned int timeout_to_runtime(void) > +{ > + if (results->timeout <= RUNTIME_TIMEOUT_OFFSET) { > + tst_res(TWARN, "Timeout too short for runtime offset %i!", > + RUNTIME_TIMEOUT_OFFSET); > + return 1; > + } > + > + return (results->timeout - RUNTIME_TIMEOUT_OFFSET) * RUNTIME_TIMEOUT_SCALE; > +} > + > +static unsigned int runtime_to_timeout(unsigned int runtime) > +{ > + return runtime / RUNTIME_TIMEOUT_SCALE + RUNTIME_TIMEOUT_OFFSET; > +} > + > +static unsigned int divide_runtime(unsigned int runtime) > +{ > + if (tst_test->test_variants) > + runtime = 1.00 * runtime / tst_test->test_variants; > + > + if (tst_test->all_filesystems) > + runtime = 1.00 * runtime / tst_fs_max_types(); > + > + return runtime; > +} > + > +unsigned int tst_remaining_runtime(void) > { > static struct timespec now; > unsigned int elapsed; > > + if (!results->max_runtime) { > + const char *runtime = getenv("LTP_MAX_TEST_RUNTIME"); > + > + if (runtime) { > + results->max_runtime = atoi(runtime); POSIX says atoi is deprecated. It should probably be strtoul(). > + } else { > + results->max_runtime = timeout_to_runtime(); > + } > + > + if (!results->max_runtime) > + tst_brk(TBROK, "Test runtime too small!"); > + > + > + if (runtime_to_timeout(results->max_runtime) > > results->timeout) { Maybe should rename the "results" struct? It is turning into general shared test state. -- Thank you, Richard. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [LTP] [PATCH 4/4] syscalls/writev03: Adjust test runtime 2021-06-09 11:46 [LTP] [PATCH 0/4] Introduce a concept of test runtime cap Cyril Hrubis ` (2 preceding siblings ...) 2021-06-09 11:46 ` [LTP] [PATCH 3/4] lib: Introduce concept of max_test_runtime Cyril Hrubis @ 2021-06-09 11:46 ` Cyril Hrubis 2021-06-09 14:54 ` [LTP] [PATCH 0/4] Introduce a concept of test runtime cap Richard Palethorpe 4 siblings, 0 replies; 14+ messages in thread From: Cyril Hrubis @ 2021-06-09 11:46 UTC (permalink / raw) To: ltp Bump the test timeout to 600s which gives us about 1 minute of runtime per iteration. Signed-off-by: Cyril Hrubis <chrubis@suse.cz> --- testcases/kernel/syscalls/writev/writev03.c | 1 + 1 file changed, 1 insertion(+) diff --git a/testcases/kernel/syscalls/writev/writev03.c b/testcases/kernel/syscalls/writev/writev03.c index d0c64743b..5408d0279 100644 --- a/testcases/kernel/syscalls/writev/writev03.c +++ b/testcases/kernel/syscalls/writev/writev03.c @@ -144,6 +144,7 @@ static struct tst_test test = { .mntpoint = MNTPOINT, .all_filesystems = 1, .min_cpus = 2, + .timeout = 600, .setup = setup, .cleanup = cleanup, .tags = (const struct tst_tag[]) { -- 2.31.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [LTP] [PATCH 0/4] Introduce a concept of test runtime cap 2021-06-09 11:46 [LTP] [PATCH 0/4] Introduce a concept of test runtime cap Cyril Hrubis ` (3 preceding siblings ...) 2021-06-09 11:46 ` [LTP] [PATCH 4/4] syscalls/writev03: Adjust test runtime Cyril Hrubis @ 2021-06-09 14:54 ` Richard Palethorpe 4 siblings, 0 replies; 14+ messages in thread From: Richard Palethorpe @ 2021-06-09 14:54 UTC (permalink / raw) To: ltp Hello Cyril, Cyril Hrubis <chrubis@suse.cz> writes: > Recently we had a problem with tests that use .all_filesystems and > tst_timeout_remaining(). > > The problem is that the tst_timeout_remaining() function wasn't aware of > the loop over fork_testrun() that executes a testrun for each supported > filesystem, which caused such test, e.g. writev03, to run for > unpredictable and long periods. > > This is an attempt how to solve this properly by uncoupling timeouts > from test runtime so that test never runs for longer than its timeout. > > Cyril Hrubis (4): > lib: tst_supported_fs_types: Add tst_fs_max_types() > lib: tst_test: Move timeout scaling out of fork_testrun() > lib: Introduce concept of max_test_runtime > syscalls/writev03: Adjust test runtime > > include/tst_fs.h | 5 ++ > include/tst_fuzzy_sync.h | 4 +- > include/tst_test.h | 7 +- > lib/newlib_tests/.gitignore | 3 +- > .../{test18.c => test_runtime01.c} | 7 +- > lib/newlib_tests/test_runtime02.c | 31 +++++++ > lib/tst_supported_fs_types.c | 5 ++ > lib/tst_test.c | 87 ++++++++++++++++--- > testcases/kernel/crypto/af_alg02.c | 2 +- > testcases/kernel/crypto/pcrypt_aead01.c | 2 +- > testcases/kernel/mem/mtest01/mtest01.c | 6 +- > testcases/kernel/mem/mtest06/mmap1.c | 13 ++- > .../kernel/syscalls/move_pages/move_pages12.c | 4 +- > testcases/kernel/syscalls/writev/writev03.c | 1 + > 14 files changed, 141 insertions(+), 36 deletions(-) > rename lib/newlib_tests/{test18.c => test_runtime01.c} (59%) > create mode 100644 lib/newlib_tests/test_runtime02.c > > -- > 2.31.1 The changes generally look good. I think the new environment variable needs documenting in the user guide though. -- Thank you, Richard. ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-06-14 8:02 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-09 11:46 [LTP] [PATCH 0/4] Introduce a concept of test runtime cap Cyril Hrubis 2021-06-09 11:46 ` [LTP] [PATCH 1/4] lib: tst_supported_fs_types: Add tst_fs_max_types() Cyril Hrubis 2021-06-09 11:46 ` [LTP] [PATCH 2/4] lib: tst_test: Move timeout scaling out of fork_testrun() Cyril Hrubis 2021-06-09 11:46 ` [LTP] [PATCH 3/4] lib: Introduce concept of max_test_runtime Cyril Hrubis 2021-06-09 13:24 ` [LTP] [Automated-testing] " Petr Vorel 2021-06-09 13:32 ` Cyril Hrubis 2021-06-09 14:05 ` Petr Vorel 2021-06-09 13:43 ` Cyril Hrubis 2021-06-11 15:07 ` Martin Doucha 2021-06-13 19:44 ` Petr Vorel 2021-06-14 8:02 ` Richard Palethorpe 2021-06-09 14:44 ` [LTP] " Richard Palethorpe 2021-06-09 11:46 ` [LTP] [PATCH 4/4] syscalls/writev03: Adjust test runtime Cyril Hrubis 2021-06-09 14:54 ` [LTP] [PATCH 0/4] Introduce a concept of test runtime cap Richard Palethorpe
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.