[LTP] [PATCH v3 1/4] lib: Allow user to easily get LTP_TIMEOUT_MUL value

[LTP] [PATCH v3 1/4] lib: Allow user to easily get LTP_TIMEOUT_MUL value

[Richard Palethorpe]

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 include/tst_test.h |  1 +
 lib/tst_test.c     | 23 ++++++++++++++---------
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/include/tst_test.h b/include/tst_test.h
index 98dacf387..0ad9025f2 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -217,6 +217,7 @@ const char *tst_strsig(int sig);
  */
 const char *tst_strstatus(int status);
 
+float tst_timeout_mul(void);
 void tst_set_timeout(int timeout);
 
 #ifndef TST_NO_DEFAULT_MAIN
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 2f3d357d2..f760e05b3 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -992,26 +992,31 @@ static void sigint_handler(int sig LTP_ATTRIBUTE_UNUSED)
 	}
 }
 
-void tst_set_timeout(int timeout)
+float tst_timeout_mul(void)
 {
 	char *mul = getenv("LTP_TIMEOUT_MUL");
 
-	if (timeout == -1) {
-		tst_res(TINFO, "Timeout per run is disabled");
-		return;
-	}
-
-	results->timeout = timeout;
-
 	if (mul) {
 		float m = atof(mul);
 
 		if (m < 1)
 			tst_brk(TBROK, "Invalid timeout multiplier '%s'", mul);
 
-		results->timeout = results->timeout * m + 0.5;
+		return m;
+	}
+
+	return 1;
+}
+
+void tst_set_timeout(int timeout)
+{
+	if (timeout == -1) {
+		tst_res(TINFO, "Timeout per run is disabled");
+		return;
 	}
 
+	results->timeout = timeout * tst_timeout_mul() + 0.5;
+
 	tst_res(TINFO, "Timeout per run is %uh %02um %02us",
 		results->timeout/3600, (results->timeout%3600)/60,
 		results->timeout % 60);
-- 
2.18.0

[LTP] [PATCH v3 2/4] tst_timer: Convert to new library

[Richard Palethorpe]

So we can use functions from new lib.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 lib/tst_timer.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/lib/tst_timer.c b/lib/tst_timer.c
index 53ff36777..d5e4c49d2 100644
--- a/lib/tst_timer.c
+++ b/lib/tst_timer.c
@@ -23,7 +23,9 @@
 
 #include <errno.h>
 
-#include "test.h"
+#define TST_NO_DEFAULT_MAIN
+
+#include "tst_test.h"
 #include "tst_timer.h"
 #include "tst_clocks.h"
 #include "lapi/posix_clocks.h"
@@ -59,13 +61,13 @@ void tst_timer_check(clockid_t clk_id)
 {
 	if (tst_clock_gettime(clk_id, &start_time)) {
 		if (errno == EINVAL) {
-			tst_brkm(TCONF, NULL,
+			tst_brk(TCONF,
 			         "Clock id %s(%u) not supported by kernel",
 				 clock_name(clk_id), clk_id);
 			return;
 		}
 
-		tst_brkm(TBROK | TERRNO, NULL, "tst_clock_gettime() failed");
+		tst_brk(TBROK | TERRNO, "tst_clock_gettime() failed");
 	}
 }
 
@@ -74,7 +76,7 @@ void tst_timer_start(clockid_t clk_id)
 	clock_id = clk_id;
 
 	if (tst_clock_gettime(clock_id, &start_time))
-		tst_resm(TWARN | TERRNO, "tst_clock_gettime() failed");
+		tst_res(TWARN | TERRNO, "tst_clock_gettime() failed");
 }
 
 int tst_timer_expired_ms(long long ms)
@@ -82,7 +84,7 @@ int tst_timer_expired_ms(long long ms)
 	struct timespec cur_time;
 
 	if (tst_clock_gettime(clock_id, &cur_time))
-		tst_resm(TWARN | TERRNO, "tst_clock_gettime() failed");
+		tst_res(TWARN | TERRNO, "tst_clock_gettime() failed");
 
 	return tst_timespec_diff_ms(cur_time, start_time) >= ms;
 }
@@ -90,7 +92,7 @@ int tst_timer_expired_ms(long long ms)
 void tst_timer_stop(void)
 {
 	if (tst_clock_gettime(clock_id, &stop_time))
-		tst_resm(TWARN | TERRNO, "tst_clock_gettime() failed");
+		tst_res(TWARN | TERRNO, "tst_clock_gettime() failed");
 }
 
 struct timespec tst_timer_elapsed(void)
-- 
2.18.0

[LTP] [PATCH v3 3/4] tst_timer: Create interface for using multiple timers

[Richard Palethorpe]

Encapsulate information about a stopwatch timer in a struct and provide
methods for operating on that timer. This allows test authors to create
multiple timers in parallel.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 include/tst_timer.h | 30 ++++++++++++++++++++++++++++++
 lib/tst_timer.c     | 17 +++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/include/tst_timer.h b/include/tst_timer.h
index 0fd7ed6cf..06deae88a 100644
--- a/include/tst_timer.h
+++ b/include/tst_timer.h
@@ -34,6 +34,20 @@
 #include <sys/time.h>
 #include <time.h>
 
+/**
+ * Encapsulates a stopwatch timer.
+ *
+ * Useful when you need to use multiple timers in a single test.
+ */
+struct tst_timer {
+	/** The POSIX clock type  */
+	clockid_t clock_id;
+	/** How much time can pass before tst_timer_expired_st() returns true */
+	struct timespec limit;
+	/** Where to start measuring time from */
+	struct timespec start_time;
+};
+
 /*
  * Converts timespec to microseconds.
  */
@@ -251,6 +265,14 @@ void tst_timer_check(clockid_t clk_id);
  */
 void tst_timer_start(clockid_t clk_id);
 
+/**
+ * Sets the start time for timer tim.
+ *
+ * @relates tst_timer
+ * @param tim The timer to start
+ */
+void tst_timer_start_st(struct tst_timer *tim);
+
 /*
  * Returns true if timer started by tst_timer_start() has been running for
  * longer than ms seconds.
@@ -259,6 +281,14 @@ void tst_timer_start(clockid_t clk_id);
  */
 int tst_timer_expired_ms(long long ms);
 
+/**
+ * Returns true if timer tim has been running for longer than tst_timer::limit
+ *
+ * @relates tst_timer
+ * @param tim The timer to check
+ */
+int tst_timer_expired_st(struct tst_timer *tim);
+
 /*
  * Marks timer end time.
  */
diff --git a/lib/tst_timer.c b/lib/tst_timer.c
index d5e4c49d2..baec18280 100644
--- a/lib/tst_timer.c
+++ b/lib/tst_timer.c
@@ -79,6 +79,12 @@ void tst_timer_start(clockid_t clk_id)
 		tst_res(TWARN | TERRNO, "tst_clock_gettime() failed");
 }
 
+void tst_timer_start_st(struct tst_timer *tim)
+{
+	if (tst_clock_gettime(tim->clock_id, &tim->start_time))
+		tst_res(TWARN | TERRNO, "tst_clock_gettime() failed");
+}
+
 int tst_timer_expired_ms(long long ms)
 {
 	struct timespec cur_time;
@@ -89,6 +95,17 @@ int tst_timer_expired_ms(long long ms)
 	return tst_timespec_diff_ms(cur_time, start_time) >= ms;
 }
 
+int tst_timer_expired_st(struct tst_timer *tim)
+{
+	struct timespec cur_time;
+
+	if (tst_clock_gettime(tim->clock_id, &cur_time))
+		tst_res(TWARN | TERRNO, "tst_clock_gettime() failed");
+
+	return tst_timespec_lt(tim->limit,
+			       tst_timespec_diff(cur_time, tim->start_time));
+}
+
 void tst_timer_stop(void)
 {
 	if (tst_clock_gettime(clock_id, &stop_time))
-- 
2.18.0

[LTP] [PATCH v3 4/4] fzsync: Limit execution time to prevent test timeouts

[Richard Palethorpe]

Use the tst_timer library to check how long the main test loop has been
running for and break the loop if it exceeds (60 * LTP_TIMEOUT_MUL)
seconds. This prevents an overall test timeout which is reported as a failure.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 include/tst_fuzzy_sync.h                      | 50 +++++++++++++------
 lib/newlib_tests/test16.c                     |  4 +-
 testcases/cve/cve-2014-0196.c                 |  4 +-
 testcases/cve/cve-2016-7117.c                 |  6 ++-
 testcases/cve/cve-2017-2671.c                 |  4 +-
 testcases/kernel/syscalls/inotify/inotify09.c |  5 +-
 6 files changed, 51 insertions(+), 22 deletions(-)

diff --git a/include/tst_fuzzy_sync.h b/include/tst_fuzzy_sync.h
index bcc625e24..5e0ff3602 100644
--- a/include/tst_fuzzy_sync.h
+++ b/include/tst_fuzzy_sync.h
@@ -42,6 +42,7 @@
 #include <time.h>
 #include <math.h>
 #include "tst_atomic.h"
+#include "tst_timer.h"
 
 #ifndef CLOCK_MONOTONIC_RAW
 # define CLOCK_MONOTONIC_RAW CLOCK_MONOTONIC
@@ -66,6 +67,7 @@
  * @a_cntr: Internal; Atomic counter used by fzsync_pair_wait()
  * @b_cntr: Internal; Atomic counter used by fzsync_pair_wait()
  * @exit: Internal; Used by tst_fzsync_pair_exit() and fzsync_pair_wait()
+ * @timer: Internal; Used to limit the execution time.
  *
  * This contains all the necessary state for synchronising two points A and
  * B. Where A is the time of an event in one process and B is the time of an
@@ -87,6 +89,7 @@ struct tst_fzsync_pair {
 	int a_cntr;
 	int b_cntr;
 	int exit;
+	struct tst_timer timer;
 };
 
 /**
@@ -99,6 +102,15 @@ struct tst_fzsync_pair {
 	.info_gap = 0x7FFFF     \
 }
 
+static void tst_fzsync_pair_reset(struct tst_fzsync_pair *pair)
+{
+	pair->timer.clock_id = CLOCK_MONOTONIC_RAW;
+	pair->timer.limit.tv_sec = 60 * tst_timeout_mul();
+	pair->timer.limit.tv_nsec = 0;
+
+	tst_timer_start_st(&pair->timer);
+}
+
 /**
  * tst_fzsync_pair_info - Print some synchronisation statistics
  */
@@ -265,9 +277,7 @@ static inline int tst_fzsync_pair_wait(struct tst_fzsync_pair *pair,
 			;
 	}
 
-	/* Only exit if we have done the same amount of work as the other thread */
-	return !tst_atomic_load(&pair->exit) ||
-	  tst_atomic_load(other_cntr) <= tst_atomic_load(our_cntr);
+	return !tst_atomic_load(&pair->exit);
 }
 
 static inline int tst_fzsync_wait_a(struct tst_fzsync_pair *pair)
@@ -280,6 +290,17 @@ static inline int tst_fzsync_wait_b(struct tst_fzsync_pair *pair)
 	return tst_fzsync_pair_wait(pair, &pair->b_cntr, &pair->a_cntr);
 }
 
+/**
+ * tst_fzsync_pair_exit - Signal that the other thread should exit
+ *
+ * Causes tst_fzsync_pair_wait() and tst_fzsync_pair_wait_update() to return
+ * 0.
+ */
+static inline void tst_fzsync_pair_exit(struct tst_fzsync_pair *pair)
+{
+	tst_atomic_store(1, &pair->exit);
+}
+
 /**
  * tst_fzsync_pair_wait_update_{a,b} - Wait and then recalculate
  *
@@ -303,8 +324,16 @@ static inline int tst_fzsync_wait_update_a(struct tst_fzsync_pair *pair)
 	static int loop_index;
 
 	tst_fzsync_pair_wait(pair, &pair->a_cntr, &pair->b_cntr);
-	loop_index++;
-	tst_fzsync_pair_update(loop_index, pair);
+
+	if (tst_timer_expired_st(&pair->timer)) {
+		tst_res(TINFO,
+			"Exceeded fuzzy sync time limit, requesting exit");
+		tst_fzsync_pair_exit(pair);
+	} else {
+		loop_index++;
+		tst_fzsync_pair_update(loop_index, pair);
+	}
+
 	return tst_fzsync_pair_wait(pair, &pair->a_cntr, &pair->b_cntr);
 }
 
@@ -313,14 +342,3 @@ static inline int tst_fzsync_wait_update_b(struct tst_fzsync_pair *pair)
 	tst_fzsync_pair_wait(pair, &pair->b_cntr, &pair->a_cntr);
 	return tst_fzsync_pair_wait(pair, &pair->b_cntr, &pair->a_cntr);
 }
-
-/**
- * tst_fzsync_pair_exit - Signal that the other thread should exit
- *
- * Causes tst_fzsync_pair_wait() and tst_fzsync_pair_wait_update() to return
- * 0.
- */
-static inline void tst_fzsync_pair_exit(struct tst_fzsync_pair *pair)
-{
-	tst_atomic_store(1, &pair->exit);
-}
diff --git a/lib/newlib_tests/test16.c b/lib/newlib_tests/test16.c
index d80bd5369..3447dbc5a 100644
--- a/lib/newlib_tests/test16.c
+++ b/lib/newlib_tests/test16.c
@@ -64,8 +64,10 @@ static void run(void)
 {
 	unsigned int i, j, fail = 0;
 
+	tst_fzsync_pair_reset(&pair);
 	for (i = 0; i < LOOPS; i++) {
-		tst_fzsync_wait_update_a(&pair);
+		if (!tst_fzsync_wait_update_a(&pair))
+			break;
 		tst_fzsync_delay_a(&pair);
 		seq[seq_n] = 'A';
 		seq_n = i * 2 + 1;
diff --git a/testcases/cve/cve-2014-0196.c b/testcases/cve/cve-2014-0196.c
index d18108897..4a3f24026 100644
--- a/testcases/cve/cve-2014-0196.c
+++ b/testcases/cve/cve-2014-0196.c
@@ -102,6 +102,7 @@ static void run(void)
 
 	tst_res(TINFO, "Attempting to overflow into a tty_struct...");
 
+	tst_fzsync_pair_reset(&fzsync_pair);
 	for (i = 0; i < ATTEMPTS; i++) {
 		create_pty((int *)&master_fd, (int *)&slave_fd);
 
@@ -118,7 +119,8 @@ static void run(void)
 		t.c_lflag |= ECHO;
 		tcsetattr(master_fd, TCSANOW, &t);
 
-		tst_fzsync_wait_update_a(&fzsync_pair);
+		if (!tst_fzsync_wait_update_a(&fzsync_pair))
+			break;
 
 		tst_fzsync_delay_a(&fzsync_pair);
 		tst_fzsync_time_a(&fzsync_pair);
diff --git a/testcases/cve/cve-2016-7117.c b/testcases/cve/cve-2016-7117.c
index 3cb7efcdf..fecc5889f 100644
--- a/testcases/cve/cve-2016-7117.c
+++ b/testcases/cve/cve-2016-7117.c
@@ -136,11 +136,13 @@ static void run(void)
 
 	msghdrs[0].msg_hdr.msg_iov->iov_base = (void *)&rbuf;
 
+	tst_fzsync_pair_reset(&fzsync_pair);
 	for (i = 1; i < ATTEMPTS; i++) {
 		if (socketpair(AF_LOCAL, SOCK_DGRAM, 0, (int *)socket_fds))
 			tst_brk(TBROK | TERRNO, "Socket creation failed");
 
-		tst_fzsync_wait_update_a(&fzsync_pair);
+		if (!tst_fzsync_wait_update_a(&fzsync_pair))
+			break;
 		tst_fzsync_delay_a(&fzsync_pair);
 
 		stat = tst_syscall(__NR_recvmmsg,
@@ -156,7 +158,7 @@ static void run(void)
 		tst_fzsync_wait_a(&fzsync_pair);
 	}
 
-	tst_res(TPASS, "Nothing happened after %d attempts", ATTEMPTS);
+	tst_res(TPASS, "Nothing happened after %d attempts", i);
 }
 
 static struct tst_test test = {
diff --git a/testcases/cve/cve-2017-2671.c b/testcases/cve/cve-2017-2671.c
index b0471bfff..ce3022ce2 100644
--- a/testcases/cve/cve-2017-2671.c
+++ b/testcases/cve/cve-2017-2671.c
@@ -111,11 +111,13 @@ static void run(void)
 {
 	int i;
 
+	tst_fzsync_pair_reset(&fzsync_pair);
 	for (i = 0; i < ATTEMPTS; i++) {
 		SAFE_CONNECT(sockfd,
 			     (struct sockaddr *)&iaddr, sizeof(iaddr));
 
-		tst_fzsync_wait_update_a(&fzsync_pair);
+		if (!tst_fzsync_wait_update_a(&fzsync_pair))
+			break;
 		tst_fzsync_delay_a(&fzsync_pair);
 		connect(sockfd, (struct sockaddr *)&uaddr, sizeof(uaddr));
 		tst_fzsync_time_a(&fzsync_pair);
diff --git a/testcases/kernel/syscalls/inotify/inotify09.c b/testcases/kernel/syscalls/inotify/inotify09.c
index 475411311..701ab1487 100644
--- a/testcases/kernel/syscalls/inotify/inotify09.c
+++ b/testcases/kernel/syscalls/inotify/inotify09.c
@@ -97,12 +97,15 @@ static void verify_inotify(void)
 	inotify_fd = myinotify_init1(0);
 	if (inotify_fd < 0)
 		tst_brk(TBROK | TERRNO, "inotify_init failed");
+
+	tst_fzsync_pair_reset(&fzsync_pair);
 	for (tests = 0; tests < TEARDOWNS; tests++) {
 		wd = myinotify_add_watch(inotify_fd, FNAME, IN_MODIFY);
 		if (wd < 0)
 			tst_brk(TBROK | TERRNO, "inotify_add_watch() failed.");
 
-		tst_fzsync_wait_update_a(&fzsync_pair);
+		if (!tst_fzsync_wait_update_a(&fzsync_pair))
+			break;
 		tst_fzsync_delay_a(&fzsync_pair);
 
 		wd = myinotify_rm_watch(inotify_fd, wd);
-- 
2.18.0

[LTP] [PATCH v3 2/4] tst_timer: Convert to new library

[Cyril Hrubis]

Hi!
> So we can use functions from new lib.

We still do have oldlib tests that link against this code.

Just grep for tst_timer_start, there are at least:

testcases/kernel/syscalls/nanosleep/nanosleep02.c
testcases/kernel/syscalls/fcntl/fcntl33.c

That has to be converted before we can apply this patch.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3 3/4] tst_timer: Create interface for using multiple timers

[Cyril Hrubis]

Hi!
> +/**
> + * Encapsulates a stopwatch timer.
> + *
> + * Useful when you need to use multiple timers in a single test.
> + */
> +struct tst_timer {
> +	/** The POSIX clock type  */
> +	clockid_t clock_id;
> +	/** How much time can pass before tst_timer_expired_st() returns true */
> +	struct timespec limit;
> +	/** Where to start measuring time from */
> +	struct timespec start_time;
> +};
> +
>  /*
>   * Converts timespec to microseconds.
>   */
> @@ -251,6 +265,14 @@ void tst_timer_check(clockid_t clk_id);
>   */
>  void tst_timer_start(clockid_t clk_id);
>  
> +/**
> + * Sets the start time for timer tim.
> + *
> + * @relates tst_timer
> + * @param tim The timer to start
> + */
> +void tst_timer_start_st(struct tst_timer *tim);

So user is expected to fill in the tst_timer structure before this call
is called, right?

Maybe it would be easier to pass the timeout and CLOCK_ID to this
function and keep the initialization in the test library, that way we
can also check if the timer is supported first before we return from the
start function.

And lastly but not least, we were also discussion special timer id,
something as TST_TIMER_DEFAULT that would select appropriate timer,
althoug I guess that CLOCK_MONOTONIC is supporoted on anything that we
care for these days.

>  /*
>   * Returns true if timer started by tst_timer_start() has been running for
>   * longer than ms seconds.
> @@ -259,6 +281,14 @@ void tst_timer_start(clockid_t clk_id);
>   */
>  int tst_timer_expired_ms(long long ms);
>  
> +/**
> + * Returns true if timer tim has been running for longer than tst_timer::limit
> + *
> + * @relates tst_timer
> + * @param tim The timer to check
> + */
> +int tst_timer_expired_st(struct tst_timer *tim);
> +
>  /*
>   * Marks timer end time.
>   */
> diff --git a/lib/tst_timer.c b/lib/tst_timer.c
> index d5e4c49d2..baec18280 100644
> --- a/lib/tst_timer.c
> +++ b/lib/tst_timer.c
> @@ -79,6 +79,12 @@ void tst_timer_start(clockid_t clk_id)
>  		tst_res(TWARN | TERRNO, "tst_clock_gettime() failed");
>  }
>  
> +void tst_timer_start_st(struct tst_timer *tim)
> +{
> +	if (tst_clock_gettime(tim->clock_id, &tim->start_time))
> +		tst_res(TWARN | TERRNO, "tst_clock_gettime() failed");
> +}
> +
>  int tst_timer_expired_ms(long long ms)
>  {
>  	struct timespec cur_time;
> @@ -89,6 +95,17 @@ int tst_timer_expired_ms(long long ms)
>  	return tst_timespec_diff_ms(cur_time, start_time) >= ms;
>  }
>  
> +int tst_timer_expired_st(struct tst_timer *tim)
> +{
> +	struct timespec cur_time;
> +
> +	if (tst_clock_gettime(tim->clock_id, &cur_time))
> +		tst_res(TWARN | TERRNO, "tst_clock_gettime() failed");
> +
> +	return tst_timespec_lt(tim->limit,
> +			       tst_timespec_diff(cur_time, tim->start_time));
> +}
> +
>  void tst_timer_stop(void)
>  {
>  	if (tst_clock_gettime(clock_id, &stop_time))
> -- 
> 2.18.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3 3/4] tst_timer: Create interface for using multiple timers

[Cyril Hrubis]

Hi!
> > +/**
> > + * Encapsulates a stopwatch timer.
> > + *
> > + * Useful when you need to use multiple timers in a single test.
> > + */
> > +struct tst_timer {
> > +	/** The POSIX clock type  */
> > +	clockid_t clock_id;
> > +	/** How much time can pass before tst_timer_expired_st() returns true */
> > +	struct timespec limit;
> > +	/** Where to start measuring time from */
> > +	struct timespec start_time;
> > +};

On a related note, do you really think that we will need nsec resoltion
for the timeout timers? I would say that keeping the limit as unsigned
integer holding number of seconds would be more than enough.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3 3/4] tst_timer: Create interface for using multiple timers

[Richard Palethorpe]

Hello,

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>> +/**
>> + * Encapsulates a stopwatch timer.
>> + *
>> + * Useful when you need to use multiple timers in a single test.
>> + */
>> +struct tst_timer {
>> +	/** The POSIX clock type  */
>> +	clockid_t clock_id;
>> +	/** How much time can pass before tst_timer_expired_st() returns true */
>> +	struct timespec limit;
>> +	/** Where to start measuring time from */
>> +	struct timespec start_time;
>> +};
>> +
>>  /*
>>   * Converts timespec to microseconds.
>>   */
>> @@ -251,6 +265,14 @@ void tst_timer_check(clockid_t clk_id);
>>   */
>>  void tst_timer_start(clockid_t clk_id);
>>
>> +/**
>> + * Sets the start time for timer tim.
>> + *
>> + * @relates tst_timer
>> + * @param tim The timer to start
>> + */
>> +void tst_timer_start_st(struct tst_timer *tim);
>
> So user is expected to fill in the tst_timer structure before this call
> is called, right?
>
> Maybe it would be easier to pass the timeout and CLOCK_ID to this
> function and keep the initialization in the test library, that way we
> can also check if the timer is supported first before we return from the
> start function.

Well if the clock is not supported and the call fails, then the user
will have to deal with that at the call site of tst_timer_start which is
not very convenient for tst_fuzzy_sync. I would rather have a seperate
initialisation function for the tst_timer struct which can be called in
the setup function. e.g. tst_timer_init[_st](clock_id, timeout, ...).

>
> And lastly but not least, we were also discussion special timer id,
> something as TST_TIMER_DEFAULT that would select appropriate timer,
> althoug I guess that CLOCK_MONOTONIC is supporoted on anything that we
> care for these days.

Yeah, fuzzy_sync just defines CLOCK_MONOTIC to CLOCK_MONOTONIC_RAW if
possible and uses that. Not had any reports of CLOCK_MONOTONIC being
unavailabe.

>
>>  /*
>>   * Returns true if timer started by tst_timer_start() has been running for
>>   * longer than ms seconds.
>> @@ -259,6 +281,14 @@ void tst_timer_start(clockid_t clk_id);
>>   */
>>  int tst_timer_expired_ms(long long ms);
>>
>> +/**
>> + * Returns true if timer tim has been running for longer than tst_timer::limit
>> + *
>> + * @relates tst_timer
>> + * @param tim The timer to check
>> + */
>> +int tst_timer_expired_st(struct tst_timer *tim);
>> +
>>  /*
>>   * Marks timer end time.
>>   */
>> diff --git a/lib/tst_timer.c b/lib/tst_timer.c
>> index d5e4c49d2..baec18280 100644
>> --- a/lib/tst_timer.c
>> +++ b/lib/tst_timer.c
>> @@ -79,6 +79,12 @@ void tst_timer_start(clockid_t clk_id)
>>  		tst_res(TWARN | TERRNO, "tst_clock_gettime() failed");
>>  }
>>
>> +void tst_timer_start_st(struct tst_timer *tim)
>> +{
>> +	if (tst_clock_gettime(tim->clock_id, &tim->start_time))
>> +		tst_res(TWARN | TERRNO, "tst_clock_gettime() failed");
>> +}
>> +
>>  int tst_timer_expired_ms(long long ms)
>>  {
>>  	struct timespec cur_time;
>> @@ -89,6 +95,17 @@ int tst_timer_expired_ms(long long ms)
>>  	return tst_timespec_diff_ms(cur_time, start_time) >= ms;
>>  }
>>
>> +int tst_timer_expired_st(struct tst_timer *tim)
>> +{
>> +	struct timespec cur_time;
>> +
>> +	if (tst_clock_gettime(tim->clock_id, &cur_time))
>> +		tst_res(TWARN | TERRNO, "tst_clock_gettime() failed");
>> +
>> +	return tst_timespec_lt(tim->limit,
>> +			       tst_timespec_diff(cur_time, tim->start_time));
>> +}
>> +
>>  void tst_timer_stop(void)
>>  {
>>  	if (tst_clock_gettime(clock_id, &stop_time))
>> --
>> 2.18.0
>>
>>
>> --
>> Mailing list info: https://lists.linux.it/listinfo/ltp


--
Thank you,
Richard.

[LTP] [PATCH v3 3/4] tst_timer: Create interface for using multiple timers

[Cyril Hrubis]

Hi!
> Well if the clock is not supported and the call fails, then the user
> will have to deal with that at the call site of tst_timer_start which is
> not very convenient for tst_fuzzy_sync. I would rather have a seperate
> initialisation function for the tst_timer struct which can be called in
> the setup function. e.g. tst_timer_init[_st](clock_id, timeout, ...).
> 
> >
> > And lastly but not least, we were also discussion special timer id,
> > something as TST_TIMER_DEFAULT that would select appropriate timer,
> > althoug I guess that CLOCK_MONOTONIC is supporoted on anything that we
> > care for these days.
> 
> Yeah, fuzzy_sync just defines CLOCK_MONOTIC to CLOCK_MONOTONIC_RAW if
> possible and uses that. Not had any reports of CLOCK_MONOTONIC being
> unavailabe.

Maybe we can just do:

#ifdef CLOCK_MONOTONIC_RAW
# define TST_CLOCK_DEFAULT CLOCK_MONOTONIC_RAW
#else
# define TST_CLOCK_DEFAULT CLOCK_MONOTONIC
#endif

And be done with it.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3 3/4] tst_timer: Create interface for using multiple timers

[Richard Palethorpe]

Hello,

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>> > +/**
>> > + * Encapsulates a stopwatch timer.
>> > + *
>> > + * Useful when you need to use multiple timers in a single test.
>> > + */
>> > +struct tst_timer {
>> > +	/** The POSIX clock type  */
>> > +	clockid_t clock_id;
>> > +	/** How much time can pass before tst_timer_expired_st() returns true */
>> > +	struct timespec limit;
>> > +	/** Where to start measuring time from */
>> > +	struct timespec start_time;
>> > +};
>
> On a related note, do you really think that we will need nsec resoltion
> for the timeout timers? I would say that keeping the limit as unsigned
> integer holding number of seconds would be more than enough.

I agree that seconds are appropriate for test timeouts, but it is
practically 'free' to use the struct here because of the existing API to
compare timespec structs. Also I might use this interface to time
syscalls and maybe remove some code duplication.

--
Thank you,
Richard.

[LTP] [PATCH v3 4/4] fzsync: Limit execution time to prevent test timeouts

[Li Wang]

Richard Palethorpe <rpalethorpe@suse.com> wrote:
>
> ...
>
> @@ -99,6 +102,15 @@ struct tst_fzsync_pair {
>         .info_gap = 0x7FFFF     \
>  }
>
> +
> static void tst_fzsync_pair_reset(struct tst_fzsync_pair *pair)
> +{
> +       pair->timer.clock_id = CLOCK_MONOTONIC_RAW;
> +       pair->timer.limit.tv_sec = 60 * tst_timeout_mul();
> +       pair->timer.limit.tv_nsec = 0;
> +
> +       tst_timer_start_st(&pair->timer);
> +}
> +
>

There is a loop defect in this method as I commented in patch V2.

If we don't reset the pair->exit to 0 after one loop, it will be never run
into the second
fzsync function because the pair->exit has been set to 1 at the first
expired time.

something result like:
----------------------------
# ./cve-2016-7117 -i 3
tst_test.c:1022: INFO: Timeout per run is 0h 05m 00s
../../include/tst_fuzzy_sync.h:121: INFO: avg_diff = -216ns, avg_dev =
565ns, delay = 02474 loops
../../include/tst_fuzzy_sync.h:121: INFO: avg_diff = 12ns, avg_dev = 430ns,
delay = 02604 loops
../../include/tst_fuzzy_sync.h:330: INFO: Exceeded fuzzy sync time limit,
requesting exit
cve-2016-7117.c:161: PASS: Nothing happened after 1564741 attempts
cve-2016-7117.c:161: PASS: Nothing happened after 1 attempts
cve-2016-7117.c:161: PASS: Nothing happened after 1 attempts

Summary:
passed   3
failed   0
skipped  0
warnings 0

But, if we just reset the pair->exit to 0 in the new function
tst_fzsync_pair_reset(),
there still NOT fix the problem totally, because in the last test expired
time, all threads
created by setup() function have exited, and here we'll only loop
in tst_fzsync_wait_a()
and wait there forever. :(

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20180817/df3e421d/attachment.html>

[LTP] [PATCH v3 4/4] fzsync: Limit execution time to prevent test timeouts

[Li Wang]

Li Wang <liwang@redhat.com> wrote:

>
> Richard Palethorpe <rpalethorpe@suse.com> wrote:
>>
>> ...
>>
>> @@ -99,6 +102,15 @@ struct tst_fzsync_pair {
>>         .info_gap = 0x7FFFF     \
>>  }
>>
>> +
>> static void tst_fzsync_pair_reset(struct tst_fzsync_pair *pair)
>> +{
>> +       pair->timer.clock_id = CLOCK_MONOTONIC_RAW;
>> +       pair->timer.limit.tv_sec = 60 * tst_timeout_mul();
>> +       pair->timer.limit.tv_nsec = 0;
>> +
>> +       tst_timer_start_st(&pair->timer);
>> +}
>> +
>>
>
> There is a loop defect in this method as I commented in patch V2.
>
> If we don't reset the pair->exit to 0 after one loop, it will be never run
> into the second
> fzsync function because the pair->exit has been set to 1 at the first
> expired time.
>
> something result like:
> ----------------------------
> # ./cve-2016-7117 -i 3
> tst_test.c:1022: INFO: Timeout per run is 0h 05m 00s
> ../../include/tst_fuzzy_sync.h:121: INFO: avg_diff = -216ns, avg_dev =
> 565ns, delay = 02474 loops
> ../../include/tst_fuzzy_sync.h:121: INFO: avg_diff = 12ns, avg_dev =
> 430ns, delay = 02604 loops
> ../../include/tst_fuzzy_sync.h:330: INFO: Exceeded fuzzy sync time limit,
> requesting exit
> cve-2016-7117.c:161: PASS: Nothing happened after 1564741 attempts
> cve-2016-7117.c:161: PASS: Nothing happened after 1 attempts
> cve-2016-7117.c:161: PASS: Nothing happened after 1 attempts
>
> Summary:
> passed   3
> failed   0
> skipped  0
> warnings 0
>
> But, if we just reset the pair->exit to 0 in the new function
> tst_fzsync_pair_reset(),
> there still NOT fix the problem totally, because in the last test expired
> time, all threads
> created by setup() function have exited, and here we'll only loop
> in tst_fzsync_wait_a()
> and wait there forever. :(
>

I just come up with a stupid patch to fix that, but personally I insist
believe
that maybe we should not leave this kind of works to LTP user, we'd better
encapsulate that all in fuzzy_sync library.

Just FYI:

diff --git a/include/tst_fuzzy_sync.h b/include/tst_fuzzy_sync.h
index 5e0ff36..862ab7e 100644
--- a/include/tst_fuzzy_sync.h
+++ b/include/tst_fuzzy_sync.h
@@ -102,8 +102,14 @@ struct tst_fzsync_pair {
        .info_gap = 0x7FFFF     \
 }

-static void tst_fzsync_pair_reset(struct tst_fzsync_pair *pair)
+static void tst_fzsync_pair_init(struct tst_fzsync_pair *pair)
 {
+       pair->exit = 0;
+       pair->delay = 0;
+       pair->a_cntr = pair->b_cntr = 0;
+       pair->avg_dev = pair->avg_diff = 0;
+       pair->a.tv_sec = pair->a.tv_nsec = 0;
+       pair->b.tv_sec = pair->b.tv_nsec = 0;
        pair->timer.clock_id = CLOCK_MONOTONIC_RAW;
        pair->timer.limit.tv_sec = 60 * tst_timeout_mul();
        pair->timer.limit.tv_nsec = 0;
diff --git a/testcases/cve/cve-2016-7117.c b/testcases/cve/cve-2016-7117.c
index fecc588..f8993c7 100644
--- a/testcases/cve/cve-2016-7117.c
+++ b/testcases/cve/cve-2016-7117.c
@@ -136,7 +136,10 @@ static void run(void)

        msghdrs[0].msg_hdr.msg_iov->iov_base = (void *)&rbuf;

-       tst_fzsync_pair_reset(&fzsync_pair);
+       if (fzsync_pair.exit == 1)
+               setup();
+
+       tst_fzsync_pair_init(&fzsync_pair);
        for (i = 1; i < ATTEMPTS; i++) {
                if (socketpair(AF_LOCAL, SOCK_DGRAM, 0, (int *)socket_fds))
                        tst_brk(TBROK | TERRNO, "Socket creation failed");

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20180817/fb6ddbf3/attachment.html>