[LTP] [PATCH RFC v3 1/3] lib: add tst_timer_find_clock()

[LTP] [PATCH RFC v3 1/3] lib: add tst_timer_find_clock()

[Jan Stancek]

Adds function that returns first available clock usable for measuring
elapsed test time.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 include/tst_timer.h |  5 +++++
 lib/tst_timer.c     | 16 ++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/include/tst_timer.h b/include/tst_timer.h
index 0fd7ed6cfc03..b92d975d8edc 100644
--- a/include/tst_timer.h
+++ b/include/tst_timer.h
@@ -245,6 +245,11 @@ static inline long long tst_timespec_abs_diff_ms(struct timespec t1,
 void tst_timer_check(clockid_t clk_id);
 
 /*
+ * Returns usable clock id for measuring elapsed test time.
+ */
+clockid_t tst_timer_find_clock(void);
+
+/*
  * Marks a start time for given clock type.
  *
  * @clk_id: Posix clock to use.
diff --git a/lib/tst_timer.c b/lib/tst_timer.c
index dffaba0cbd5c..bbd460364b70 100644
--- a/lib/tst_timer.c
+++ b/lib/tst_timer.c
@@ -39,6 +39,22 @@ static const char *clock_name(clockid_t clk_id)
 	}
 }
 
+clockid_t tst_timer_find_clock(void)
+{
+	unsigned int i;
+	static struct timespec tmp;
+	clockid_t clocks[] = { CLOCK_MONOTONIC, CLOCK_MONOTONIC_RAW, CLOCK_MONOTONIC_COARSE,
+		CLOCK_BOOTTIME, CLOCK_REALTIME, CLOCK_REALTIME_COARSE };
+
+	for (i = 0; i < ARRAY_SIZE(clocks); i++) {
+		if (tst_clock_gettime(clocks[i], &tmp) == 0)
+			return clocks[i];
+	}
+
+	tst_brk(TCONF, "Failed to find usable clock");
+	return -1;
+}
+
 void tst_timer_check(clockid_t clk_id)
 {
 	if (tst_clock_gettime(clk_id, &start_time)) {
-- 
1.8.3.1

[LTP] [PATCH RFC v3 2/3] lib: introduce tst_timeout_remaining()

[Jan Stancek]

Which returns number of seconds remaining till timeout.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 include/tst_test.h        |   1 +
 lib/newlib_tests/test18   | Bin 0 -> 384032 bytes
 lib/newlib_tests/test18.c |  32 ++++++++++++++++++++++++++++++++
 lib/tst_test.c            |  21 +++++++++++++++++++++
 4 files changed, 54 insertions(+)
 create mode 100755 lib/newlib_tests/test18
 create mode 100644 lib/newlib_tests/test18.c

diff --git a/include/tst_test.h b/include/tst_test.h
index 98dacf3873ab..c0c9a7c7b995 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);
 
+unsigned int tst_timeout_remaining(void);
 void tst_set_timeout(int timeout);
 
 #ifndef TST_NO_DEFAULT_MAIN
diff --git a/lib/newlib_tests/test18 b/lib/newlib_tests/test18
new file mode 100755
index 000000000000..01dc1d7976fe
Binary files /dev/null and b/lib/newlib_tests/test18 differ
diff --git a/lib/newlib_tests/test18.c b/lib/newlib_tests/test18.c
new file mode 100644
index 000000000000..aba007d3689c
--- /dev/null
+++ b/lib/newlib_tests/test18.c
@@ -0,0 +1,32 @@
+/*
+ * Copyright (c) 2018, Linux Test Project
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <stdlib.h>
+#include "tst_test.h"
+
+static void run(void)
+{
+	unsigned int remaining = tst_timeout_remaining();
+	if (remaining >= 200)
+		tst_res(TPASS, "Timeout remaining: %d", remaining);
+	else
+		tst_res(TFAIL, "Timeout remaining: %d", remaining);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+};
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 2f3d357d2fcc..75619fabffa4 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -47,6 +47,8 @@ static int iterations = 1;
 static float duration = -1;
 static pid_t main_pid, lib_pid;
 static int mntpoint_mounted;
+static clockid_t tst_clock;
+static struct timespec tst_start_time;
 
 struct results {
 	int passed;
@@ -758,6 +760,7 @@ static void do_setup(int argc, char *argv[])
 
 	if (tst_test->sample)
 		tst_test = tst_timer_test_setup(tst_test);
+	tst_clock = tst_timer_find_clock();
 
 	parse_opts(argc, argv);
 
@@ -992,6 +995,21 @@ static void sigint_handler(int sig LTP_ATTRIBUTE_UNUSED)
 	}
 }
 
+unsigned int tst_timeout_remaining(void)
+{
+	static struct timespec now;
+	unsigned int elapsed;
+
+	if (tst_clock_gettime(tst_clock, &now))
+		tst_res(TWARN | TERRNO, "tst_clock_gettime() failed");
+
+	elapsed = tst_timespec_diff_ms(now, tst_start_time) / 1000;
+	if (results->timeout > elapsed)
+		return results->timeout - elapsed;
+
+	return 0;
+}
+
 void tst_set_timeout(int timeout)
 {
 	char *mul = getenv("LTP_TIMEOUT_MUL");
@@ -1012,6 +1030,9 @@ void tst_set_timeout(int timeout)
 		results->timeout = results->timeout * m + 0.5;
 	}
 
+	if (tst_clock_gettime(tst_clock, &tst_start_time))
+		tst_res(TWARN | TERRNO, "tst_clock_gettime() failed");
+
 	tst_res(TINFO, "Timeout per run is %uh %02um %02us",
 		results->timeout/3600, (results->timeout%3600)/60,
 		results->timeout % 60);
-- 
1.8.3.1

[LTP] [PATCH RFC v3 3/3] move_pages12: end early if runtime gets close to test time

[Jan Stancek]

Most systems can complete this reproducer in standard test time.
Small groups of systems (e.g. aarch64 with 512M hugepages) are hitting
a timeout.

Add a check for elapsed time and end test early if we are getting close (80%).

Fixes: #387

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 testcases/kernel/syscalls/move_pages/move_pages12.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/testcases/kernel/syscalls/move_pages/move_pages12.c b/testcases/kernel/syscalls/move_pages/move_pages12.c
index 43acb42aabb1..3f60c0ca47fc 100644
--- a/testcases/kernel/syscalls/move_pages/move_pages12.c
+++ b/testcases/kernel/syscalls/move_pages/move_pages12.c
@@ -101,6 +101,7 @@ static void do_test(void)
 	int i;
 	pid_t cpid = -1;
 	int status;
+	unsigned int _20_percent = (tst_timeout_remaining() / 5);
 
 	addr = SAFE_MMAP(NULL, TEST_PAGES * hpsz, PROT_READ | PROT_WRITE,
 		MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
@@ -123,14 +124,15 @@ static void do_test(void)
 		memset(addr, 0, TEST_PAGES * hpsz);
 
 		SAFE_MUNMAP(addr, TEST_PAGES * hpsz);
-	}
 
-	if (i == LOOPS) {
-		SAFE_KILL(cpid, SIGKILL);
-		SAFE_WAITPID(cpid, &status, 0);
-		if (!WIFEXITED(status))
-			tst_res(TPASS, "Bug not reproduced");
+		if (tst_timeout_remaining() < _20_percent)
+			break;
 	}
+
+	SAFE_KILL(cpid, SIGKILL);
+	SAFE_WAITPID(cpid, &status, 0);
+	if (!WIFEXITED(status))
+		tst_res(TPASS, "Bug not reproduced");
 }
 
 static void alloc_free_huge_on_node(unsigned int node, size_t size)
-- 
1.8.3.1

[LTP] [PATCH RFC v3 2/3] lib: introduce tst_timeout_remaining()

[Cyril Hrubis]

Hi!
> Which returns number of seconds remaining till timeout.
> 
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> ---
>  include/tst_test.h        |   1 +
>  lib/newlib_tests/test18   | Bin 0 -> 384032 bytes
>  lib/newlib_tests/test18.c |  32 ++++++++++++++++++++++++++++++++
>  lib/tst_test.c            |  21 +++++++++++++++++++++
>  4 files changed, 54 insertions(+)
>  create mode 100755 lib/newlib_tests/test18
>  create mode 100644 lib/newlib_tests/test18.c
> 
> diff --git a/include/tst_test.h b/include/tst_test.h
> index 98dacf3873ab..c0c9a7c7b995 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);
>  
> +unsigned int tst_timeout_remaining(void);
>  void tst_set_timeout(int timeout);
>  
>  #ifndef TST_NO_DEFAULT_MAIN
> diff --git a/lib/newlib_tests/test18 b/lib/newlib_tests/test18
> new file mode 100755
> index 000000000000..01dc1d7976fe
> Binary files /dev/null and b/lib/newlib_tests/test18 differ
> diff --git a/lib/newlib_tests/test18.c b/lib/newlib_tests/test18.c
> new file mode 100644
> index 000000000000..aba007d3689c
> --- /dev/null
> +++ b/lib/newlib_tests/test18.c
> @@ -0,0 +1,32 @@
> +/*
> + * Copyright (c) 2018, Linux Test Project
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <stdlib.h>
> +#include "tst_test.h"
> +
> +static void run(void)
> +{
> +	unsigned int remaining = tst_timeout_remaining();

Maybe we should do something as:

	while (tst_timeout_remaining() > 2)
		sleep(1);

	tst_res(TPASS, ...);

And set timeout in tst_test to something as 10s, to really test the API.

> +	if (remaining >= 200)
> +		tst_res(TPASS, "Timeout remaining: %d", remaining);
> +	else
> +		tst_res(TFAIL, "Timeout remaining: %d", remaining);
> +}
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +};
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 2f3d357d2fcc..75619fabffa4 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -47,6 +47,8 @@ static int iterations = 1;
>  static float duration = -1;
>  static pid_t main_pid, lib_pid;
>  static int mntpoint_mounted;
> +static clockid_t tst_clock;
> +static struct timespec tst_start_time;
>  
>  struct results {
>  	int passed;
> @@ -758,6 +760,7 @@ static void do_setup(int argc, char *argv[])
>  
>  	if (tst_test->sample)
>  		tst_test = tst_timer_test_setup(tst_test);
> +	tst_clock = tst_timer_find_clock();

I wonder if we really need this, we were running with CLOCK_MONOTONIC
timer in the testrun() for quite some time now and nobody complained so
far.

Well I guess that it would be nice to use CLOCK_MONOTONIC_COARSE for the
tst_timeout_remaining if available, which should save us some CPU since
it's supposed to be called in a loop.

>  	parse_opts(argc, argv);
>  
> @@ -992,6 +995,21 @@ static void sigint_handler(int sig LTP_ATTRIBUTE_UNUSED)
>  	}
>  }
>  
> +unsigned int tst_timeout_remaining(void)
> +{
> +	static struct timespec now;
> +	unsigned int elapsed;
> +
> +	if (tst_clock_gettime(tst_clock, &now))
> +		tst_res(TWARN | TERRNO, "tst_clock_gettime() failed");
> +
> +	elapsed = tst_timespec_diff_ms(now, tst_start_time) / 1000;
> +	if (results->timeout > elapsed)
> +		return results->timeout - elapsed;
> +
> +	return 0;
> +}

This is obviously correct.

>  void tst_set_timeout(int timeout)
>  {
>  	char *mul = getenv("LTP_TIMEOUT_MUL");
> @@ -1012,6 +1030,9 @@ void tst_set_timeout(int timeout)
>  		results->timeout = results->timeout * m + 0.5;
>  	}
>  
> +	if (tst_clock_gettime(tst_clock, &tst_start_time))
> +		tst_res(TWARN | TERRNO, "tst_clock_gettime() failed");

Looking into this, this will not work with the -i option, since the
timeout is restarted after each iteration in heartbeat_handler().
However clock_gettime() is supposedly signal-safe. So as far as I can
tell we have to take the timestamp in the heartbeat_handler() instead
and that should be it.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH RFC v3 3/3] move_pages12: end early if runtime gets close to test time

[Cyril Hrubis]

Hi!
> +	unsigned int _20_percent = (tst_timeout_remaining() / 5);

I would like to avoid variables that start with underscore if possible,
since these are reserved for libc...

Otherwise it's fine.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH RFC v3 2/3] lib: introduce tst_timeout_remaining()

[Jan Stancek]

----- Original Message -----
> Hi!
> > +#include <stdlib.h>
> > +#include "tst_test.h"
> > +
> > +static void run(void)
> > +{
> > +	unsigned int remaining = tst_timeout_remaining();
> 
> Maybe we should do something as:
> 
> 	while (tst_timeout_remaining() > 2)
> 		sleep(1);
> 
> 	tst_res(TPASS, ...);

Yeah, I felt guilty adding more sleeps() :-).

> 
> And set timeout in tst_test to something as 10s, to really test the API.
> 
> > +	if (remaining >= 200)
> > +		tst_res(TPASS, "Timeout remaining: %d", remaining);
> > +	else
> > +		tst_res(TFAIL, "Timeout remaining: %d", remaining);
> > +}
> > +
> > +static struct tst_test test = {
> > +	.test_all = run,
> > +};
> > diff --git a/lib/tst_test.c b/lib/tst_test.c
> > index 2f3d357d2fcc..75619fabffa4 100644
> > --- a/lib/tst_test.c
> > +++ b/lib/tst_test.c
> > @@ -47,6 +47,8 @@ static int iterations = 1;
> >  static float duration = -1;
> >  static pid_t main_pid, lib_pid;
> >  static int mntpoint_mounted;
> > +static clockid_t tst_clock;
> > +static struct timespec tst_start_time;
> >  
> >  struct results {
> >  	int passed;
> > @@ -758,6 +760,7 @@ static void do_setup(int argc, char *argv[])
> >  
> >  	if (tst_test->sample)
> >  		tst_test = tst_timer_test_setup(tst_test);
> > +	tst_clock = tst_timer_find_clock();
> 
> I wonder if we really need this, we were running with CLOCK_MONOTONIC
> timer in the testrun() for quite some time now and nobody complained so
> far.

I don't have strong opinion on this. It's fairly cheap to go through that list,
and we can be more courageous to change order later.

> 
> Well I guess that it would be nice to use CLOCK_MONOTONIC_COARSE for the
> tst_timeout_remaining if available, which should save us some CPU since
> it's supposed to be called in a loop.
> 
> >  	parse_opts(argc, argv);
> >  
> > @@ -992,6 +995,21 @@ static void sigint_handler(int sig
> > LTP_ATTRIBUTE_UNUSED)
> >  	}
> >  }
> >  
> > +unsigned int tst_timeout_remaining(void)
> > +{
> > +	static struct timespec now;
> > +	unsigned int elapsed;
> > +
> > +	if (tst_clock_gettime(tst_clock, &now))
> > +		tst_res(TWARN | TERRNO, "tst_clock_gettime() failed");
> > +
> > +	elapsed = tst_timespec_diff_ms(now, tst_start_time) / 1000;
> > +	if (results->timeout > elapsed)
> > +		return results->timeout - elapsed;
> > +
> > +	return 0;
> > +}
> 
> This is obviously correct.
> 
> >  void tst_set_timeout(int timeout)
> >  {
> >  	char *mul = getenv("LTP_TIMEOUT_MUL");
> > @@ -1012,6 +1030,9 @@ void tst_set_timeout(int timeout)
> >  		results->timeout = results->timeout * m + 0.5;
> >  	}
> >  
> > +	if (tst_clock_gettime(tst_clock, &tst_start_time))
> > +		tst_res(TWARN | TERRNO, "tst_clock_gettime() failed");
> 
> Looking into this, this will not work with the -i option, since the
> timeout is restarted after each iteration in heartbeat_handler().
> However clock_gettime() is supposedly signal-safe. So as far as I can
> tell we have to take the timestamp in the heartbeat_handler() instead
> and that should be it.

heartbeat() is called in tst_set_timeout() only for non-lib pids.
And testrun() calls it only after run_tests().

So I think it will have to be at both locations: anytime we call alarm(),
we'll need to re-initialize tst_start_time:

void timeout_restart(void)
{
    alarm(results->timeout);
    if (tst_clock_gettime(tst_clock, &tst_start_time))
        tst_res(TWARN | TERRNO, "tst_clock_gettime() failed");
}

and call it in tst_set_timeout() and heartbeat_handler()

---

What is your opinion on API? Absolute numbers vs ratio approach?

Regards,
Jan

[LTP] [PATCH RFC v3 2/3] lib: introduce tst_timeout_remaining()

[Cyril Hrubis]

Hi!
> > Maybe we should do something as:
> > 
> > 	while (tst_timeout_remaining() > 2)
> > 		sleep(1);
> > 
> > 	tst_res(TPASS, ...);
> 
> Yeah, I felt guilty adding more sleeps() :-).

Well in this case it's reasonable use... ;-)

> > And set timeout in tst_test to something as 10s, to really test the API.
> > 
> > > +	if (remaining >= 200)
> > > +		tst_res(TPASS, "Timeout remaining: %d", remaining);
> > > +	else
> > > +		tst_res(TFAIL, "Timeout remaining: %d", remaining);
> > > +}
> > > +
> > > +static struct tst_test test = {
> > > +	.test_all = run,
> > > +};
> > > diff --git a/lib/tst_test.c b/lib/tst_test.c
> > > index 2f3d357d2fcc..75619fabffa4 100644
> > > --- a/lib/tst_test.c
> > > +++ b/lib/tst_test.c
> > > @@ -47,6 +47,8 @@ static int iterations = 1;
> > >  static float duration = -1;
> > >  static pid_t main_pid, lib_pid;
> > >  static int mntpoint_mounted;
> > > +static clockid_t tst_clock;
> > > +static struct timespec tst_start_time;
> > >  
> > >  struct results {
> > >  	int passed;
> > > @@ -758,6 +760,7 @@ static void do_setup(int argc, char *argv[])
> > >  
> > >  	if (tst_test->sample)
> > >  		tst_test = tst_timer_test_setup(tst_test);
> > > +	tst_clock = tst_timer_find_clock();
> > 
> > I wonder if we really need this, we were running with CLOCK_MONOTONIC
> > timer in the testrun() for quite some time now and nobody complained so
> > far.
> 
> I don't have strong opinion on this. It's fairly cheap to go through that list,
> and we can be more courageous to change order later.

We do use CLOCK_MONOTONIC in the tst_test.c unconditionally anyways, so
I wouldn't bother with this unless somebody complains.

> > Well I guess that it would be nice to use CLOCK_MONOTONIC_COARSE for the
> > tst_timeout_remaining if available, which should save us some CPU since
> > it's supposed to be called in a loop.
> > 
> > >  	parse_opts(argc, argv);
> > >  
> > > @@ -992,6 +995,21 @@ static void sigint_handler(int sig
> > > LTP_ATTRIBUTE_UNUSED)
> > >  	}
> > >  }
> > >  
> > > +unsigned int tst_timeout_remaining(void)
> > > +{
> > > +	static struct timespec now;
> > > +	unsigned int elapsed;
> > > +
> > > +	if (tst_clock_gettime(tst_clock, &now))
> > > +		tst_res(TWARN | TERRNO, "tst_clock_gettime() failed");
> > > +
> > > +	elapsed = tst_timespec_diff_ms(now, tst_start_time) / 1000;
> > > +	if (results->timeout > elapsed)
> > > +		return results->timeout - elapsed;
> > > +
> > > +	return 0;
> > > +}
> > 
> > This is obviously correct.
> > 
> > >  void tst_set_timeout(int timeout)
> > >  {
> > >  	char *mul = getenv("LTP_TIMEOUT_MUL");
> > > @@ -1012,6 +1030,9 @@ void tst_set_timeout(int timeout)
> > >  		results->timeout = results->timeout * m + 0.5;
> > >  	}
> > >  
> > > +	if (tst_clock_gettime(tst_clock, &tst_start_time))
> > > +		tst_res(TWARN | TERRNO, "tst_clock_gettime() failed");
> > 
> > Looking into this, this will not work with the -i option, since the
> > timeout is restarted after each iteration in heartbeat_handler().
> > However clock_gettime() is supposedly signal-safe. So as far as I can
> > tell we have to take the timestamp in the heartbeat_handler() instead
> > and that should be it.
> 
> heartbeat() is called in tst_set_timeout() only for non-lib pids.
> And testrun() calls it only after run_tests().
> 
> So I think it will have to be at both locations: anytime we call alarm(),
> we'll need to re-initialize tst_start_time:
> 
> void timeout_restart(void)
> {
>     alarm(results->timeout);
>     if (tst_clock_gettime(tst_clock, &tst_start_time))
>         tst_res(TWARN | TERRNO, "tst_clock_gettime() failed");
> }
> 
> and call it in tst_set_timeout() and heartbeat_handler()

Ah, right. I guess that we primarily care about the test process, which
calls the heartbeat() function. I doubt that we will need this to be
working in the test library, so I guess that adding this to heartbeat()
function will suffice...

> ---
> 
> What is your opinion on API? Absolute numbers vs ratio approach?

Absolute value sounds better to me.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH RFC v3 2/3] lib: introduce tst_timeout_remaining()

[Li Wang]

Hi Jan,

I'm OK to go this way, as you said that with the remaining seconds return,
we can do something more for testcase. And I also suggest Richard to have
try on using it in fzsync API.


On Tue, Aug 28, 2018 at 10:40 PM, Jan Stancek <jstancek@redhat.com> wrote:

> Which returns number of seconds remaining till timeout.
>
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> ---
>  include/tst_test.h        |   1 +
>  lib/newlib_tests/test18   | Bin 0 -> 384032 bytes
>  lib/newlib_tests/test18.c |  32 ++++++++++++++++++++++++++++++++
>  lib/tst_test.c            |  21 +++++++++++++++++++++
>  4 files changed, 54 insertions(+)
>  create mode 100755 lib/newlib_tests/test18
>

This test18 is a binary file which should not be submitted.


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

[LTP] [PATCH RFC v3 2/3] lib: introduce tst_timeout_remaining()

[Richard Palethorpe]

Hello,

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>> > Maybe we should do something as:
>> >
>> > 	while (tst_timeout_remaining() > 2)
>> > 		sleep(1);
>> >
>> > 	tst_res(TPASS, ...);
>>
>> Yeah, I felt guilty adding more sleeps() :-).
>
> Well in this case it's reasonable use... ;-)
>
>> > And set timeout in tst_test to something as 10s, to really test the API.
>> >
>> > > +	if (remaining >= 200)
>> > > +		tst_res(TPASS, "Timeout remaining: %d", remaining);
>> > > +	else
>> > > +		tst_res(TFAIL, "Timeout remaining: %d", remaining);
>> > > +}
>> > > +
>> > > +static struct tst_test test = {
>> > > +	.test_all = run,
>> > > +};
>> > > diff --git a/lib/tst_test.c b/lib/tst_test.c
>> > > index 2f3d357d2fcc..75619fabffa4 100644
>> > > --- a/lib/tst_test.c
>> > > +++ b/lib/tst_test.c
>> > > @@ -47,6 +47,8 @@ static int iterations = 1;
>> > >  static float duration = -1;
>> > >  static pid_t main_pid, lib_pid;
>> > >  static int mntpoint_mounted;
>> > > +static clockid_t tst_clock;
>> > > +static struct timespec tst_start_time;
>> > >
>> > >  struct results {
>> > >  	int passed;
>> > > @@ -758,6 +760,7 @@ static void do_setup(int argc, char *argv[])
>> > >
>> > >  	if (tst_test->sample)
>> > >  		tst_test = tst_timer_test_setup(tst_test);
>> > > +	tst_clock = tst_timer_find_clock();
>> >
>> > I wonder if we really need this, we were running with CLOCK_MONOTONIC
>> > timer in the testrun() for quite some time now and nobody complained so
>> > far.
>>
>> I don't have strong opinion on this. It's fairly cheap to go through that list,
>> and we can be more courageous to change order later.
>
> We do use CLOCK_MONOTONIC in the tst_test.c unconditionally anyways, so
> I wouldn't bother with this unless somebody complains.
>
>> > Well I guess that it would be nice to use CLOCK_MONOTONIC_COARSE for the
>> > tst_timeout_remaining if available, which should save us some CPU since
>> > it's supposed to be called in a loop.

I doubt we will notice the difference in speed, if there is any on a
modern CPU. On all the implementations I found in glibc
CLOCK_MONOTONIC(_RAW) appears to just read a register value and scale it
to the CPU frequency. It doesn't look like an expensive operation in LTP
terms. However in some scenarious we definitely need a high resolution
clock.

I think defining TST_DEFAULT_CLOCK as CLOCK_MONOTONIC(_RAW) and using it
everywhere should be fine, although I don't have anything against
returning it from an inline function either.

--
Thank you,
Richard.