[LTP] [PATCH 0/6] Introduce a concept of test max_runtime

[LTP] [PATCH 0/6] Introduce a concept of test max_runtime

[Cyril Hrubis]

This patchset introduces a concept of test max_runtime and convert all
tests that were (mis)using timeout to get around timeouts when a test
runtime was longer than expected timeout.

Generally this removes quite a bit of code and makes things simpler
since it's easier to set up a runtime and compute test timeout based on
that rather than the other way around.

If this gets merged I may follow up with other changes, there are a few
tests that deliberately set up timeout to be 10 or 20 seconds in order
to timeout faster than the default 360. So logicall followup would be
setting the default timeout somewhat shorter and get rid of these as
well.

Cyril Hrubis (6):
  lib: tst_test: Move timeout scaling out of fork_testrun()
  lib: Add .max_runtime and tst_remaining_runtime()
  mtest06/mmap3: Convert to tst_remaining_runtime()
  syscalls/gettimeofday02: Convert to tst_remaining_runtime()
  cve-2015-3290: convert tst_remining_runtime()
  lib: Add tst_set_runtime() & remove tst_set_timeout()

 include/tst_fuzzy_sync.h                      |  7 ++-
 include/tst_test.h                            | 19 +++++-
 include/tst_timer_test.h                      |  3 +
 lib/newlib_tests/.gitignore                   |  4 +-
 lib/newlib_tests/test18.c                     | 22 -------
 lib/newlib_tests/test_max_runtime01.c         | 28 +++++++++
 lib/newlib_tests/test_max_runtime02.c         | 29 +++++++++
 lib/newlib_tests/test_max_runtime03.c         | 35 +++++++++++
 lib/newlib_tests/tst_fuzzy_sync01.c           |  1 +
 lib/newlib_tests/tst_fuzzy_sync02.c           |  1 +
 lib/newlib_tests/tst_fuzzy_sync03.c           |  1 +
 lib/tst_test.c                                | 62 ++++++++++++-------
 lib/tst_timer_test.c                          |  4 +-
 runtest/mm                                    |  2 +-
 testcases/cve/cve-2014-0196.c                 |  1 +
 testcases/cve/cve-2015-3290.c                 |  8 +--
 testcases/cve/cve-2016-7117.c                 |  1 +
 testcases/cve/cve-2017-2671.c                 |  1 +
 testcases/kernel/crypto/af_alg02.c            |  4 +-
 testcases/kernel/crypto/af_alg07.c            |  1 +
 testcases/kernel/crypto/pcrypt_aead01.c       |  3 +-
 testcases/kernel/mem/mtest01/mtest01.c        |  9 ++-
 testcases/kernel/mem/mtest06/mmap1.c          | 27 ++------
 testcases/kernel/mem/mtest06/mmap3.c          | 12 +---
 testcases/kernel/mem/thp/thp04.c              |  1 +
 testcases/kernel/pty/pty03.c                  |  1 +
 testcases/kernel/pty/pty05.c                  |  1 +
 testcases/kernel/sound/snd_seq01.c            |  2 +-
 testcases/kernel/sound/snd_timer01.c          |  1 +
 testcases/kernel/syscalls/bind/bind06.c       |  2 +-
 .../clock_nanosleep/clock_nanosleep02.c       |  1 +
 .../syscalls/epoll_pwait/epoll_pwait03.c      |  1 +
 .../kernel/syscalls/epoll_wait/epoll_wait02.c |  1 +
 .../kernel/syscalls/epoll_wait/epoll_wait04.c |  2 +-
 .../syscalls/futex/futex_cmp_requeue01.c      |  2 +-
 .../kernel/syscalls/futex/futex_wait05.c      |  1 +
 .../syscalls/gettimeofday/gettimeofday02.c    | 35 +----------
 testcases/kernel/syscalls/inotify/inotify09.c |  1 +
 .../kernel/syscalls/ipc/shmctl/shmctl05.c     |  2 +-
 .../kernel/syscalls/move_pages/move_pages12.c |  4 +-
 .../kernel/syscalls/nanosleep/nanosleep01.c   |  1 +
 testcases/kernel/syscalls/poll/poll02.c       |  1 +
 testcases/kernel/syscalls/prctl/prctl09.c     |  1 +
 testcases/kernel/syscalls/pselect/pselect01.c |  1 +
 testcases/kernel/syscalls/select/select02.c   |  1 +
 testcases/kernel/syscalls/sendmsg/sendmsg03.c |  1 +
 .../kernel/syscalls/setsockopt/setsockopt06.c |  1 +
 .../kernel/syscalls/setsockopt/setsockopt07.c |  1 +
 .../syscalls/timerfd/timerfd_settime02.c      |  1 +
 testcases/kernel/syscalls/writev/writev03.c   |  1 +
 testcases/network/can/cve/can_bcm01.c         |  1 +
 testcases/network/packet/fanout01.c           |  1 +
 testcases/network/sockets/vsock01.c           |  1 +
 53 files changed, 219 insertions(+), 137 deletions(-)
 delete mode 100644 lib/newlib_tests/test18.c
 create mode 100644 lib/newlib_tests/test_max_runtime01.c
 create mode 100644 lib/newlib_tests/test_max_runtime02.c
 create mode 100644 lib/newlib_tests/test_max_runtime03.c

-- 
2.32.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 1/6] lib: tst_test: Move timeout scaling out of fork_testrun()

[Cyril Hrubis]

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 02ae28335..71e938466 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -1348,7 +1348,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");
@@ -1363,24 +1363,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()");
@@ -1467,6 +1465,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.32.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 2/6] lib: Add .max_runtime and tst_remaining_runtime()

[Cyril Hrubis]

This is another attempt of decoupling test runtime from timeouts.

Fundamentally there are two types of tests in LTP. First type are tests
that are rather quick (much less than a second) and can live with
whatever default timeout we set up. Second type of tests are tests that
run in a loop until timeout or a number of iterations is reached, these
are the tests that are going to be converted to the .max_runtime added
by this patch and followups.

This patch does:

- adds .max_runtime to tst_test structure - this is the maximal runtime
  per test iteration

- if .max_runtime is set
  - test timeout is computed based on it and set as:
    max_runtime + MAX(10, max_runtime)

  - the -I option is mapped to the max runtime
    this nicely unifies the test runtime command line option

- replaces the tst_timeout_remining() whith tst_remaining_runtime() and
  simplifies the code since we do no have to keep any margin for the
  timeout, since we computed the timeout based on the runtime

Side efect of this work is that all tests that use
tst_remaining_runtime() must define it's runtime explicitelly which I
think is step into the right direction as well.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 include/tst_fuzzy_sync.h                      |  7 +++--
 include/tst_test.h                            | 11 +++++--
 lib/newlib_tests/.gitignore                   |  3 +-
 lib/newlib_tests/test18.c                     | 22 --------------
 lib/newlib_tests/test_max_runtime01.c         | 28 ++++++++++++++++++
 lib/newlib_tests/test_max_runtime02.c         | 29 +++++++++++++++++++
 lib/newlib_tests/tst_fuzzy_sync01.c           |  1 +
 lib/newlib_tests/tst_fuzzy_sync02.c           |  1 +
 lib/newlib_tests/tst_fuzzy_sync03.c           |  1 +
 lib/tst_test.c                                | 20 +++++++++----
 testcases/cve/cve-2014-0196.c                 |  1 +
 testcases/cve/cve-2016-7117.c                 |  1 +
 testcases/cve/cve-2017-2671.c                 |  1 +
 testcases/kernel/crypto/af_alg02.c            |  4 +--
 testcases/kernel/crypto/af_alg07.c            |  1 +
 testcases/kernel/crypto/pcrypt_aead01.c       |  3 +-
 testcases/kernel/mem/mtest01/mtest01.c        |  9 +++---
 testcases/kernel/mem/mtest06/mmap1.c          | 27 ++++-------------
 testcases/kernel/mem/thp/thp04.c              |  1 +
 testcases/kernel/pty/pty03.c                  |  1 +
 testcases/kernel/pty/pty05.c                  |  1 +
 testcases/kernel/sound/snd_seq01.c            |  2 +-
 testcases/kernel/sound/snd_timer01.c          |  1 +
 testcases/kernel/syscalls/bind/bind06.c       |  2 +-
 testcases/kernel/syscalls/inotify/inotify09.c |  1 +
 .../kernel/syscalls/ipc/shmctl/shmctl05.c     |  2 +-
 .../kernel/syscalls/move_pages/move_pages12.c |  4 +--
 testcases/kernel/syscalls/sendmsg/sendmsg03.c |  1 +
 .../kernel/syscalls/setsockopt/setsockopt06.c |  1 +
 .../kernel/syscalls/setsockopt/setsockopt07.c |  1 +
 .../syscalls/timerfd/timerfd_settime02.c      |  1 +
 testcases/kernel/syscalls/writev/writev03.c   |  1 +
 testcases/network/can/cve/can_bcm01.c         |  1 +
 testcases/network/packet/fanout01.c           |  1 +
 testcases/network/sockets/vsock01.c           |  1 +
 35 files changed, 127 insertions(+), 66 deletions(-)
 delete mode 100644 lib/newlib_tests/test18.c
 create mode 100644 lib/newlib_tests/test_max_runtime01.c
 create mode 100644 lib/newlib_tests/test_max_runtime02.c

diff --git a/include/tst_fuzzy_sync.h b/include/tst_fuzzy_sync.h
index 8f97bb8f6..42bda47f5 100644
--- a/include/tst_fuzzy_sync.h
+++ b/include/tst_fuzzy_sync.h
@@ -319,7 +319,10 @@ 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();
+
+	if (!pair->exec_time_start)
+		tst_brk(TBROK, "Make sure to set .max_runtime!");
 }
 
 /**
@@ -663,7 +666,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 3dcb45de0..a9746b440 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -224,6 +224,9 @@ struct tst_test {
 	/* override default timeout per test run, disabled == -1 */
 	int timeout;
 
+	/* Maximal runtime per _single_ iteration in seconds, used by tst_remaining_runtime() */
+	unsigned int max_runtime;
+
 	void (*setup)(void);
 	void (*cleanup)(void);
 
@@ -300,11 +303,15 @@ const char *tst_strsig(int sig);
  */
 const char *tst_strstatus(int status);
 
-unsigned int tst_timeout_remaining(void);
+/*
+ * Returns remaining test iteration 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);
 
-
 /*
  * Returns path to the test temporary directory in a newly allocated buffer.
  */
diff --git a/lib/newlib_tests/.gitignore b/lib/newlib_tests/.gitignore
index cf467b5a0..403076ba5 100644
--- a/lib/newlib_tests/.gitignore
+++ b/lib/newlib_tests/.gitignore
@@ -22,7 +22,6 @@ tst_safe_fileops
 tst_res_hexd
 tst_strstatus
 tst_print_result
-test18
 test19
 test20
 test22
@@ -47,3 +46,5 @@ tst_fuzzy_sync01
 tst_fuzzy_sync02
 tst_fuzzy_sync03
 test_zero_hugepage
+test_max_runtime01
+test_max_runtime02
diff --git a/lib/newlib_tests/test18.c b/lib/newlib_tests/test18.c
deleted file mode 100644
index 026435d7d..000000000
--- a/lib/newlib_tests/test18.c
+++ /dev/null
@@ -1,22 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-/*
- * Copyright (c) 2018, Linux Test Project
- */
-
-#include <stdlib.h>
-#include <unistd.h>
-#include "tst_test.h"
-
-static void run(void)
-{
-	do {
-		sleep(1);
-	} while (tst_timeout_remaining() >= 4);
-
-	tst_res(TPASS, "Timeout remaining: %d", tst_timeout_remaining());
-}
-
-static struct tst_test test = {
-	.test_all = run,
-	.timeout = 5
-};
diff --git a/lib/newlib_tests/test_max_runtime01.c b/lib/newlib_tests/test_max_runtime01.c
new file mode 100644
index 000000000..2d7dd2fc2
--- /dev/null
+++ b/lib/newlib_tests/test_max_runtime01.c
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2018, Linux Test Project
+ */
+
+#include <stdlib.h>
+#include <unistd.h>
+#include "tst_test.h"
+
+static void run(void)
+{
+	unsigned int ms = 0;
+
+	do {
+		usleep(1000);
+		ms++;
+	} while (tst_remaining_runtime());
+
+	if (ms > test.max_runtime * 1000)
+		tst_res(TFAIL, "Slept for longer than 1s %u", tst_remaining_runtime());
+	else
+		tst_res(TPASS, "Timeout remaining: %d, Slept for %ums", tst_remaining_runtime(), ms);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.max_runtime = 2
+};
diff --git a/lib/newlib_tests/test_max_runtime02.c b/lib/newlib_tests/test_max_runtime02.c
new file mode 100644
index 000000000..74812361e
--- /dev/null
+++ b/lib/newlib_tests/test_max_runtime02.c
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2018, Linux Test Project
+ */
+
+#include <stdlib.h>
+#include <unistd.h>
+#include "tst_test.h"
+
+static void run(void)
+{
+	unsigned int ms = 0;
+
+	do {
+		usleep(1000);
+		ms++;
+	} while (tst_remaining_runtime());
+
+	if (ms > 1000)
+		tst_res(TFAIL, "Slept for longer than 1s %u", tst_remaining_runtime());
+	else
+		tst_res(TPASS, "Timeout remaining: %d, Slept for %ums", tst_remaining_runtime(), ms);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.max_runtime = 1,
+	.test_variants = 2,
+};
diff --git a/lib/newlib_tests/tst_fuzzy_sync01.c b/lib/newlib_tests/tst_fuzzy_sync01.c
index ae3ea4e09..8e6ae127d 100644
--- a/lib/newlib_tests/tst_fuzzy_sync01.c
+++ b/lib/newlib_tests/tst_fuzzy_sync01.c
@@ -232,4 +232,5 @@ static struct tst_test test = {
 	.test = run,
 	.setup = setup,
 	.cleanup = cleanup,
+	.max_runtime = 1200,
 };
diff --git a/lib/newlib_tests/tst_fuzzy_sync02.c b/lib/newlib_tests/tst_fuzzy_sync02.c
index 51075f3c3..674ecc789 100644
--- a/lib/newlib_tests/tst_fuzzy_sync02.c
+++ b/lib/newlib_tests/tst_fuzzy_sync02.c
@@ -174,4 +174,5 @@ static struct tst_test test = {
 	.test = run,
 	.setup = setup,
 	.cleanup = cleanup,
+	.max_runtime = 60,
 };
diff --git a/lib/newlib_tests/tst_fuzzy_sync03.c b/lib/newlib_tests/tst_fuzzy_sync03.c
index 0d74e1eae..7113e2344 100644
--- a/lib/newlib_tests/tst_fuzzy_sync03.c
+++ b/lib/newlib_tests/tst_fuzzy_sync03.c
@@ -99,4 +99,5 @@ static struct tst_test test = {
 	.setup = setup,
 	.cleanup = cleanup,
 	.test_all = run,
+	.max_runtime = 60,
 };
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 71e938466..23f55b306 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -598,7 +598,10 @@ static void parse_opts(int argc, char *argv[])
 			iterations = atoi(optarg);
 		break;
 		case 'I':
-			duration = atof(optarg);
+			if (tst_test->max_runtime)
+				tst_test->max_runtime = atoi(optarg);
+			else
+				duration = atoi(optarg);
 		break;
 		case 'C':
 #ifdef UCLINUX
@@ -969,6 +972,13 @@ static void do_setup(int argc, char *argv[])
 
 	parse_opts(argc, argv);
 
+	if (tst_test->max_runtime) {
+		if (tst_test->timeout)
+			tst_brk(TBROK, "Only one of timeout and max_iter_runtime can be set!");
+
+		tst_test->timeout = tst_test->max_runtime + MAX(10u, tst_test->max_runtime);
+	}
+
 	if (tst_test->needs_root && geteuid() != 0)
 		tst_brk(TCONF, "Test needs to be run as root");
 
@@ -1307,7 +1317,7 @@ static void sigint_handler(int sig LTP_ATTRIBUTE_UNUSED)
 	}
 }
 
-unsigned int tst_timeout_remaining(void)
+unsigned int tst_remaining_runtime(void)
 {
 	static struct timespec now;
 	unsigned int elapsed;
@@ -1315,9 +1325,9 @@ unsigned int tst_timeout_remaining(void)
 	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;
+	elapsed = tst_timespec_diff_ms(now, tst_start_time)/1000;
+	if (tst_test->max_runtime > elapsed)
+		return tst_test->max_runtime - elapsed;
 
 	return 0;
 }
diff --git a/testcases/cve/cve-2014-0196.c b/testcases/cve/cve-2014-0196.c
index 012cbb7cd..05f9cf766 100644
--- a/testcases/cve/cve-2014-0196.c
+++ b/testcases/cve/cve-2014-0196.c
@@ -141,6 +141,7 @@ static struct tst_test test = {
 	.setup = setup,
 	.cleanup = cleanup,
 	.test_all = run,
+	.max_runtime = 360,
 	.tags = (const struct tst_tag[]) {
 		{"linux-git", "4291086b1f08"},
 		{"CVE", "2014-0196"},
diff --git a/testcases/cve/cve-2016-7117.c b/testcases/cve/cve-2016-7117.c
index dca002924..337ed347f 100644
--- a/testcases/cve/cve-2016-7117.c
+++ b/testcases/cve/cve-2016-7117.c
@@ -150,6 +150,7 @@ static struct tst_test test = {
 	.setup = setup,
 	.cleanup = cleanup,
 	.min_kver = "2.6.33",
+	.max_runtime = 360,
 	.tags = (const struct tst_tag[]) {
 		{"linux-git", "a2e2725541fa"},
 		{"CVE", "2016-7117"},
diff --git a/testcases/cve/cve-2017-2671.c b/testcases/cve/cve-2017-2671.c
index e72795d15..1993b0a13 100644
--- a/testcases/cve/cve-2017-2671.c
+++ b/testcases/cve/cve-2017-2671.c
@@ -109,6 +109,7 @@ static struct tst_test test = {
 	.test_all = run,
 	.cleanup = cleanup,
 	.needs_root = 1,
+	.max_runtime = 300,
 	.tags = (const struct tst_tag[]) {
 		{"linux-git", "43a6684519ab"},
 		{"CVE", "2017-2671"},
diff --git a/testcases/kernel/crypto/af_alg02.c b/testcases/kernel/crypto/af_alg02.c
index 9894ffacd..40d07ca90 100644
--- a/testcases/kernel/crypto/af_alg02.c
+++ b/testcases/kernel/crypto/af_alg02.c
@@ -65,7 +65,7 @@ static void run(void)
 	TST_CHECKPOINT_WAIT(0);
 
 	while (!completed) {
-		if (tst_timeout_remaining() <= 10) {
+		if (!tst_remaining_runtime()) {
 			pthread_cancel(thr);
 			tst_brk(TBROK,
 				"Timed out while reading from request socket.");
@@ -77,7 +77,7 @@ static void run(void)
 
 static struct tst_test test = {
 	.test_all = run,
-	.timeout = 20,
+	.max_runtime = 20,
 	.needs_checkpoints = 1,
 	.tags = (const struct tst_tag[]) {
 		{"linux-git", "ecaaab564978"},
diff --git a/testcases/kernel/crypto/af_alg07.c b/testcases/kernel/crypto/af_alg07.c
index ef13ad764..34fa1f620 100644
--- a/testcases/kernel/crypto/af_alg07.c
+++ b/testcases/kernel/crypto/af_alg07.c
@@ -125,6 +125,7 @@ static struct tst_test test = {
 	.cleanup = cleanup,
 	.min_kver = "4.10.0",
 	.min_cpus = 2,
+	.max_runtime = 360,
 	.taint_check = TST_TAINT_W | TST_TAINT_D,
 	.tags = (const struct tst_tag[]) {
 		{"linux-git", "ff7b11aa481f"},
diff --git a/testcases/kernel/crypto/pcrypt_aead01.c b/testcases/kernel/crypto/pcrypt_aead01.c
index 0609af9f6..69798dfea 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 at "
 				"%d iterations", i);
 			break;
@@ -74,6 +74,7 @@ static struct tst_test test = {
 	.setup = setup,
 	.test_all = run,
 	.cleanup = cleanup,
+	.max_runtime = 300,
 	.needs_root = 1,
 	.tags = (const struct tst_tag[]) {
 		{"linux-git", "d76c68109f37"},
diff --git a/testcases/kernel/mem/mtest01/mtest01.c b/testcases/kernel/mem/mtest01/mtest01.c
index 9676ea4b5..8ca70f37b 100644
--- a/testcases/kernel/mem/mtest01/mtest01.c
+++ b/testcases/kernel/mem/mtest01/mtest01.c
@@ -41,8 +41,6 @@
 #define ALLOC_THRESHOLD		(6*FIVE_HUNDRED_MB)
 #endif
 
-#define STOP_THRESHOLD 15	/* seconds remaining before reaching timeout */
-
 static pid_t *pid_list;
 static sig_atomic_t children_done;
 static int max_pids;
@@ -155,10 +153,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 +193,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");
 
@@ -234,6 +232,7 @@ static struct tst_test test = {
 		{"v",  &verbose,     	"-v  verbose"},
 		{}
 	},
+	.max_runtime = 300,
 	.setup = setup,
 	.cleanup = cleanup,
 	.test_all = mem_test,
diff --git a/testcases/kernel/mem/mtest06/mmap1.c b/testcases/kernel/mem/mtest06/mmap1.c
index 10c47c35c..b9fa5d107 100644
--- a/testcases/kernel/mem/mtest06/mmap1.c
+++ b/testcases/kernel/mem/mtest06/mmap1.c
@@ -35,17 +35,12 @@
 #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;
 static int num_iter = 5000;
-static float exec_time = 0.05; /* default is 3 min */
 
 static void *distant_area;
-static char *str_exec_time;
 static jmp_buf jmpbuf;
 static volatile unsigned char *map_address;
 static unsigned long page_sz;
@@ -206,17 +201,10 @@ static void setup(void)
 	SAFE_MUNMAP(distant_area, distant_mmap_size);
 	distant_area += distant_mmap_size / 2;
 
-	if (tst_parse_float(str_exec_time, &exec_time, 0, FLT_MAX)) {
-		tst_brk(TBROK, "Invalid number for exec_time '%s'",
-			str_exec_time);
-	}
-
 	sigptr.sa_sigaction = sig_handler;
 	sigemptyset(&sigptr.sa_mask);
 	sigptr.sa_flags = SA_SIGINFO | SA_NODEFER;
 	SAFE_SIGACTION(SIGSEGV, &sigptr, NULL);
-
-	tst_set_timeout((int)(exec_time * 3600));
 }
 
 static void run(void)
@@ -224,8 +212,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 +228,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 - last_update,
 				map_count, mapped_sigsegv_count,
 				threads_spawned);
 			tst_res(TINFO, "      repeated_reads: %ld, "
@@ -258,9 +246,6 @@ static void run(void)
 static struct tst_test test = {
 	.test_all = run,
 	.setup = setup,
-	.options = (struct tst_option[]) {
-		{"x:", &str_exec_time, "Exec time (hours)"},
-		{}
-	},
+	.max_runtime = 300,
 	.needs_tmpdir = 1,
 };
diff --git a/testcases/kernel/mem/thp/thp04.c b/testcases/kernel/mem/thp/thp04.c
index 985394dc3..a0971a3f6 100644
--- a/testcases/kernel/mem/thp/thp04.c
+++ b/testcases/kernel/mem/thp/thp04.c
@@ -163,6 +163,7 @@ static struct tst_test test = {
 	.test_all = run,
 	.setup = setup,
 	.cleanup = cleanup,
+	.max_runtime = 360,
 	.tags = (const struct tst_tag[]) {
 		{"linux-git", "a8f97366452e"},
 		{"linux-git", "8310d48b125d"},
diff --git a/testcases/kernel/pty/pty03.c b/testcases/kernel/pty/pty03.c
index 71bcb2eb6..2c06a9fbc 100644
--- a/testcases/kernel/pty/pty03.c
+++ b/testcases/kernel/pty/pty03.c
@@ -151,6 +151,7 @@ static struct tst_test test = {
 	.setup = setup,
 	.cleanup = cleanup,
 	.needs_root = 1,
+	.max_runtime = 360,
 	.tags = (const struct tst_tag[]) {
 		{"linux-git", "0ace17d568241"},
 		{"CVE", "2020-14416"},
diff --git a/testcases/kernel/pty/pty05.c b/testcases/kernel/pty/pty05.c
index afef051c8..93f903c5e 100644
--- a/testcases/kernel/pty/pty05.c
+++ b/testcases/kernel/pty/pty05.c
@@ -97,6 +97,7 @@ static struct tst_test test = {
 	.setup = setup,
 	.cleanup = cleanup,
 	.taint_check = TST_TAINT_W | TST_TAINT_D,
+	.max_runtime = 360,
 	.tags = (const struct tst_tag[]) {
 		{"linux-git", "82f2341c94d27"},
 		{"CVE", "2017-2636"},
diff --git a/testcases/kernel/sound/snd_seq01.c b/testcases/kernel/sound/snd_seq01.c
index c56752230..fb25537f3 100644
--- a/testcases/kernel/sound/snd_seq01.c
+++ b/testcases/kernel/sound/snd_seq01.c
@@ -123,7 +123,7 @@ static struct tst_test test = {
 	.tcnt = ARRAY_SIZE(testfunc_list),
 	.setup = setup,
 	.cleanup = cleanup,
-	.timeout = 120,
+	.max_runtime = 120,
 	.taint_check = TST_TAINT_W | TST_TAINT_D,
 	.tags = (const struct tst_tag[]) {
 		{"linux-git", "d15d662e89fc"},
diff --git a/testcases/kernel/sound/snd_timer01.c b/testcases/kernel/sound/snd_timer01.c
index 51591c18e..4446cbcbc 100644
--- a/testcases/kernel/sound/snd_timer01.c
+++ b/testcases/kernel/sound/snd_timer01.c
@@ -139,6 +139,7 @@ static struct tst_test test = {
 	.setup = setup,
 	.cleanup = cleanup,
 	.taint_check = TST_TAINT_W | TST_TAINT_D,
+	.max_runtime = 360,
 	.tags = (const struct tst_tag[]) {
 		{"linux-git", "d11662f4f798"},
 		{"linux-git", "ba3021b2c79b"},
diff --git a/testcases/kernel/syscalls/bind/bind06.c b/testcases/kernel/syscalls/bind/bind06.c
index 297311c04..08e528d9d 100644
--- a/testcases/kernel/syscalls/bind/bind06.c
+++ b/testcases/kernel/syscalls/bind/bind06.c
@@ -102,7 +102,7 @@ static struct tst_test test = {
 	.test_all = run,
 	.setup = setup,
 	.cleanup = cleanup,
-	.timeout = 600,
+	.max_runtime = 600,
 	.taint_check = TST_TAINT_W | TST_TAINT_D,
 	.needs_kconfigs = (const char *[]) {
 		"CONFIG_USER_NS=y",
diff --git a/testcases/kernel/syscalls/inotify/inotify09.c b/testcases/kernel/syscalls/inotify/inotify09.c
index fdfc9c078..240e4c5f0 100644
--- a/testcases/kernel/syscalls/inotify/inotify09.c
+++ b/testcases/kernel/syscalls/inotify/inotify09.c
@@ -94,6 +94,7 @@ static struct tst_test test = {
 	.setup = setup,
 	.cleanup = cleanup,
 	.test_all = verify_inotify,
+	.max_runtime = 360,
 	.tags = (const struct tst_tag[]) {
 		{"linux-git", "d90a10e2444b"},
 		{}
diff --git a/testcases/kernel/syscalls/ipc/shmctl/shmctl05.c b/testcases/kernel/syscalls/ipc/shmctl/shmctl05.c
index 8569322dc..191e6e82f 100644
--- a/testcases/kernel/syscalls/ipc/shmctl/shmctl05.c
+++ b/testcases/kernel/syscalls/ipc/shmctl/shmctl05.c
@@ -106,7 +106,7 @@ static void cleanup(void)
 }
 
 static struct tst_test test = {
-	.timeout = 20,
+	.max_runtime = 20,
 	.setup = setup,
 	.test_all = do_test,
 	.cleanup = cleanup,
diff --git a/testcases/kernel/syscalls/move_pages/move_pages12.c b/testcases/kernel/syscalls/move_pages/move_pages12.c
index 220130f4b..e43bc5888 100644
--- a/testcases/kernel/syscalls/move_pages/move_pages12.c
+++ b/testcases/kernel/syscalls/move_pages/move_pages12.c
@@ -153,7 +153,6 @@ static void do_test(unsigned int n)
 	void *ptr;
 	pid_t cpid = -1;
 	int status;
-	unsigned int twenty_percent = (tst_timeout_remaining() / 5);
 
 	addr = SAFE_MMAP(NULL, tcases[n].tpages * hpsz, PROT_READ | PROT_WRITE,
 		MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
@@ -198,7 +197,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())
 			break;
 	}
 
@@ -339,6 +338,7 @@ static struct tst_test test = {
 	.forks_child = 1,
 	.setup = setup,
 	.cleanup = cleanup,
+	.max_runtime = 60,
 	.test = do_test,
 	.tcnt = ARRAY_SIZE(tcases),
 	.tags = (const struct tst_tag[]) {
diff --git a/testcases/kernel/syscalls/sendmsg/sendmsg03.c b/testcases/kernel/syscalls/sendmsg/sendmsg03.c
index c7d72f686..716e2a96d 100644
--- a/testcases/kernel/syscalls/sendmsg/sendmsg03.c
+++ b/testcases/kernel/syscalls/sendmsg/sendmsg03.c
@@ -104,6 +104,7 @@ static struct tst_test test = {
 	.test_all = run,
 	.setup = setup,
 	.cleanup = cleanup,
+	.max_runtime = 360,
 	.taint_check = TST_TAINT_W | TST_TAINT_D,
 	.tags = (const struct tst_tag[]) {
 		{"linux-git", "8f659a03a0ba"},
diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt06.c b/testcases/kernel/syscalls/setsockopt/setsockopt06.c
index 12a80dee4..3d99e4fa0 100644
--- a/testcases/kernel/syscalls/setsockopt/setsockopt06.c
+++ b/testcases/kernel/syscalls/setsockopt/setsockopt06.c
@@ -122,6 +122,7 @@ static struct tst_test test = {
 	.setup = setup,
 	.cleanup = cleanup,
 	.taint_check = TST_TAINT_W | TST_TAINT_D,
+	.max_runtime = 360,
 	.needs_kconfigs = (const char *[]) {
 		"CONFIG_USER_NS=y",
 		"CONFIG_NET_NS=y",
diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt07.c b/testcases/kernel/syscalls/setsockopt/setsockopt07.c
index d2c568e3e..764798b92 100644
--- a/testcases/kernel/syscalls/setsockopt/setsockopt07.c
+++ b/testcases/kernel/syscalls/setsockopt/setsockopt07.c
@@ -134,6 +134,7 @@ static struct tst_test test = {
 	.test_all = run,
 	.setup = setup,
 	.cleanup = cleanup,
+	.max_runtime = 360,
 	.needs_kconfigs = (const char *[]) {
 		"CONFIG_USER_NS=y",
 		"CONFIG_NET_NS=y",
diff --git a/testcases/kernel/syscalls/timerfd/timerfd_settime02.c b/testcases/kernel/syscalls/timerfd/timerfd_settime02.c
index bd92ee964..226c06e1e 100644
--- a/testcases/kernel/syscalls/timerfd/timerfd_settime02.c
+++ b/testcases/kernel/syscalls/timerfd/timerfd_settime02.c
@@ -112,6 +112,7 @@ static struct tst_test test = {
 	.cleanup = cleanup,
 	.min_kver = "2.6.25",
 	.taint_check = TST_TAINT_W | TST_TAINT_D,
+	.max_runtime = 300,
 	.tags = (const struct tst_tag[]) {
 		{"linux-git", "1e38da300e1e"},
 		{"CVE", "2017-10661"},
diff --git a/testcases/kernel/syscalls/writev/writev03.c b/testcases/kernel/syscalls/writev/writev03.c
index d0c64743b..1c949a69f 100644
--- a/testcases/kernel/syscalls/writev/writev03.c
+++ b/testcases/kernel/syscalls/writev/writev03.c
@@ -146,6 +146,7 @@ static struct tst_test test = {
 	.min_cpus = 2,
 	.setup = setup,
 	.cleanup = cleanup,
+	.max_runtime = 140,
 	.tags = (const struct tst_tag[]) {
 		{"linux-git", "d4690f1e1cda"},
 		{}
diff --git a/testcases/network/can/cve/can_bcm01.c b/testcases/network/can/cve/can_bcm01.c
index 1c527da7a..ba3fe0d6c 100644
--- a/testcases/network/can/cve/can_bcm01.c
+++ b/testcases/network/can/cve/can_bcm01.c
@@ -137,6 +137,7 @@ static struct tst_test test = {
 	.cleanup = cleanup,
 	.taint_check = TST_TAINT_W | TST_TAINT_D,
 	.needs_root = 1,
+	.max_runtime = 360,
 	.needs_drivers = (const char *const[]) {
 		"vcan",
 		"can-bcm",
diff --git a/testcases/network/packet/fanout01.c b/testcases/network/packet/fanout01.c
index 5067d83a8..71eec6f94 100644
--- a/testcases/network/packet/fanout01.c
+++ b/testcases/network/packet/fanout01.c
@@ -106,6 +106,7 @@ static struct tst_test test = {
 	.test_all = run,
 	.cleanup = cleanup,
 	.needs_root = 1,
+	.max_runtime = 360,
 	.tags = (const struct tst_tag[]) {
 		{"CVE", "2017-15649"},
 		{"linux-git", "4971613c1639"},
diff --git a/testcases/network/sockets/vsock01.c b/testcases/network/sockets/vsock01.c
index a168e4401..dc3b34f6a 100644
--- a/testcases/network/sockets/vsock01.c
+++ b/testcases/network/sockets/vsock01.c
@@ -111,6 +111,7 @@ static struct tst_test test = {
 	.setup = setup,
 	.cleanup = cleanup,
 	.taint_check = TST_TAINT_W | TST_TAINT_D,
+	.max_runtime = 360,
 	.needs_kconfigs = (const char *[]) {
 		"CONFIG_VSOCKETS_LOOPBACK",
 		NULL
-- 
2.32.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/6] lib: Add .max_runtime and tst_remaining_runtime()

[Jan Stancek]

On Mon, Oct 25, 2021 at 6:01 PM Cyril Hrubis <chrubis@suse.cz> wrote:
>
> This is another attempt of decoupling test runtime from timeouts.
>
> Fundamentally there are two types of tests in LTP. First type are tests
> that are rather quick (much less than a second) and can live with
> whatever default timeout we set up. Second type of tests are tests that
> run in a loop until timeout or a number of iterations is reached, these
> are the tests that are going to be converted to the .max_runtime added
> by this patch and followups.
>
> This patch does:
>
> - adds .max_runtime to tst_test structure - this is the maximal runtime
>   per test iteration

I'm looking at the series, but I'm not sure I see much difference between
runtime and timeout. Other than some margin added by library (the MAX formula)
is there some other difference?

>
> - if .max_runtime is set
>   - test timeout is computed based on it and set as:
>     max_runtime + MAX(10, max_runtime)
>
>   - the -I option is mapped to the max runtime
>     this nicely unifies the test runtime command line option
>
> - replaces the tst_timeout_remining() whith tst_remaining_runtime() and
>   simplifies the code since we do no have to keep any margin for the
>   timeout, since we computed the timeout based on the runtime
>
> Side efect of this work is that all tests that use
> tst_remaining_runtime() must define it's runtime explicitelly which I
> think is step into the right direction as well.
>
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---

<snip>

> diff --git a/testcases/kernel/syscalls/move_pages/move_pages12.c b/testcases/kernel/syscalls/move_pages/move_pages12.c
> index 220130f4b..e43bc5888 100644
> --- a/testcases/kernel/syscalls/move_pages/move_pages12.c
> +++ b/testcases/kernel/syscalls/move_pages/move_pages12.c
> @@ -153,7 +153,6 @@ static void do_test(unsigned int n)
>         void *ptr;
>         pid_t cpid = -1;
>         int status;
> -       unsigned int twenty_percent = (tst_timeout_remaining() / 5);
>
>         addr = SAFE_MMAP(NULL, tcases[n].tpages * hpsz, PROT_READ | PROT_WRITE,
>                 MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
> @@ -198,7 +197,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())
>                         break;
>         }
>
> @@ -339,6 +338,7 @@ static struct tst_test test = {
>         .forks_child = 1,
>         .setup = setup,
>         .cleanup = cleanup,
> +       .max_runtime = 60,

Where does 60 come from?


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/6] lib: Add .max_runtime and tst_remaining_runtime()

[Cyril Hrubis]

Hi!
> > Fundamentally there are two types of tests in LTP. First type are tests
> > that are rather quick (much less than a second) and can live with
> > whatever default timeout we set up. Second type of tests are tests that
> > run in a loop until timeout or a number of iterations is reached, these
> > are the tests that are going to be converted to the .max_runtime added
> > by this patch and followups.
> >
> > This patch does:
> >
> > - adds .max_runtime to tst_test structure - this is the maximal runtime
> >   per test iteration
> 
> I'm looking at the series, but I'm not sure I see much difference between
> runtime and timeout. Other than some margin added by library (the MAX formula)
> is there some other difference?

Well it mostly makes the code simpler and clearer since we can, for
instance, map the -I option to the test runtime in the library, we got
rid of all the custom test options for that, etc.

But it also allows us to tune the timeouts based on the expected runtime
so that the test has enough time for a cleanup and we can do that in a
single place rather than in 20 different heuristics in a different
testcases.

> > diff --git a/testcases/kernel/syscalls/move_pages/move_pages12.c b/testcases/kernel/syscalls/move_pages/move_pages12.c
> > index 220130f4b..e43bc5888 100644
> > --- a/testcases/kernel/syscalls/move_pages/move_pages12.c
> > +++ b/testcases/kernel/syscalls/move_pages/move_pages12.c
> > @@ -153,7 +153,6 @@ static void do_test(unsigned int n)
> >         void *ptr;
> >         pid_t cpid = -1;
> >         int status;
> > -       unsigned int twenty_percent = (tst_timeout_remaining() / 5);
> >
> >         addr = SAFE_MMAP(NULL, tcases[n].tpages * hpsz, PROT_READ | PROT_WRITE,
> >                 MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
> > @@ -198,7 +197,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())
> >                         break;
> >         }
> >
> > @@ -339,6 +338,7 @@ static struct tst_test test = {
> >         .forks_child = 1,
> >         .setup = setup,
> >         .cleanup = cleanup,
> > +       .max_runtime = 60,
> 
> Where does 60 come from?

Ah, my bad, the code did tst_timeout_remaining() < aprox. 60 so this
should have been 300 instead. Good catch.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/6] lib: Add .max_runtime and tst_remaining_runtime()

[Richard Palethorpe]

Hello,

Cyril Hrubis <chrubis@suse.cz> writes:

> This is another attempt of decoupling test runtime from timeouts.

Do we have documentation explaining what runtimes and timeouts are?

These two words are interchangeable.

>
> Fundamentally there are two types of tests in LTP. First type are tests
> that are rather quick (much less than a second) and can live with
> whatever default timeout we set up. Second type of tests are tests that
> run in a loop until timeout or a number of iterations is reached, these
> are the tests that are going to be converted to the .max_runtime added
> by this patch and followups.

The code looks good, but this all feels quite hard to parse on a
more abstract level. I think it is because you are trying to over
generalise between different categories of test.

So we have tests which run to completion as fast as they can. Then we
have tests which iterate for some arbitrary time period (usually capped
by some number of iterations as well, but it's not important).

In the first case the concept of a target runtime makes no sense. So we
end up with 'max runtime' which is the same type of thing as the timeout
(although the value of timeout is greater).

In the second case, where the test tries to execute for some length of
time. Then the target runtime and timeout are really seperate things.

I would suggest renaming 'max_runtime' to just 'runtime' and make it
optional (defaults to 0). All tests which are of the second type can set
.runtime = DEFAULT_RUNTIME (or whatever). If the test tries to use
tst_remaining_runtime() without runtime being set, then an error is
thrown.

If runtime is present then it can simply be added to the timeout to
produce the "total timeout" (total_timeout = runtime + timeout).

This has the advantage of clearly showing in the meta data which tests
are likely to run for some time. Also it means that the concept of
'runtime' doesn't change depending on the type of test. Finally we can
set timeout very low by default.

So when calculating how long a test executable may run for we have

(runtime + timeout) * variants * iterations

The wording is still not great. runtime is more like "deliberate
runtime" and timeout is "maximal expected runtime after the deliberate
runtime".

-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/6] lib: Add .max_runtime and tst_remaining_runtime()

[Cyril Hrubis]

Hi!
> > This is another attempt of decoupling test runtime from timeouts.
> 
> Do we have documentation explaining what runtimes and timeouts are?
> 
> These two words are interchangeable.

Not yet I plan to rewrite documentation once things are finalized
enough.

> > Fundamentally there are two types of tests in LTP. First type are tests
> > that are rather quick (much less than a second) and can live with
> > whatever default timeout we set up. Second type of tests are tests that
> > run in a loop until timeout or a number of iterations is reached, these
> > are the tests that are going to be converted to the .max_runtime added
> > by this patch and followups.
> 
> The code looks good, but this all feels quite hard to parse on a
> more abstract level. I think it is because you are trying to over
> generalise between different categories of test.
> 
> So we have tests which run to completion as fast as they can. Then we
> have tests which iterate for some arbitrary time period (usually capped
> by some number of iterations as well, but it's not important).
> 
> In the first case the concept of a target runtime makes no sense. So we
> end up with 'max runtime' which is the same type of thing as the timeout
> (although the value of timeout is greater).
>
> In the second case, where the test tries to execute for some length of
> time. Then the target runtime and timeout are really seperate things.
> 
> I would suggest renaming 'max_runtime' to just 'runtime' and make it
> optional (defaults to 0). All tests which are of the second type can set
> .runtime = DEFAULT_RUNTIME (or whatever). If the test tries to use
> tst_remaining_runtime() without runtime being set, then an error is
> thrown.

That more of less what the patchset attempts to do. It hardcodes the
runtime as a number instead of having DEFAULT_RUNTIME constant though.

> If runtime is present then it can simply be added to the timeout to
> produce the "total timeout" (total_timeout = runtime + timeout).

I guess that would work reasonably well.

> This has the advantage of clearly showing in the meta data which tests
> are likely to run for some time. Also it means that the concept of
> 'runtime' doesn't change depending on the type of test. Finally we can
> set timeout very low by default.
> 
> So when calculating how long a test executable may run for we have
> 
> (runtime + timeout) * variants * iterations
>
> The wording is still not great. runtime is more like "deliberate
> runtime" and timeout is "maximal expected runtime after the deliberate
> runtime".

Sounds good. I will work on v2 that would include these changes.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 3/6] mtest06/mmap3: Convert to tst_remaining_runtime()

[Cyril Hrubis]

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 runtest/mm                           |  2 +-
 testcases/kernel/mem/mtest06/mmap3.c | 12 ++----------
 2 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/runtest/mm b/runtest/mm
index 6537666a9..4ecb61f24 100644
--- a/runtest/mm
+++ b/runtest/mm
@@ -16,7 +16,7 @@ mtest01w mtest01 -p80 -w
 mtest05   mmstress
 mtest06   mmap1
 mtest06_2 mmap2 -x 0.002 -a -p
-mtest06_3 mmap3 -x 0.002 -p
+mtest06_3 mmap3 -p
 # Remains diabled till the infinite loop problem is solved
 #mtest-6_4 shmat1 -x 0.00005
 
diff --git a/testcases/kernel/mem/mtest06/mmap3.c b/testcases/kernel/mem/mtest06/mmap3.c
index c18bd2f56..c6d853f98 100644
--- a/testcases/kernel/mem/mtest06/mmap3.c
+++ b/testcases/kernel/mem/mtest06/mmap3.c
@@ -23,11 +23,9 @@
 static char *str_loops;
 static char *str_threads;
 static char *map_private;
-static char *str_exec_time;
 
 static int loops = 1000;
 static int threads = 40;
-static float exec_time = 24;
 
 static volatile int sig_caught;
 static int threads_running;
@@ -109,7 +107,7 @@ static void test_mmap(void)
 	long i;
 	pthread_t thids[threads];
 
-	alarm(exec_time * 3600);
+	alarm(tst_remaining_runtime());
 
 	while (!sig_caught) {
 		for (i = 0; i < threads; i++) {
@@ -138,11 +136,6 @@ static void setup(void)
 	if (tst_parse_int(str_threads, &threads, 1, INT_MAX))
 		tst_brk(TBROK, "Invalid number of threads '%s'", str_threads);
 
-	if (tst_parse_float(str_exec_time, &exec_time, 0.0005, 9000))
-		tst_brk(TBROK, "Invalid execution time '%s'", str_exec_time);
-
-	tst_set_timeout(exec_time * 3600 + 300);
-
 	SAFE_SIGNAL(SIGALRM, sig_handler);
 	SAFE_SIGNAL(SIGBUS, sig_handler);
 	SAFE_SIGNAL(SIGSEGV, sig_handler);
@@ -155,7 +148,6 @@ static void setup(void)
 	tst_res(TINFO, "Number of loops %i", loops);
 	tst_res(TINFO, "Number of threads %i", threads);
 	tst_res(TINFO, "MAP_PRIVATE = %i", map_private ? 1 : 0);
-	tst_res(TINFO, "Execution time %fH", exec_time);
 }
 
 static void cleanup(void)
@@ -184,10 +176,10 @@ static struct tst_test test = {
 		{"l:", &str_loops, "-l uint  Number of map-write-unmap loops"},
 		{"n:", &str_threads, "-n uint  Number of worker threads"},
 		{"p", &map_private, "-p       Turns on MAP_PRIVATE (default MAP_SHARED)"},
-		{"x:", &str_exec_time, "-x float Execution time in hours (default 24H)"},
 		{}
 	},
 	.needs_tmpdir = 1,
+	.max_runtime = 10,
 	.setup = setup,
 	.cleanup = cleanup,
 	.test_all = test_mmap,
-- 
2.32.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 4/6] syscalls/gettimeofday02: Convert to tst_remaining_runtime()

[Cyril Hrubis]

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 .../syscalls/gettimeofday/gettimeofday02.c    | 35 ++-----------------
 1 file changed, 3 insertions(+), 32 deletions(-)

diff --git a/testcases/kernel/syscalls/gettimeofday/gettimeofday02.c b/testcases/kernel/syscalls/gettimeofday/gettimeofday02.c
index c0e56ed34..cdd5984b7 100644
--- a/testcases/kernel/syscalls/gettimeofday/gettimeofday02.c
+++ b/testcases/kernel/syscalls/gettimeofday/gettimeofday02.c
@@ -24,28 +24,15 @@
 #include "tst_timer.h"
 #include "lapi/syscalls.h"
 
-static volatile sig_atomic_t done;
-static char *str_rtime;
-static int rtime = 10;
-
-static void breakout(int sig)
-{
-	done = sig;
-}
-
 static void verify_gettimeofday(void)
 {
 	struct __kernel_old_timeval tv1, tv2;
 	unsigned long long cnt = 0;
 
-	done = 0;
-
-	alarm(rtime);
-
 	if (tst_syscall(__NR_gettimeofday, &tv1, NULL))
 		tst_brk(TFAIL | TERRNO, "gettimeofday() failed");
 
-	while (!done) {
+	while (tst_remaining_runtime()) {
 		if (tst_syscall(__NR_gettimeofday, &tv2, NULL))
 			tst_brk(TFAIL | TERRNO, "gettimeofday() failed");
 
@@ -63,26 +50,10 @@ static void verify_gettimeofday(void)
 	}
 
 	tst_res(TINFO, "gettimeofday() called %llu times", cnt);
-	tst_res(TPASS, "gettimeofday() monotonous in %i seconds", rtime);
-}
-
-static void setup(void)
-{
-	if (str_rtime) {
-		rtime = atoi(str_rtime);
-		if (rtime <= 0)
-			tst_brk(TBROK, "Invalid runtime '%s'", str_rtime);
-		tst_set_timeout(rtime + 60);
-	}
-
-	SAFE_SIGNAL(SIGALRM, breakout);
+	tst_res(TPASS, "gettimeofday() monotonous in %i seconds", test.max_runtime);
 }
 
 static struct tst_test test = {
-	.setup = setup,
-	.options = (struct tst_option[]) {
-		{"T:", &str_rtime, "-T len   Test iteration runtime in seconds"},
-		{},
-	},
+	.max_runtime = 10,
 	.test_all = verify_gettimeofday,
 };
-- 
2.32.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 5/6] cve-2015-3290: convert tst_remining_runtime()

[Cyril Hrubis]

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 testcases/cve/cve-2015-3290.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/testcases/cve/cve-2015-3290.c b/testcases/cve/cve-2015-3290.c
index 6c4fd57ac..9eb7127df 100644
--- a/testcases/cve/cve-2015-3290.c
+++ b/testcases/cve/cve-2015-3290.c
@@ -394,10 +394,6 @@ static void *child_thread(void *arg LTP_ATTRIBUTE_UNUSED)
 	return (void *)niter;
 }
 
-#define TIMEOUT		(180)
-#define TIME_TO_GIVEUP	(TIMEOUT - 5)
-#define TIMER_TYPE	CLOCK_MONOTONIC
-
 static void do_child(void)
 {
 	int i, ncpus;
@@ -414,7 +410,7 @@ static void do_child(void)
 	for (i = 0; i < ncpus; i++)
 		SAFE_PTHREAD_CREATE(&threads[i], NULL, child_thread, NULL);
 
-	sleep(TIME_TO_GIVEUP);
+	sleep(tst_remaining_runtime());
 	running = 0;
 
 	for (i = 0; i < ncpus; i++) {
@@ -467,7 +463,7 @@ static struct tst_test test = {
 	.needs_root = 1,
 	.needs_checkpoints = 1,
 	.setup = setup,
-	.timeout = TIMEOUT,
+	.max_runtime = 180,
 	.test_all = run,
 	.tags = (const struct tst_tag[]) {
 		{"linux-git", "9b6e6a8334d5"},
-- 
2.32.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 6/6] lib: Add tst_set_runtime() & remove tst_set_timeout()

[Cyril Hrubis]

Rarely there is a need to set the test runtime dynamically, the only
tests in LTP that does this are the timer tests that can get two
parameters, number of iterations and sleep time, and the test runtime is
close to the multiplication of these two.

It's still cleaner to set the runtime and let the test library figure
out the timeout in this case.

Also when no parameters are passed to these tests the runtime is a sum
of multiplications from the tst_timer_test.c source so we define a
constant in the tst_timer_test.h header and set the max_runtime in the
testcases accordingly. With this we get correct estimate for the test
runtime and tighter, but still forgiving enough, timeout as well.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 include/tst_test.h                            |  8 ++++-
 include/tst_timer_test.h                      |  3 ++
 lib/newlib_tests/.gitignore                   |  1 +
 lib/newlib_tests/test_max_runtime03.c         | 35 +++++++++++++++++++
 lib/tst_test.c                                | 35 +++++++++++--------
 lib/tst_timer_test.c                          |  4 +--
 .../clock_nanosleep/clock_nanosleep02.c       |  1 +
 .../syscalls/epoll_pwait/epoll_pwait03.c      |  1 +
 .../kernel/syscalls/epoll_wait/epoll_wait02.c |  1 +
 .../kernel/syscalls/epoll_wait/epoll_wait04.c |  2 +-
 .../syscalls/futex/futex_cmp_requeue01.c      |  2 +-
 .../kernel/syscalls/futex/futex_wait05.c      |  1 +
 .../kernel/syscalls/nanosleep/nanosleep01.c   |  1 +
 testcases/kernel/syscalls/poll/poll02.c       |  1 +
 testcases/kernel/syscalls/prctl/prctl09.c     |  1 +
 testcases/kernel/syscalls/pselect/pselect01.c |  1 +
 testcases/kernel/syscalls/select/select02.c   |  1 +
 17 files changed, 79 insertions(+), 20 deletions(-)
 create mode 100644 lib/newlib_tests/test_max_runtime03.c

diff --git a/include/tst_test.h b/include/tst_test.h
index a9746b440..3348eda96 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -309,8 +309,14 @@ const char *tst_strstatus(int status);
  */
 unsigned int tst_remaining_runtime(void);
 
+/*
+ * Sets the test runtime dymamically, this is supposed to be used from a test
+ * setup() in cases where the test runtime depends on parameters passed to the
+ * test.
+ */
+void tst_set_runtime(unsigned int runtime);
+
 unsigned int tst_multiply_timeout(unsigned int timeout);
-void tst_set_timeout(int timeout);
 
 /*
  * Returns path to the test temporary directory in a newly allocated buffer.
diff --git a/include/tst_timer_test.h b/include/tst_timer_test.h
index b825a4d1a..2a863391a 100644
--- a/include/tst_timer_test.h
+++ b/include/tst_timer_test.h
@@ -39,6 +39,9 @@
 
 void tst_timer_sample(void);
 
+/* This is a bit more than the sum of all the iterations the test does */
+#define TST_TIMER_TEST_MAX_RUNTIME 10
+
 # ifdef TST_NO_DEFAULT_MAIN
 struct tst_test *tst_timer_test_setup(struct tst_test *test);
 # endif /* TST_NO_DEFAULT_MAIN */
diff --git a/lib/newlib_tests/.gitignore b/lib/newlib_tests/.gitignore
index 403076ba5..1a33d8967 100644
--- a/lib/newlib_tests/.gitignore
+++ b/lib/newlib_tests/.gitignore
@@ -48,3 +48,4 @@ tst_fuzzy_sync03
 test_zero_hugepage
 test_max_runtime01
 test_max_runtime02
+test_max_runtime03
diff --git a/lib/newlib_tests/test_max_runtime03.c b/lib/newlib_tests/test_max_runtime03.c
new file mode 100644
index 000000000..5d1c1099c
--- /dev/null
+++ b/lib/newlib_tests/test_max_runtime03.c
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2021, Linux Test Project
+ */
+
+#include <stdlib.h>
+#include <unistd.h>
+#include "tst_test.h"
+
+#define MAX_RUNTIME 3
+
+static void run(void)
+{
+	unsigned int ms = 0;
+
+	do {
+		usleep(10000);
+		ms++;
+	} while (tst_remaining_runtime());
+
+	if (ms > MAX_RUNTIME * 100)
+		tst_res(TFAIL, "Slept for longer than 1s %u", tst_remaining_runtime());
+	else
+		tst_res(TPASS, "Timeout remaining: %d, Slept for %ums", tst_remaining_runtime(), 10 * ms);
+}
+
+static void setup(void)
+{
+	tst_set_runtime(MAX_RUNTIME);
+}
+
+static struct tst_test test = {
+	.setup = setup,
+	.test_all = run,
+};
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 23f55b306..9a1c23dce 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;
@@ -975,8 +976,6 @@ static void do_setup(int argc, char *argv[])
 	if (tst_test->max_runtime) {
 		if (tst_test->timeout)
 			tst_brk(TBROK, "Only one of timeout and max_iter_runtime can be set!");
-
-		tst_test->timeout = tst_test->max_runtime + MAX(10u, tst_test->max_runtime);
 	}
 
 	if (tst_test->needs_root && geteuid() != 0)
@@ -1326,8 +1325,8 @@ unsigned int tst_remaining_runtime(void)
 		tst_res(TWARN | TERRNO, "tst_clock_gettime() failed");
 
 	elapsed = tst_timespec_diff_ms(now, tst_start_time)/1000;
-	if (tst_test->max_runtime > elapsed)
-		return tst_test->max_runtime - elapsed;
+	if (results->max_runtime > elapsed)
+		return results->max_runtime - elapsed;
 
 	return 0;
 }
@@ -1358,16 +1357,13 @@ unsigned int tst_multiply_timeout(unsigned int timeout)
 	return timeout * timeout_mul;
 }
 
-static void set_timeout(int timeout)
+static int runtime_to_timeout(unsigned int runtime)
 {
-	if (timeout == -1) {
-		tst_res(TINFO, "Timeout per run is disabled");
-		return;
-	}
-
-	if (timeout < 1)
-		tst_brk(TBROK, "timeout must to be >= 1! (%d)", timeout);
+	return runtime + MAX(10u, runtime);
+}
 
+static void set_timeout(int timeout)
+{
 	results->timeout = tst_multiply_timeout(timeout);
 
 	tst_res(TINFO, "Timeout per run is %uh %02um %02us",
@@ -1375,9 +1371,16 @@ static void set_timeout(int timeout)
 		results->timeout % 60);
 }
 
-void tst_set_timeout(int timeout)
+static void set_runtime(unsigned int max_runtime)
+{
+	tst_res(TINFO, "Test max runtime %us", max_runtime);
+	results->max_runtime = max_runtime;
+	set_timeout(runtime_to_timeout(max_runtime));
+}
+
+void tst_set_runtime(unsigned int max_runtime)
 {
-	set_timeout(timeout);
+	set_runtime(max_runtime);
 	heartbeat();
 }
 
@@ -1475,7 +1478,9 @@ 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)
+	if (tst_test->max_runtime)
+		set_runtime(tst_test->max_runtime);
+	else if (tst_test->timeout)
 		set_timeout(tst_test->timeout);
 	else
 		set_timeout(300);
diff --git a/lib/tst_timer_test.c b/lib/tst_timer_test.c
index 3cd52fc9d..19de1ff97 100644
--- a/lib/tst_timer_test.c
+++ b/lib/tst_timer_test.c
@@ -441,9 +441,9 @@ static void parse_timer_opts(void)
 		if (!sample_cnt)
 			sample_cnt = 500;
 
-		long long timeout = sleep_time * sample_cnt / 1000000;
+		long long runtime = sleep_time * sample_cnt / 1000000;
 
-		tst_set_timeout(timeout + timeout/10);
+		tst_set_runtime(runtime);
 
 		test->test_all = single_timer_test;
 		test->test = NULL;
diff --git a/testcases/kernel/syscalls/clock_nanosleep/clock_nanosleep02.c b/testcases/kernel/syscalls/clock_nanosleep/clock_nanosleep02.c
index feb3e4791..656555652 100644
--- a/testcases/kernel/syscalls/clock_nanosleep/clock_nanosleep02.c
+++ b/testcases/kernel/syscalls/clock_nanosleep/clock_nanosleep02.c
@@ -32,5 +32,6 @@ int sample_fn(int clk_id, long long usec)
 
 static struct tst_test test = {
 	.scall = "clock_nanosleep()",
+	.max_runtime = TST_TIMER_TEST_MAX_RUNTIME,
 	.sample = sample_fn,
 };
diff --git a/testcases/kernel/syscalls/epoll_pwait/epoll_pwait03.c b/testcases/kernel/syscalls/epoll_pwait/epoll_pwait03.c
index 2ad1a6abc..19a5ea664 100644
--- a/testcases/kernel/syscalls/epoll_pwait/epoll_pwait03.c
+++ b/testcases/kernel/syscalls/epoll_pwait/epoll_pwait03.c
@@ -70,5 +70,6 @@ static struct tst_test test = {
 	.sample = sample_fn,
 	.setup = setup,
 	.cleanup = cleanup,
+	.max_runtime = TST_TIMER_TEST_MAX_RUNTIME,
 	.test_variants = TEST_VARIANTS,
 };
diff --git a/testcases/kernel/syscalls/epoll_wait/epoll_wait02.c b/testcases/kernel/syscalls/epoll_wait/epoll_wait02.c
index d2c0b6ef4..5e9986905 100644
--- a/testcases/kernel/syscalls/epoll_wait/epoll_wait02.c
+++ b/testcases/kernel/syscalls/epoll_wait/epoll_wait02.c
@@ -69,5 +69,6 @@ static struct tst_test test = {
 	.scall = "epoll_wait()",
 	.sample = sample_fn,
 	.setup = setup,
+	.max_runtime = TST_TIMER_TEST_MAX_RUNTIME,
 	.cleanup = cleanup,
 };
diff --git a/testcases/kernel/syscalls/epoll_wait/epoll_wait04.c b/testcases/kernel/syscalls/epoll_wait/epoll_wait04.c
index dc62e9202..623c044d2 100644
--- a/testcases/kernel/syscalls/epoll_wait/epoll_wait04.c
+++ b/testcases/kernel/syscalls/epoll_wait/epoll_wait04.c
@@ -13,7 +13,7 @@
 #include <sys/epoll.h>
 
 #include "tst_test.h"
-#include "tst_timer_test.h"
+#include "tst_timer.h"
 
 #define USEC_PRECISION 1000	/* Error margin allowed in usec */
 
diff --git a/testcases/kernel/syscalls/futex/futex_cmp_requeue01.c b/testcases/kernel/syscalls/futex/futex_cmp_requeue01.c
index 13e67c758..03cc544ab 100644
--- a/testcases/kernel/syscalls/futex/futex_cmp_requeue01.c
+++ b/testcases/kernel/syscalls/futex/futex_cmp_requeue01.c
@@ -15,7 +15,7 @@
 #include <linux/futex.h>
 #include <sys/time.h>
 
-#include "tst_timer_test.h"
+#include "tst_timer.h"
 #include "tst_test.h"
 #include "futextest.h"
 
diff --git a/testcases/kernel/syscalls/futex/futex_wait05.c b/testcases/kernel/syscalls/futex/futex_wait05.c
index 8fad5d858..f7d66133c 100644
--- a/testcases/kernel/syscalls/futex/futex_wait05.c
+++ b/testcases/kernel/syscalls/futex/futex_wait05.c
@@ -41,4 +41,5 @@ int sample_fn(int clk_id, long long usec)
 static struct tst_test test = {
 	.scall = "futex_wait()",
 	.sample = sample_fn,
+	.max_runtime = TST_TIMER_TEST_MAX_RUNTIME,
 };
diff --git a/testcases/kernel/syscalls/nanosleep/nanosleep01.c b/testcases/kernel/syscalls/nanosleep/nanosleep01.c
index eaacb89fa..9dec2276e 100644
--- a/testcases/kernel/syscalls/nanosleep/nanosleep01.c
+++ b/testcases/kernel/syscalls/nanosleep/nanosleep01.c
@@ -35,4 +35,5 @@ int sample_fn(int clk_id, long long usec)
 static struct tst_test test = {
 	.scall = "nanosleep()",
 	.sample = sample_fn,
+	.max_runtime = TST_TIMER_TEST_MAX_RUNTIME,
 };
diff --git a/testcases/kernel/syscalls/poll/poll02.c b/testcases/kernel/syscalls/poll/poll02.c
index c0665927b..7e2ae91f4 100644
--- a/testcases/kernel/syscalls/poll/poll02.c
+++ b/testcases/kernel/syscalls/poll/poll02.c
@@ -55,4 +55,5 @@ static struct tst_test test = {
 	.sample = sample_fn,
 	.setup = setup,
 	.cleanup = cleanup,
+	.max_runtime = TST_TIMER_TEST_MAX_RUNTIME,
 };
diff --git a/testcases/kernel/syscalls/prctl/prctl09.c b/testcases/kernel/syscalls/prctl/prctl09.c
index 07ce57063..be5583f98 100644
--- a/testcases/kernel/syscalls/prctl/prctl09.c
+++ b/testcases/kernel/syscalls/prctl/prctl09.c
@@ -44,4 +44,5 @@ static struct tst_test test = {
 	.setup = setup,
 	.scall = "prctl()",
 	.sample = sample_fn,
+	.max_runtime = TST_TIMER_TEST_MAX_RUNTIME,
 };
diff --git a/testcases/kernel/syscalls/pselect/pselect01.c b/testcases/kernel/syscalls/pselect/pselect01.c
index 5b2b8b3ba..5c97c386c 100644
--- a/testcases/kernel/syscalls/pselect/pselect01.c
+++ b/testcases/kernel/syscalls/pselect/pselect01.c
@@ -34,4 +34,5 @@ int sample_fn(int clk_id, long long usec)
 static struct tst_test test = {
 	.scall = "pselect()",
 	.sample = sample_fn,
+	.max_runtime = TST_TIMER_TEST_MAX_RUNTIME,
 };
diff --git a/testcases/kernel/syscalls/select/select02.c b/testcases/kernel/syscalls/select/select02.c
index 784ec9211..f3f2b0e9d 100644
--- a/testcases/kernel/syscalls/select/select02.c
+++ b/testcases/kernel/syscalls/select/select02.c
@@ -62,4 +62,5 @@ static struct tst_test test = {
 	.setup = setup,
 	.test_variants = TEST_VARIANTS,
 	.cleanup = cleanup,
+	.max_runtime = TST_TIMER_TEST_MAX_RUNTIME,
 };
-- 
2.32.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 6/6] lib: Add tst_set_runtime() & remove tst_set_timeout()

[Li Wang]

[-- Attachment #1.1: Type: text/plain, Size: 1115 bytes --]

On Tue, Oct 26, 2021 at 12:02 AM Cyril Hrubis <chrubis@suse.cz> wrote:

> Rarely there is a need to set the test runtime dynamically, the only
> tests in LTP that does this are the timer tests that can get two
> parameters, number of iterations and sleep time, and the test runtime is
> close to the multiplication of these two.
>
> It's still cleaner to set the runtime and let the test library figure
> out the timeout in this case.
>

If so, should we consider to hinden the .timeout in struct tst_test
to prevent users from changing it?

IIRC, we currently have ".timeout == -1" to disable test timed
out in unsure situation, e.g some OOM tests. But in this patch,
I saw you remove that, but not handle it in tst_set_runtime.



>
> Also when no parameters are passed to these tests the runtime is a sum
> of multiplications from the tst_timer_test.c source so we define a
> constant in the tst_timer_test.h header and set the max_runtime in the
> testcases accordingly. With this we get correct estimate for the test
> runtime and tighter, but still forgiving enough, timeout as well.
>


-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 2119 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 6/6] lib: Add tst_set_runtime() & remove tst_set_timeout()

[Cyril Hrubis]

Hi!
> > Rarely there is a need to set the test runtime dynamically, the only
> > tests in LTP that does this are the timer tests that can get two
> > parameters, number of iterations and sleep time, and the test runtime is
> > close to the multiplication of these two.
> >
> > It's still cleaner to set the runtime and let the test library figure
> > out the timeout in this case.
> >
> 
> If so, should we consider to hinden the .timeout in struct tst_test
> to prevent users from changing it?

If we decide to apply this patchset that would be logical end result.
There are only a few .timeout = foo left in the codebase after this
patchset that either disable timeout for the few unpredictable cases or
shorten it to make the test timeout faster if it gets stuck. We can deal
with these by making the .max_runtime accept -1 and by shortening the
default timeout considerably.

> IIRC, we currently have ".timeout == -1" to disable test timed
> out in unsure situation, e.g some OOM tests. But in this patch,
> I saw you remove that, but not handle it in tst_set_runtime.

Ah, right, I've removed the timeout == -1 handling by mistake. I wanted
to keep it working after this patchset as well until a follow up
patchset deals with the rest of the tests that set the .timeout.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 6/6] lib: Add tst_set_runtime() & remove tst_set_timeout()

[Li Wang]

[-- Attachment #1.1: Type: text/plain, Size: 2096 bytes --]

On Tue, Oct 26, 2021 at 3:13 PM Cyril Hrubis <chrubis@suse.cz> wrote:

> Hi!
> > > Rarely there is a need to set the test runtime dynamically, the only
> > > tests in LTP that does this are the timer tests that can get two
> > > parameters, number of iterations and sleep time, and the test runtime
> is
> > > close to the multiplication of these two.
> > >
> > > It's still cleaner to set the runtime and let the test library figure
> > > out the timeout in this case.
> > >
> >
> > If so, should we consider to hinden the .timeout in struct tst_test
> > to prevent users from changing it?
>
> If we decide to apply this patchset that would be logical end result.
> There are only a few .timeout = foo left in the codebase after this
> patchset that either disable timeout for the few unpredictable cases or
> shorten it to make the test timeout faster if it gets stuck. We can deal
> with these by making the .max_runtime accept -1 and by shortening the
> default timeout considerably.
>

Yes, that should be great.

After a quick reviewing the whole patchset, I feel that .timeout is
redundant since .max_runtime can do more thing to totally replace
it by the end.

----------------

Btw, it looks weird to simply double the runtime by plus MAX(10u, runtime)
in the runtime_to_timeout, I guess you probably just wanna another
10sec for some reclaiming work.

And the .max_runtime is also maximal time per test iteration,
but from the output below misleading me to think it is for the
whole test time.

See:

# LTP_TIMEOUT_MUL=1 ./pty03
tst_test.c:1376: TINFO: Test max runtime 360s
tst_test.c:1371: TINFO: Timeout per run is 0h 12m 00s
....



>
> > IIRC, we currently have ".timeout == -1" to disable test timed
> > out in unsure situation, e.g some OOM tests. But in this patch,
> > I saw you remove that, but not handle it in tst_set_runtime.
>
> Ah, right, I've removed the timeout == -1 handling by mistake. I wanted
> to keep it working after this patchset as well until a follow up
> patchset deals with the rest of the tests that set the .timeout.
>

Sound good.

-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 4137 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 6/6] lib: Add tst_set_runtime() & remove tst_set_timeout()

[Cyril Hrubis]

Hi!
> Yes, that should be great.
> 
> After a quick reviewing the whole patchset, I feel that .timeout is
> redundant since .max_runtime can do more thing to totally replace
> it by the end.

Agreed.

> ----------------
> 
> Btw, it looks weird to simply double the runtime by plus MAX(10u, runtime)
> in the runtime_to_timeout, I guess you probably just wanna another
> 10sec for some reclaiming work.

The exact formula is up for a discussion, but I do not think that we
should make it too tight, it's just a safety that is not going to be
triggered unless there is a real bug.

> And the .max_runtime is also maximal time per test iteration,
> but from the output below misleading me to think it is for the
> whole test time.
> 
> See:
> 
> # LTP_TIMEOUT_MUL=1 ./pty03
> tst_test.c:1376: TINFO: Test max runtime 360s
> tst_test.c:1371: TINFO: Timeout per run is 0h 12m 00s
> ....

Ah, right, that should be fixed. Anything else that should be fixed?

Also once we get this merged as well as the preprocessor support for the
metadata extractor it's going to be fairly easy to write a short script
that parses the metadata and then prints overall test runtime given a
test name, which is what I'm going to send once these two patchsets are
merged.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 6/6] lib: Add tst_set_runtime() & remove tst_set_timeout()

[Li Wang]

[-- Attachment #1.1: Type: text/plain, Size: 147 bytes --]

>
> Ah, right, that should be fixed. Anything else that should be fixed?
>

No more from my side, the rest looks quite good.

-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 545 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp