[LTP] [PATCH 1/2] syscalls/timer_settime01: Improve print messages

[LTP] [PATCH 1/2] syscalls/timer_settime01: Improve print messages

[Viresh Kumar]

This improves the print messages by providing additional information
about the tests.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 .../syscalls/timer_settime/timer_settime01.c  | 31 ++++++++++---------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/testcases/kernel/syscalls/timer_settime/timer_settime01.c b/testcases/kernel/syscalls/timer_settime/timer_settime01.c
index 08fb56e4943a..52c435ee3d91 100644
--- a/testcases/kernel/syscalls/timer_settime/timer_settime01.c
+++ b/testcases/kernel/syscalls/timer_settime/timer_settime01.c
@@ -82,12 +82,12 @@ static void run(unsigned int n)
 		if (TST_RET != 0) {
 			if (possibly_unsupported(clock) &&
 				(TST_ERR == EINVAL || TST_ERR == ENOTSUP)) {
-				tst_res(TCONF | TTERRNO, "%s unsupported",
-					get_clock_str(clock));
+				tst_res(TCONF | TTERRNO, "%s: %s unsupported",
+					tv->desc, get_clock_str(clock));
 			} else {
 				tst_res(TFAIL | TTERRNO,
-					"timer_create(%s) failed",
-					get_clock_str(clock));
+					"%s: timer_create(%s) failed",
+					tv->desc, get_clock_str(clock));
 			}
 			continue;
 		}
@@ -102,9 +102,8 @@ static void run(unsigned int n)
 		if (tc->flag & TIMER_ABSTIME) {
 			timenow.type = tv->type;
 			if (tv->gettime(clock, tst_ts_get(&timenow)) < 0) {
-				tst_res(TFAIL,
-					"clock_gettime(%s) failed - skipping the test",
-					get_clock_str(clock));
+				tst_res(TFAIL, "%s: clock_gettime(%s) failed - skipping the test",
+					tv->desc, get_clock_str(clock));
 				continue;
 			}
 			val += tst_ts_get_sec(timenow);
@@ -115,19 +114,21 @@ static void run(unsigned int n)
 		tst_its_set_value_sec(&new_set, val);
 		tst_its_set_value_nsec(&new_set, 0);
 
-		TEST(tv->func(timer, tc->flag, tst_its_get(&new_set), tst_its_get(tc->old_ptr)));
-
+		TEST(tv->func(timer, tc->flag, tst_its_get(&new_set),
+			      tst_its_get(tc->old_ptr)));
 		if (TST_RET != 0) {
-			tst_res(TFAIL | TTERRNO, "%s failed",
-					get_clock_str(clock));
+			tst_res(TFAIL | TTERRNO, "%s: timer_settime(%s) failed",
+				tv->desc, get_clock_str(clock));
 		} else {
-			tst_res(TPASS, "%s was successful",
-					get_clock_str(clock));
+			tst_res(TPASS, "%s: timer_settime(%s) was successful",
+				tv->desc, get_clock_str(clock));
 		}
 
 		TEST(tst_syscall(__NR_timer_delete, timer));
-		if (TST_RET != 0)
-			tst_res(TFAIL | TTERRNO, "timer_delete() failed!");
+		if (TST_RET != 0) {
+			tst_res(TFAIL | TTERRNO, "%s: %s: timer_delete() failed!",
+				tv->desc, get_clock_str(clock));
+		}
 	}
 }
 
-- 
2.25.0.rc1.19.g042ed3e048af

[LTP] [PATCH 2/2] syscalls/timer_settime01: Make sure the timer fires

[Viresh Kumar]

This patch improves the testcase by doing multiple things:

- Make sure the timer fires and catch the signals.

- Verify the values set to the itimerspec by reading them again using
  timer_gettime() syscalls.

- Reduce the timer interval, 5 seconds was way too much.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
Cyril/Arnd,

This works well for all clocks except CLOCK_PROCESS_CPUTIME_ID and
CLOCK_THREAD_CPUTIME_ID for some reason. I tried to read the values for
those clocks by sleeping for 1 second and then reading values using
timer_gettime() in a loop, and the value incremented every 15-16 seconds
by a value of 1 (which was in ms if I am not wrong).

No idea what the hell is going on here and so need experts advice :)

 .../syscalls/timer_settime/timer_settime01.c  | 72 +++++++++++++------
 1 file changed, 52 insertions(+), 20 deletions(-)

diff --git a/testcases/kernel/syscalls/timer_settime/timer_settime01.c b/testcases/kernel/syscalls/timer_settime/timer_settime01.c
index 52c435ee3d91..d786938c7aa5 100644
--- a/testcases/kernel/syscalls/timer_settime/timer_settime01.c
+++ b/testcases/kernel/syscalls/timer_settime/timer_settime01.c
@@ -38,25 +38,25 @@ static struct testcase {
 	int			flag;
 	char			*description;
 } tcases[] = {
-	{NULL,     5, 0, 0, "general initialization"},
-	{&old_set, 5, 0, 0, "setting old_value"},
-	{&old_set, 0, 5, 0, "using periodic timer"},
-	{&old_set, 5, 0, TIMER_ABSTIME, "using absolute time"},
+	{NULL, 1, 0, 0, "general initialization"},
+	{&old_set, 1, 0, 0, "setting old_value"},
+	{&old_set, 1, 1, 0, "using periodic timer"},
+	{&old_set, 1, 0, TIMER_ABSTIME, "using absolute time"},
 };
 
 static struct test_variants {
-	int (*gettime)(clockid_t clk_id, void *ts);
-	int (*func)(timer_t timerid, int flags, void *its,
-		    void *old_its);
+	int (*cgettime)(clockid_t clk_id, void *ts);
+	int (*tgettime)(timer_t timer, void *its);
+	int (*func)(timer_t timerid, int flags, void *its, void *old_its);
 	enum tst_ts_type type;
 	char *desc;
 } variants[] = {
 #if (__NR_timer_settime != __LTP__NR_INVALID_SYSCALL)
-	{ .gettime = sys_clock_gettime, .func = sys_timer_settime, .type = TST_KERN_OLD_TIMESPEC, .desc = "syscall with old kernel spec"},
+	{ .cgettime = sys_clock_gettime, .tgettime = sys_timer_gettime, .func = sys_timer_settime, .type = TST_KERN_OLD_TIMESPEC, .desc = "syscall with old kernel spec"},
 #endif
 
 #if (__NR_timer_settime64 != __LTP__NR_INVALID_SYSCALL)
-	{ .gettime = sys_clock_gettime64, .func = sys_timer_settime64, .type = TST_KERN_TIMESPEC, .desc = "syscall time64 with kernel spec"},
+	{ .cgettime = sys_clock_gettime64, .tgettime = sys_timer_gettime64, .func = sys_timer_settime64, .type = TST_KERN_TIMESPEC, .desc = "syscall time64 with kernel spec"},
 #endif
 };
 
@@ -64,8 +64,23 @@ static void run(unsigned int n)
 {
 	struct test_variants *tv = &variants[tst_variant];
 	struct testcase *tc = &tcases[n];
+	struct sigevent ev = {
+		.sigev_notify = SIGEV_SIGNAL,
+		.sigev_signo = SIGABRT,
+	};
 	long long val;
+	sigset_t set;
 	unsigned int i;
+	int sig;
+
+	if (sigemptyset(&set) == -1)
+		tst_brk(TBROK, "sigemptyset() failed");
+
+	if (sigaddset(&set, SIGABRT) == -1)
+		tst_brk(TBROK, "sigaddset() failed");
+
+	if (sigprocmask(SIG_BLOCK, &set, NULL) == -1)
+		tst_brk(TBROK, "sigprocmask() failed");
 
 	tst_res(TINFO, "Testing for %s:", tc->description);
 
@@ -78,7 +93,7 @@ static void run(unsigned int n)
 				continue;
 		}
 
-		TEST(tst_syscall(__NR_timer_create, clock, NULL, &timer));
+		TEST(tst_syscall(__NR_timer_create, clock, &ev, &timer));
 		if (TST_RET != 0) {
 			if (possibly_unsupported(clock) &&
 				(TST_ERR == EINVAL || TST_ERR == ENOTSUP)) {
@@ -101,7 +116,7 @@ static void run(unsigned int n)
 
 		if (tc->flag & TIMER_ABSTIME) {
 			timenow.type = tv->type;
-			if (tv->gettime(clock, tst_ts_get(&timenow)) < 0) {
+			if (tv->cgettime(clock, tst_ts_get(&timenow)) < 0) {
 				tst_res(TFAIL, "%s: clock_gettime(%s) failed - skipping the test",
 					tv->desc, get_clock_str(clock));
 				continue;
@@ -119,11 +134,35 @@ static void run(unsigned int n)
 		if (TST_RET != 0) {
 			tst_res(TFAIL | TTERRNO, "%s: timer_settime(%s) failed",
 				tv->desc, get_clock_str(clock));
-		} else {
-			tst_res(TPASS, "%s: timer_settime(%s) was successful",
+		}
+
+		TEST(tv->tgettime(timer, tst_its_get(&new_set)));
+		if (TST_RET != 0) {
+			tst_res(TFAIL | TTERRNO, "%s: timer_gettime(%s) failed",
 				tv->desc, get_clock_str(clock));
 		}
 
+		if (tst_its_get_interval_sec(new_set) > tc->it_interval_tv_sec ||
+		    tst_its_get_value_sec(new_set) > val) {
+			tst_res(TFAIL | TTERRNO, "%s: timer_gettime(%s) reported bad values (%llu: %llu)",
+				tv->desc, get_clock_str(clock),
+				tst_its_get_interval_sec(new_set),
+				tst_its_get_value_sec(new_set));
+		}
+
+		if (sigwait(&set, &sig) == -1) {
+			tst_brk(TBROK, "%s: %s: sigwait() failed", tv->desc,
+				get_clock_str(clock));
+		} else if (tc->it_interval_tv_sec) {
+			if (sigwait(&set, &sig) == -1) {
+				tst_brk(TBROK, "%s: %s: Second sigwait() failed",
+					tv->desc, get_clock_str(clock));
+			}
+		}
+
+		tst_res(TPASS, "%s: timer_settime(%s) passed", tv->desc,
+			get_clock_str(clock));
+
 		TEST(tst_syscall(__NR_timer_delete, timer));
 		if (TST_RET != 0) {
 			tst_res(TFAIL | TTERRNO, "%s: %s: timer_delete() failed!",
@@ -132,16 +171,9 @@ static void run(unsigned int n)
 	}
 }
 
-static void sighandler(int sig)
-{
-	/* sighandler for CLOCK_*_ALARM */
-	tst_res(TINFO, "Caught signal %s", tst_strsig(sig));
-}
-
 static void setup(void)
 {
 	tst_res(TINFO, "Testing variant: %s", variants[tst_variant].desc);
-	SAFE_SIGNAL(SIGALRM, sighandler);
 }
 
 static struct tst_test test = {
-- 
2.25.0.rc1.19.g042ed3e048af

[LTP] [PATCH 2/2] syscalls/timer_settime01: Make sure the timer fires

[Arnd Bergmann]

On Mon, Jun 29, 2020 at 1:43 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> This patch improves the testcase by doing multiple things:
>
> - Make sure the timer fires and catch the signals.
>
> - Verify the values set to the itimerspec by reading them again using
>   timer_gettime() syscalls.
>
> - Reduce the timer interval, 5 seconds was way too much.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> Cyril/Arnd,
>
> This works well for all clocks except CLOCK_PROCESS_CPUTIME_ID and
> CLOCK_THREAD_CPUTIME_ID for some reason. I tried to read the values for
> those clocks by sleeping for 1 second and then reading values using
> timer_gettime() in a loop, and the value incremented every 15-16 seconds
> by a value of 1 (which was in ms if I am not wrong).
>
> No idea what the hell is going on here and so need experts advice :)

The problem is that these clocks only tick while the process is running. Instead
of sleeping for one second, you need to be in a busy-loop to ensure they
actually advance.

        Arnd

[LTP] [PATCH 2/2] syscalls/timer_settime01: Make sure the timer fires

[Cyril Hrubis]

Hi!
> > This works well for all clocks except CLOCK_PROCESS_CPUTIME_ID and
> > CLOCK_THREAD_CPUTIME_ID for some reason. I tried to read the values for
> > those clocks by sleeping for 1 second and then reading values using
> > timer_gettime() in a loop, and the value incremented every 15-16 seconds
> > by a value of 1 (which was in ms if I am not wrong).
> >
> > No idea what the hell is going on here and so need experts advice :)
> 
> The problem is that these clocks only tick while the process is running. Instead
> of sleeping for one second, you need to be in a busy-loop to ensure they
> actually advance.

Indeed, we may as well do something as:

	while (!caught_signal);

instead of sleep() in the case of the CPUTIME clocks.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 2/2] syscalls/timer_settime01: Make sure the timer fires

[Viresh Kumar]

On 29-06-20, 14:13, Cyril Hrubis wrote:
> Hi!
> > > This works well for all clocks except CLOCK_PROCESS_CPUTIME_ID and
> > > CLOCK_THREAD_CPUTIME_ID for some reason. I tried to read the values for
> > > those clocks by sleeping for 1 second and then reading values using
> > > timer_gettime() in a loop, and the value incremented every 15-16 seconds
> > > by a value of 1 (which was in ms if I am not wrong).
> > >
> > > No idea what the hell is going on here and so need experts advice :)
> > 
> > The problem is that these clocks only tick while the process is running. Instead
> > of sleeping for one second, you need to be in a busy-loop to ensure they
> > actually advance.
> 
> Indeed, we may as well do something as:
> 
> 	while (!caught_signal);
> 
> instead of sleep() in the case of the CPUTIME clocks.

So I did the right thing by asking and not wasting time :)

-- 
viresh

[LTP] [PATCH V1.1 2/2] syscalls/timer_settime01: Make sure the timer fires

[Viresh Kumar]

This patch improves the testcase by doing multiple things:

- Make sure the timer fires and catch the signals.

- Verify the values set to the itimerspec by reading them again using
  timer_gettime() syscalls.

- Reduce the timer interval, 5 seconds was way too much.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
V1.1:
- Use busy loop while waiting for signal and use the existing signal
  mechanism.

 .../syscalls/timer_settime/timer_settime01.c  | 63 ++++++++++++++-----
 1 file changed, 48 insertions(+), 15 deletions(-)

diff --git a/testcases/kernel/syscalls/timer_settime/timer_settime01.c b/testcases/kernel/syscalls/timer_settime/timer_settime01.c
index 52c435ee3d91..7e53fce23527 100644
--- a/testcases/kernel/syscalls/timer_settime/timer_settime01.c
+++ b/testcases/kernel/syscalls/timer_settime/timer_settime01.c
@@ -38,28 +38,42 @@ static struct testcase {
 	int			flag;
 	char			*description;
 } tcases[] = {
-	{NULL,     5, 0, 0, "general initialization"},
-	{&old_set, 5, 0, 0, "setting old_value"},
-	{&old_set, 0, 5, 0, "using periodic timer"},
-	{&old_set, 5, 0, TIMER_ABSTIME, "using absolute time"},
+	{NULL, 1, 0, 0, "general initialization"},
+	{&old_set, 1, 0, 0, "setting old_value"},
+	{&old_set, 1, 1, 0, "using periodic timer"},
+	{&old_set, 1, 0, TIMER_ABSTIME, "using absolute time"},
 };
 
 static struct test_variants {
-	int (*gettime)(clockid_t clk_id, void *ts);
-	int (*func)(timer_t timerid, int flags, void *its,
-		    void *old_its);
+	int (*cgettime)(clockid_t clk_id, void *ts);
+	int (*tgettime)(timer_t timer, void *its);
+	int (*func)(timer_t timerid, int flags, void *its, void *old_its);
 	enum tst_ts_type type;
 	char *desc;
 } variants[] = {
 #if (__NR_timer_settime != __LTP__NR_INVALID_SYSCALL)
-	{ .gettime = sys_clock_gettime, .func = sys_timer_settime, .type = TST_KERN_OLD_TIMESPEC, .desc = "syscall with old kernel spec"},
+	{ .cgettime = sys_clock_gettime, .tgettime = sys_timer_gettime, .func = sys_timer_settime, .type = TST_KERN_OLD_TIMESPEC, .desc = "syscall with old kernel spec"},
 #endif
 
 #if (__NR_timer_settime64 != __LTP__NR_INVALID_SYSCALL)
-	{ .gettime = sys_clock_gettime64, .func = sys_timer_settime64, .type = TST_KERN_TIMESPEC, .desc = "syscall time64 with kernel spec"},
+	{ .cgettime = sys_clock_gettime64, .tgettime = sys_timer_gettime64, .func = sys_timer_settime64, .type = TST_KERN_TIMESPEC, .desc = "syscall time64 with kernel spec"},
 #endif
 };
 
+static volatile int caught_signal;
+
+static void clear_signal(void)
+{
+	/*
+	 * The busy loop is intentional. The signal is sent after X
+	 * seconds of CPU time has been accumulated for the process and
+	 * thread specific clocks.
+	 */
+	while (!caught_signal);
+
+	caught_signal = 0;
+}
+
 static void run(unsigned int n)
 {
 	struct test_variants *tv = &variants[tst_variant];
@@ -101,7 +115,7 @@ static void run(unsigned int n)
 
 		if (tc->flag & TIMER_ABSTIME) {
 			timenow.type = tv->type;
-			if (tv->gettime(clock, tst_ts_get(&timenow)) < 0) {
+			if (tv->cgettime(clock, tst_ts_get(&timenow)) < 0) {
 				tst_res(TFAIL, "%s: clock_gettime(%s) failed - skipping the test",
 					tv->desc, get_clock_str(clock));
 				continue;
@@ -119,11 +133,31 @@ static void run(unsigned int n)
 		if (TST_RET != 0) {
 			tst_res(TFAIL | TTERRNO, "%s: timer_settime(%s) failed",
 				tv->desc, get_clock_str(clock));
-		} else {
-			tst_res(TPASS, "%s: timer_settime(%s) was successful",
+		}
+
+		TEST(tv->tgettime(timer, tst_its_get(&new_set)));
+		if (TST_RET != 0) {
+			tst_res(TFAIL | TTERRNO, "%s: timer_gettime(%s) failed",
 				tv->desc, get_clock_str(clock));
 		}
 
+		if (tst_its_get_interval_sec(new_set) > tc->it_interval_tv_sec ||
+		    tst_its_get_value_sec(new_set) > val) {
+			tst_res(TFAIL | TTERRNO, "%s: timer_gettime(%s) reported bad values (%llu: %llu)",
+				tv->desc, get_clock_str(clock),
+				tst_its_get_interval_sec(new_set),
+				tst_its_get_value_sec(new_set));
+		}
+
+		clear_signal();
+
+		/* Wait for another event when interval was set */
+		if (tc->it_interval_tv_sec)
+			clear_signal();
+
+		tst_res(TPASS, "%s: timer_settime(%s) passed", tv->desc,
+			get_clock_str(clock));
+
 		TEST(tst_syscall(__NR_timer_delete, timer));
 		if (TST_RET != 0) {
 			tst_res(TFAIL | TTERRNO, "%s: %s: timer_delete() failed!",
@@ -132,10 +166,9 @@ static void run(unsigned int n)
 	}
 }
 
-static void sighandler(int sig)
+static void sighandler(int sig LTP_ATTRIBUTE_UNUSED)
 {
-	/* sighandler for CLOCK_*_ALARM */
-	tst_res(TINFO, "Caught signal %s", tst_strsig(sig));
+	caught_signal = 1;
 }
 
 static void setup(void)
-- 
2.25.0.rc1.19.g042ed3e048af

[LTP] [PATCH 1/2] syscalls/timer_settime01: Improve print messages

[Cyril Hrubis]

Hi!
> This improves the print messages by providing additional information
> about the tests.

Do we really need this?

We do print which test variant we test at the start of each block and no
other tests with test variant prefix the messages like this...

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 1/2] syscalls/timer_settime01: Improve print messages

[Viresh Kumar]

On 02-07-20, 15:34, Cyril Hrubis wrote:
> Hi!
> > This improves the print messages by providing additional information
> > about the tests.
> 
> Do we really need this?
> 
> We do print which test variant we test at the start of each block and no
> other tests with test variant prefix the messages like this...

Damn, I missed that somehow and thought some information is missing :(

-- 
viresh

[LTP] [PATCH V2] syscalls/timer_settime01: Make sure the timer fires

[Viresh Kumar]

This patch improves the testcase by doing multiple things:

- Make sure the timer fires and catch the signals.

- Verify the values set to the itimerspec by reading them again using
  timer_gettime() syscalls.

- Reduce the timer interval, 5 seconds was way too much.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
V1.1->V2:
- 1/2 patch removed and solved rebase conflicts.

 .../syscalls/timer_settime/timer_settime01.c  | 70 ++++++++++++++-----
 1 file changed, 52 insertions(+), 18 deletions(-)

diff --git a/testcases/kernel/syscalls/timer_settime/timer_settime01.c b/testcases/kernel/syscalls/timer_settime/timer_settime01.c
index 08fb56e4943a..67769d088ab8 100644
--- a/testcases/kernel/syscalls/timer_settime/timer_settime01.c
+++ b/testcases/kernel/syscalls/timer_settime/timer_settime01.c
@@ -38,28 +38,42 @@ static struct testcase {
 	int			flag;
 	char			*description;
 } tcases[] = {
-	{NULL,     5, 0, 0, "general initialization"},
-	{&old_set, 5, 0, 0, "setting old_value"},
-	{&old_set, 0, 5, 0, "using periodic timer"},
-	{&old_set, 5, 0, TIMER_ABSTIME, "using absolute time"},
+	{NULL, 1, 0, 0, "general initialization"},
+	{&old_set, 1, 0, 0, "setting old_value"},
+	{&old_set, 1, 1, 0, "using periodic timer"},
+	{&old_set, 1, 0, TIMER_ABSTIME, "using absolute time"},
 };
 
 static struct test_variants {
-	int (*gettime)(clockid_t clk_id, void *ts);
-	int (*func)(timer_t timerid, int flags, void *its,
-		    void *old_its);
+	int (*cgettime)(clockid_t clk_id, void *ts);
+	int (*tgettime)(timer_t timer, void *its);
+	int (*func)(timer_t timerid, int flags, void *its, void *old_its);
 	enum tst_ts_type type;
 	char *desc;
 } variants[] = {
 #if (__NR_timer_settime != __LTP__NR_INVALID_SYSCALL)
-	{ .gettime = sys_clock_gettime, .func = sys_timer_settime, .type = TST_KERN_OLD_TIMESPEC, .desc = "syscall with old kernel spec"},
+	{ .cgettime = sys_clock_gettime, .tgettime = sys_timer_gettime, .func = sys_timer_settime, .type = TST_KERN_OLD_TIMESPEC, .desc = "syscall with old kernel spec"},
 #endif
 
 #if (__NR_timer_settime64 != __LTP__NR_INVALID_SYSCALL)
-	{ .gettime = sys_clock_gettime64, .func = sys_timer_settime64, .type = TST_KERN_TIMESPEC, .desc = "syscall time64 with kernel spec"},
+	{ .cgettime = sys_clock_gettime64, .tgettime = sys_timer_gettime64, .func = sys_timer_settime64, .type = TST_KERN_TIMESPEC, .desc = "syscall time64 with kernel spec"},
 #endif
 };
 
+static volatile int caught_signal;
+
+static void clear_signal(void)
+{
+	/*
+	 * The busy loop is intentional. The signal is sent after X
+	 * seconds of CPU time has been accumulated for the process and
+	 * thread specific clocks.
+	 */
+	while (!caught_signal);
+
+	caught_signal = 0;
+}
+
 static void run(unsigned int n)
 {
 	struct test_variants *tv = &variants[tst_variant];
@@ -101,7 +115,7 @@ static void run(unsigned int n)
 
 		if (tc->flag & TIMER_ABSTIME) {
 			timenow.type = tv->type;
-			if (tv->gettime(clock, tst_ts_get(&timenow)) < 0) {
+			if (tv->cgettime(clock, tst_ts_get(&timenow)) < 0) {
 				tst_res(TFAIL,
 					"clock_gettime(%s) failed - skipping the test",
 					get_clock_str(clock));
@@ -118,23 +132,43 @@ static void run(unsigned int n)
 		TEST(tv->func(timer, tc->flag, tst_its_get(&new_set), tst_its_get(tc->old_ptr)));
 
 		if (TST_RET != 0) {
-			tst_res(TFAIL | TTERRNO, "%s failed",
-					get_clock_str(clock));
-		} else {
-			tst_res(TPASS, "%s was successful",
-					get_clock_str(clock));
+			tst_res(TFAIL | TTERRNO, "timer_settime(%s) failed",
+				get_clock_str(clock));
+		}
+
+		TEST(tv->tgettime(timer, tst_its_get(&new_set)));
+		if (TST_RET != 0) {
+			tst_res(TFAIL | TTERRNO, "timer_gettime(%s) failed",
+				get_clock_str(clock));
+		}
+
+		if (tst_its_get_interval_sec(new_set) > tc->it_interval_tv_sec ||
+		    tst_its_get_value_sec(new_set) > val) {
+			tst_res(TFAIL | TTERRNO,
+				"timer_gettime(%s) reported bad values (%llu: %llu)",
+				get_clock_str(clock),
+				tst_its_get_interval_sec(new_set),
+				tst_its_get_value_sec(new_set));
 		}
 
+		clear_signal();
+
+		/* Wait for another event when interval was set */
+		if (tc->it_interval_tv_sec)
+			clear_signal();
+
+		tst_res(TPASS, "timer_settime(%s) passed",
+			get_clock_str(clock));
+
 		TEST(tst_syscall(__NR_timer_delete, timer));
 		if (TST_RET != 0)
 			tst_res(TFAIL | TTERRNO, "timer_delete() failed!");
 	}
 }
 
-static void sighandler(int sig)
+static void sighandler(int sig LTP_ATTRIBUTE_UNUSED)
 {
-	/* sighandler for CLOCK_*_ALARM */
-	tst_res(TINFO, "Caught signal %s", tst_strsig(sig));
+	caught_signal = 1;
 }
 
 static void setup(void)
-- 
2.25.0.rc1.19.g042ed3e048af

[LTP] [PATCH V2] syscalls/timer_settime01: Make sure the timer fires

[Cyril Hrubis]

Hi!
> - Make sure the timer fires and catch the signals.
> 
> - Verify the values set to the itimerspec by reading them again using
>   timer_gettime() syscalls.
> 
> - Reduce the timer interval, 5 seconds was way too much.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> V1.1->V2:
> - 1/2 patch removed and solved rebase conflicts.
> 
>  .../syscalls/timer_settime/timer_settime01.c  | 70 ++++++++++++++-----
>  1 file changed, 52 insertions(+), 18 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/timer_settime/timer_settime01.c b/testcases/kernel/syscalls/timer_settime/timer_settime01.c
> index 08fb56e4943a..67769d088ab8 100644
> --- a/testcases/kernel/syscalls/timer_settime/timer_settime01.c
> +++ b/testcases/kernel/syscalls/timer_settime/timer_settime01.c
> @@ -38,28 +38,42 @@ static struct testcase {
>  	int			flag;
>  	char			*description;
>  } tcases[] = {
> -	{NULL,     5, 0, 0, "general initialization"},
> -	{&old_set, 5, 0, 0, "setting old_value"},
> -	{&old_set, 0, 5, 0, "using periodic timer"},
> -	{&old_set, 5, 0, TIMER_ABSTIME, "using absolute time"},
> +	{NULL, 1, 0, 0, "general initialization"},
> +	{&old_set, 1, 0, 0, "setting old_value"},
> +	{&old_set, 1, 1, 0, "using periodic timer"},
> +	{&old_set, 1, 0, TIMER_ABSTIME, "using absolute time"},
>  };

One second is pretty much too long as well. Can we please reduce this to
0.1s instead?

>  static struct test_variants {
> -	int (*gettime)(clockid_t clk_id, void *ts);
> -	int (*func)(timer_t timerid, int flags, void *its,
> -		    void *old_its);
> +	int (*cgettime)(clockid_t clk_id, void *ts);
> +	int (*tgettime)(timer_t timer, void *its);
> +	int (*func)(timer_t timerid, int flags, void *its, void *old_its);
>  	enum tst_ts_type type;
>  	char *desc;
>  } variants[] = {
>  #if (__NR_timer_settime != __LTP__NR_INVALID_SYSCALL)
> -	{ .gettime = sys_clock_gettime, .func = sys_timer_settime, .type = TST_KERN_OLD_TIMESPEC, .desc = "syscall with old kernel spec"},
> +	{ .cgettime = sys_clock_gettime, .tgettime = sys_timer_gettime, .func = sys_timer_settime, .type = TST_KERN_OLD_TIMESPEC, .desc = "syscall with old kernel spec"},
>  #endif
>  
>  #if (__NR_timer_settime64 != __LTP__NR_INVALID_SYSCALL)
> -	{ .gettime = sys_clock_gettime64, .func = sys_timer_settime64, .type = TST_KERN_TIMESPEC, .desc = "syscall time64 with kernel spec"},
> +	{ .cgettime = sys_clock_gettime64, .tgettime = sys_timer_gettime64, .func = sys_timer_settime64, .type = TST_KERN_TIMESPEC, .desc = "syscall time64 with kernel spec"},
>  #endif
>  };
>  
> +static volatile int caught_signal;
> +
> +static void clear_signal(void)
> +{
> +	/*
> +	 * The busy loop is intentional. The signal is sent after X
> +	 * seconds of CPU time has been accumulated for the process and
> +	 * thread specific clocks.
> +	 */
> +	while (!caught_signal);
> +
> +	caught_signal = 0;
> +}
> +
>  static void run(unsigned int n)
>  {
>  	struct test_variants *tv = &variants[tst_variant];
> @@ -101,7 +115,7 @@ static void run(unsigned int n)
>  
>  		if (tc->flag & TIMER_ABSTIME) {
>  			timenow.type = tv->type;
> -			if (tv->gettime(clock, tst_ts_get(&timenow)) < 0) {
> +			if (tv->cgettime(clock, tst_ts_get(&timenow)) < 0) {
>  				tst_res(TFAIL,
>  					"clock_gettime(%s) failed - skipping the test",
>  					get_clock_str(clock));
> @@ -118,23 +132,43 @@ static void run(unsigned int n)
>  		TEST(tv->func(timer, tc->flag, tst_its_get(&new_set), tst_its_get(tc->old_ptr)));
>  
>  		if (TST_RET != 0) {
> -			tst_res(TFAIL | TTERRNO, "%s failed",
> -					get_clock_str(clock));
> -		} else {
> -			tst_res(TPASS, "%s was successful",
> -					get_clock_str(clock));
> +			tst_res(TFAIL | TTERRNO, "timer_settime(%s) failed",
> +				get_clock_str(clock));
> +		}
> +
> +		TEST(tv->tgettime(timer, tst_its_get(&new_set)));
> +		if (TST_RET != 0) {
> +			tst_res(TFAIL | TTERRNO, "timer_gettime(%s) failed",
> +				get_clock_str(clock));
> +		}

If we fail here we should probably skip at least new_set checks below.

> +		if (tst_its_get_interval_sec(new_set) > tc->it_interval_tv_sec ||

The interval value should be equal to the one we have set, so we can
compare exactly.

> +		    tst_its_get_value_sec(new_set) > val) {

And the time to the next expiration is always relative, so this should
be:

		    if (value > MAX(tc->interval, tc->value))

> +			tst_res(TFAIL | TTERRNO,
> +				"timer_gettime(%s) reported bad values (%llu: %llu)",
> +				get_clock_str(clock),
> +				tst_its_get_interval_sec(new_set),
> +				tst_its_get_value_sec(new_set));
>  		}
>  
> +		clear_signal();
> +
> +		/* Wait for another event when interval was set */
> +		if (tc->it_interval_tv_sec)
> +			clear_signal();
> +
> +		tst_res(TPASS, "timer_settime(%s) passed",
> +			get_clock_str(clock));
> +
>  		TEST(tst_syscall(__NR_timer_delete, timer));
>  		if (TST_RET != 0)
>  			tst_res(TFAIL | TTERRNO, "timer_delete() failed!");
>  	}
>  }
>  
> -static void sighandler(int sig)
> +static void sighandler(int sig LTP_ATTRIBUTE_UNUSED)
>  {
> -	/* sighandler for CLOCK_*_ALARM */
> -	tst_res(TINFO, "Caught signal %s", tst_strsig(sig));
> +	caught_signal = 1;
>  }

We should check that sig has correct value as well, so I guess
that we can set caught_signal to -1 if sig != SIGARLM or something.

>  static void setup(void)
> -- 
> 2.25.0.rc1.19.g042ed3e048af
> 

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH V3] syscalls/timer_settime01: Make sure the timer fires

[Viresh Kumar]

This patch improves the testcase by doing multiple things:

- Make sure the timer fires and catch the signals.

- Verify the values set to the itimerspec by reading them again using
  timer_gettime() syscalls.

- Reduce the timer interval, 5 seconds was way too much.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
V3:
- Changed timer values to 100 ms instead of 1 sec and that resulted in
  few changes.
- Moved some code around to keep related bits together.
- Improved check for testing value read from timer.

 .../syscalls/timer_settime/timer_settime01.c  | 106 ++++++++++++------
 1 file changed, 70 insertions(+), 36 deletions(-)

diff --git a/testcases/kernel/syscalls/timer_settime/timer_settime01.c b/testcases/kernel/syscalls/timer_settime/timer_settime01.c
index 08fb56e4943a..f9d1456dabe9 100644
--- a/testcases/kernel/syscalls/timer_settime/timer_settime01.c
+++ b/testcases/kernel/syscalls/timer_settime/timer_settime01.c
@@ -33,33 +33,61 @@ static timer_t timer;
 
 static struct testcase {
 	struct tst_its		*old_ptr;
-	int			it_value_tv_sec;
-	int			it_interval_tv_sec;
+	int			it_value_tv_usec;
+	int			it_interval_tv_usec;
 	int			flag;
 	char			*description;
 } tcases[] = {
-	{NULL,     5, 0, 0, "general initialization"},
-	{&old_set, 5, 0, 0, "setting old_value"},
-	{&old_set, 0, 5, 0, "using periodic timer"},
-	{&old_set, 5, 0, TIMER_ABSTIME, "using absolute time"},
+	{NULL, 100000, 0, 0, "general initialization"},
+	{&old_set, 100000, 0, 0, "setting old_value"},
+	{&old_set, 100000, 100000, 0, "using periodic timer"},
+	{&old_set, 100000, 0, TIMER_ABSTIME, "using absolute time"},
 };
 
 static struct test_variants {
-	int (*gettime)(clockid_t clk_id, void *ts);
-	int (*func)(timer_t timerid, int flags, void *its,
-		    void *old_its);
+	int (*cgettime)(clockid_t clk_id, void *ts);
+	int (*tgettime)(timer_t timer, void *its);
+	int (*func)(timer_t timerid, int flags, void *its, void *old_its);
 	enum tst_ts_type type;
 	char *desc;
 } variants[] = {
 #if (__NR_timer_settime != __LTP__NR_INVALID_SYSCALL)
-	{ .gettime = sys_clock_gettime, .func = sys_timer_settime, .type = TST_KERN_OLD_TIMESPEC, .desc = "syscall with old kernel spec"},
+	{ .cgettime = sys_clock_gettime, .tgettime = sys_timer_gettime, .func = sys_timer_settime, .type = TST_KERN_OLD_TIMESPEC, .desc = "syscall with old kernel spec"},
 #endif
 
 #if (__NR_timer_settime64 != __LTP__NR_INVALID_SYSCALL)
-	{ .gettime = sys_clock_gettime64, .func = sys_timer_settime64, .type = TST_KERN_TIMESPEC, .desc = "syscall time64 with kernel spec"},
+	{ .cgettime = sys_clock_gettime64, .tgettime = sys_timer_gettime64, .func = sys_timer_settime64, .type = TST_KERN_TIMESPEC, .desc = "syscall time64 with kernel spec"},
 #endif
 };
 
+static volatile int caught_signal;
+
+static void clear_signal(void)
+{
+	/*
+	 * The busy loop is intentional. The signal is sent after X
+	 * seconds of CPU time has been accumulated for the process and
+	 * thread specific clocks.
+	 */
+	while (!caught_signal);
+
+	caught_signal = 0;
+}
+
+static void sighandler(int sig)
+{
+	if (sig != SIGALRM)
+		tst_res(TFAIL, "Received incorrect signal: %d", sig);
+
+	caught_signal = 1;
+}
+
+static void setup(void)
+{
+	tst_res(TINFO, "Testing variant: %s", variants[tst_variant].desc);
+	SAFE_SIGNAL(SIGALRM, sighandler);
+}
+
 static void run(unsigned int n)
 {
 	struct test_variants *tv = &variants[tst_variant];
@@ -96,53 +124,59 @@ static void run(unsigned int n)
 		memset(&old_set, 0, sizeof(old_set));
 
 		new_set.type = old_set.type = tv->type;
-
-		val = tc->it_value_tv_sec;
+		val = tc->it_value_tv_usec;
 
 		if (tc->flag & TIMER_ABSTIME) {
 			timenow.type = tv->type;
-			if (tv->gettime(clock, tst_ts_get(&timenow)) < 0) {
+			if (tv->cgettime(clock, tst_ts_get(&timenow)) < 0) {
 				tst_res(TFAIL,
 					"clock_gettime(%s) failed - skipping the test",
 					get_clock_str(clock));
 				continue;
 			}
-			val += tst_ts_get_sec(timenow);
+			val += tst_ts_to_us(timenow);
 		}
 
-		tst_its_set_interval_sec(&new_set, tc->it_interval_tv_sec);
-		tst_its_set_interval_nsec(&new_set, 0);
-		tst_its_set_value_sec(&new_set, val);
-		tst_its_set_value_nsec(&new_set, 0);
+		tst_its_set_interval_from_us(&new_set, tc->it_interval_tv_usec);
+		tst_its_set_value_from_us(&new_set, val);
 
 		TEST(tv->func(timer, tc->flag, tst_its_get(&new_set), tst_its_get(tc->old_ptr)));
 
 		if (TST_RET != 0) {
-			tst_res(TFAIL | TTERRNO, "%s failed",
-					get_clock_str(clock));
-		} else {
-			tst_res(TPASS, "%s was successful",
-					get_clock_str(clock));
+			tst_res(TFAIL | TTERRNO, "timer_settime(%s) failed",
+				get_clock_str(clock));
 		}
 
+		TEST(tv->tgettime(timer, tst_its_get(&new_set)));
+		if (TST_RET != 0) {
+			tst_res(TFAIL | TTERRNO, "timer_gettime(%s) failed",
+				get_clock_str(clock));
+		} else if ((tst_its_get_interval_nsec(new_set) !=
+				tc->it_interval_tv_usec * 1000) ||
+			   (tst_its_get_value_nsec(new_set) >
+				MAX(tc->it_value_tv_usec * 1000, tc->it_interval_tv_usec * 1000))) {
+			tst_res(TFAIL | TTERRNO,
+				"timer_gettime(%s) reported bad values (%llu: %llu)",
+				get_clock_str(clock),
+				tst_its_get_interval_nsec(new_set),
+				tst_its_get_value_nsec(new_set));
+		}
+
+		clear_signal();
+
+		/* Wait for another event when interval was set */
+		if (tc->it_interval_tv_usec)
+			clear_signal();
+
+		tst_res(TPASS, "timer_settime(%s) passed",
+			get_clock_str(clock));
+
 		TEST(tst_syscall(__NR_timer_delete, timer));
 		if (TST_RET != 0)
 			tst_res(TFAIL | TTERRNO, "timer_delete() failed!");
 	}
 }
 
-static void sighandler(int sig)
-{
-	/* sighandler for CLOCK_*_ALARM */
-	tst_res(TINFO, "Caught signal %s", tst_strsig(sig));
-}
-
-static void setup(void)
-{
-	tst_res(TINFO, "Testing variant: %s", variants[tst_variant].desc);
-	SAFE_SIGNAL(SIGALRM, sighandler);
-}
-
 static struct tst_test test = {
 	.test = run,
 	.needs_root = 1,
-- 
2.25.0.rc1.19.g042ed3e048af

[LTP] [PATCH V3] syscalls/timer_settime01: Make sure the timer fires

[Cyril Hrubis]

Hi!
Pushed with minor changes, thanks.

Among other fixes I've moved the signal check from the signal handler to
the clear_signal() function, since the tst_res() function is not
signal-async-safe...

diff --git a/testcases/kernel/syscalls/timer_settime/timer_settime01.c b/testcases/kernel/syscalls/timer_settime/timer_settime01.c
index f9d1456da..76f283b81 100644
--- a/testcases/kernel/syscalls/timer_settime/timer_settime01.c
+++ b/testcases/kernel/syscalls/timer_settime/timer_settime01.c
@@ -10,7 +10,7 @@
 /*
  * This tests the timer_settime(2) syscall under various conditions:
  *
- * 1) General initialization: No old_value, no flags, 5-second-timer
+ * 1) General initialization: No old_value, no flags
  * 2) Setting a pointer to a itimerspec struct as old_set parameter
  * 3) Using a periodic timer
  * 4) Using absolute time
@@ -38,10 +38,10 @@ static struct testcase {
 	int			flag;
 	char			*description;
 } tcases[] = {
-	{NULL, 100000, 0, 0, "general initialization"},
-	{&old_set, 100000, 0, 0, "setting old_value"},
-	{&old_set, 100000, 100000, 0, "using periodic timer"},
-	{&old_set, 100000, 0, TIMER_ABSTIME, "using absolute time"},
+	{NULL, 50000, 0, 0, "general initialization"},
+	{&old_set, 50000, 0, 0, "setting old_value"},
+	{&old_set, 50000, 50000, 0, "using periodic timer"},
+	{&old_set, 50000, 0, TIMER_ABSTIME, "using absolute time"},
 };
 
 static struct test_variants {
@@ -71,15 +71,17 @@ static void clear_signal(void)
 	 */
 	while (!caught_signal);
 
+	if (caught_signal != SIGALRM) {
+		tst_res(TFAIL, "Received incorrect signal: %s",
+			tst_strsig(caught_signal));
+	}
+
 	caught_signal = 0;
 }
 
 static void sighandler(int sig)
 {
-	if (sig != SIGALRM)
-		tst_res(TFAIL, "Received incorrect signal: %d", sig);
-
-	caught_signal = 1;
+	caught_signal = sig;
 }
 
 static void setup(void)

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH V3] syscalls/timer_settime01: Make sure the timer fires

[Viresh Kumar]

On 14-07-20, 16:28, Cyril Hrubis wrote:
> Hi!
> Pushed with minor changes, thanks.
> 
> Among other fixes I've moved the signal check from the signal handler to
> the clear_signal() function, since the tst_res() function is not
> signal-async-safe...

What does that mean ? I remember that I added it there in
clear_signal() first, but then I tried to print the stuff (forcefully)
from sighandler() and it worked without any issues and so I did that.

-- 
viresh

[LTP] [PATCH V3] syscalls/timer_settime01: Make sure the timer fires

[Cyril Hrubis]

Hi!
> > Pushed with minor changes, thanks.
> > 
> > Among other fixes I've moved the signal check from the signal handler to
> > the clear_signal() function, since the tst_res() function is not
> > signal-async-safe...
> 
> What does that mean ? I remember that I added it there in
> clear_signal() first, but then I tried to print the stuff (forcefully)
> from sighandler() and it worked without any issues and so I did that.

Calling printf() from signal handler is not safe, which tst_res() does.

It will work fine 99% of the cases and this is the reason most people do
it anyways. One of the failure modes is the malloc arena lock. Printf
may allocate temporary buffer so if the signal comes when the process is
in the malloc code the process will hang forewer because it will try to
acquire a lock that has been locked before we jumped into the signal
handler, but there is probably more. It usually gets triggered on
stranger architectures, x86 is quite forgiving in this regard.

Signal handlers are generally tricky and there is a list of functions
that carefuly avoid messing with any global state and can be safely used
from withing a signal handler (man 7 signal-safety) anything else is,
unlikely to fail, but still potential hazard.

Generally the safest approach to signal handlers is to set a variable
and exit, which is what I usually do in tests...

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH V3] syscalls/timer_settime01: Make sure the timer fires

[Yang Xu]

Hi Viresh, Cyril

When tesing timer_settime01 on 3.10.0-1136.el7.x86_64, this case fails 
whether we use any type clock.

timer_settime01.c:174: PASS: timer_settime(CLOCK_BOOTTIME) passed
timer_settime01.c:164: FAIL: timer_gettime(CLOCK_BOOTTIME_ALARM) 
reported bad values (0: 678013000): SUCCESS (0)
timer_settime01.c:174: PASS: timer_settime(CLOCK_BOOTTIME_ALARM) passed
timer_settime01.c:164: FAIL: timer_gettime(CLOCK_REALTIME_ALARM) 
reported bad values (0: 358240000): SUCCESS (0)
timer_settime01.c:174: PASS: timer_settime(CLOCK_REALTIME_ALARM) passed
timer_settime01.c:174: PASS: timer_settime(CLOCK_TAI) passed

Best Regards
Yang Xu


> Hi!
> Pushed with minor changes, thanks.
> 
> Among other fixes I've moved the signal check from the signal handler to
> the clear_signal() function, since the tst_res() function is not
> signal-async-safe...
> 
> diff --git a/testcases/kernel/syscalls/timer_settime/timer_settime01.c b/testcases/kernel/syscalls/timer_settime/timer_settime01.c
> index f9d1456da..76f283b81 100644
> --- a/testcases/kernel/syscalls/timer_settime/timer_settime01.c
> +++ b/testcases/kernel/syscalls/timer_settime/timer_settime01.c
> @@ -10,7 +10,7 @@
>   /*
>    * This tests the timer_settime(2) syscall under various conditions:
>    *
> - * 1) General initialization: No old_value, no flags, 5-second-timer
> + * 1) General initialization: No old_value, no flags
>    * 2) Setting a pointer to a itimerspec struct as old_set parameter
>    * 3) Using a periodic timer
>    * 4) Using absolute time
> @@ -38,10 +38,10 @@ static struct testcase {
>   	int			flag;
>   	char			*description;
>   } tcases[] = {
> -	{NULL, 100000, 0, 0, "general initialization"},
> -	{&old_set, 100000, 0, 0, "setting old_value"},
> -	{&old_set, 100000, 100000, 0, "using periodic timer"},
> -	{&old_set, 100000, 0, TIMER_ABSTIME, "using absolute time"},
> +	{NULL, 50000, 0, 0, "general initialization"},
> +	{&old_set, 50000, 0, 0, "setting old_value"},
> +	{&old_set, 50000, 50000, 0, "using periodic timer"},
> +	{&old_set, 50000, 0, TIMER_ABSTIME, "using absolute time"},
>   };
>   
>   static struct test_variants {
> @@ -71,15 +71,17 @@ static void clear_signal(void)
>   	 */
>   	while (!caught_signal);
>   
> +	if (caught_signal != SIGALRM) {
> +		tst_res(TFAIL, "Received incorrect signal: %s",
> +			tst_strsig(caught_signal));
> +	}
> +
>   	caught_signal = 0;
>   }
>   
>   static void sighandler(int sig)
>   {
> -	if (sig != SIGALRM)
> -		tst_res(TFAIL, "Received incorrect signal: %d", sig);
> -
> -	caught_signal = 1;
> +	caught_signal = sig;
>   }
>   
>   static void setup(void)
> 

[LTP] [PATCH V3] syscalls/timer_settime01: Make sure the timer fires

[Viresh Kumar]

Hi,

On 23-07-20, 10:39, Yang Xu wrote:
> When tesing timer_settime01 on 3.10.0-1136.el7.x86_64, this case fails
> whether we use any type clock.
> 
> timer_settime01.c:174: PASS: timer_settime(CLOCK_BOOTTIME) passed
> timer_settime01.c:164: FAIL: timer_gettime(CLOCK_BOOTTIME_ALARM) reported
> bad values (0: 678013000): SUCCESS (0)
> timer_settime01.c:174: PASS: timer_settime(CLOCK_BOOTTIME_ALARM) passed
> timer_settime01.c:164: FAIL: timer_gettime(CLOCK_REALTIME_ALARM) reported
> bad values (0: 358240000): SUCCESS (0)
> timer_settime01.c:174: PASS: timer_settime(CLOCK_REALTIME_ALARM) passed
> timer_settime01.c:174: PASS: timer_settime(CLOCK_TAI) passed

Can you share the complete test log? I am not sure if only the _ALARM
cocks are failing or all. You are getting values in the order of
300-700 ms, while the max value can't be greater than 50 ms. So seems
like a kernel issue to me. Over that, both _ALARM type clocks weren't
supported before 3.11 and looks like your kernel version is 3.10.

-- 
viresh

[LTP] [PATCH V3] syscalls/timer_settime01: Make sure the timer fires

[Yang Xu]

Hi Virsh


> Hi,
> 
> On 23-07-20, 10:39, Yang Xu wrote:
>> When tesing timer_settime01 on 3.10.0-1136.el7.x86_64, this case fails
>> whether we use any type clock.
>>
>> timer_settime01.c:174: PASS: timer_settime(CLOCK_BOOTTIME) passed
>> timer_settime01.c:164: FAIL: timer_gettime(CLOCK_BOOTTIME_ALARM) reported
>> bad values (0: 678013000): SUCCESS (0)
>> timer_settime01.c:174: PASS: timer_settime(CLOCK_BOOTTIME_ALARM) passed
>> timer_settime01.c:164: FAIL: timer_gettime(CLOCK_REALTIME_ALARM) reported
>> bad values (0: 358240000): SUCCESS (0)
>> timer_settime01.c:174: PASS: timer_settime(CLOCK_REALTIME_ALARM) passed
>> timer_settime01.c:174: PASS: timer_settime(CLOCK_TAI) passed
> 
> Can you share the complete test log? I am not sure if only the _ALARM
> cocks are failing or all. You are getting values in the order of
> 300-700 ms, while the max value can't be greater than 50 ms. So seems
> like a kernel issue to me. Over that, both _ALARM type clocks weren't
> supported before 3.11 and looks like your kernel version is 3.10.
Yes, only _ALARM fails. I  only find a kernel patch (commit 
11ffa9d6065f344  timerfd: Add alarm timers) introduced alarm clock types 
for timefd in kernel 3.11 and  a kernel patch (commit 9a7adcf5c6dea63d 
timers: Posix interface for alarm-timers)  in kernel 3.1. It seems my 
kernel version has supported this two alarm clock, but not sure why this 
case fails.


  the full log as below:
tst_test.c:1247: INFO: Timeout per run is 0h 05m 00s
timer_settime01.c:89: INFO: Testing variant: syscall with old kernel spec
timer_settime01.c:100: INFO: Testing for general initialization:
timer_settime01.c:174: PASS: timer_settime(CLOCK_REALTIME) passed
timer_settime01.c:174: PASS: timer_settime(CLOCK_MONOTONIC) passed
timer_settime01.c:174: PASS: timer_settime(CLOCK_PROCESS_CPUTIME_ID) passed
timer_settime01.c:174: PASS: timer_settime(CLOCK_THREAD_CPUTIME_ID) passed
timer_settime01.c:174: PASS: timer_settime(CLOCK_BOOTTIME) passed
timer_settime01.c:164: FAIL: timer_gettime(CLOCK_BOOTTIME_ALARM) 
reported bad values (0: 570013221): SUCCESS (0)
timer_settime01.c:174: PASS: timer_settime(CLOCK_BOOTTIME_ALARM) passed
timer_settime01.c:164: FAIL: timer_gettime(CLOCK_REALTIME_ALARM) 
reported bad values (0: 250240851): SUCCESS (0)
timer_settime01.c:174: PASS: timer_settime(CLOCK_REALTIME_ALARM) passed
timer_settime01.c:174: PASS: timer_settime(CLOCK_TAI) passed
timer_settime01.c:100: INFO: Testing for setting old_value:
timer_settime01.c:174: PASS: timer_settime(CLOCK_REALTIME) passed
timer_settime01.c:174: PASS: timer_settime(CLOCK_MONOTONIC) passed
timer_settime01.c:174: PASS: timer_settime(CLOCK_PROCESS_CPUTIME_ID) passed
timer_settime01.c:174: PASS: timer_settime(CLOCK_THREAD_CPUTIME_ID) passed
timer_settime01.c:174: PASS: timer_settime(CLOCK_BOOTTIME) passed
timer_settime01.c:164: FAIL: timer_gettime(CLOCK_BOOTTIME_ALARM) 
reported bad values (0: 972013941): SUCCESS (0)
timer_settime01.c:174: PASS: timer_settime(CLOCK_BOOTTIME_ALARM) passed
timer_settime01.c:164: FAIL: timer_gettime(CLOCK_REALTIME_ALARM) 
reported bad values (0: 652241546): SUCCESS (0)
timer_settime01.c:174: PASS: timer_settime(CLOCK_REALTIME_ALARM) passed
timer_settime01.c:174: PASS: timer_settime(CLOCK_TAI) passed
timer_settime01.c:100: INFO: Testing for using periodic timer:
timer_settime01.c:174: PASS: timer_settime(CLOCK_REALTIME) passed
timer_settime01.c:174: PASS: timer_settime(CLOCK_MONOTONIC) passed
timer_settime01.c:174: PASS: timer_settime(CLOCK_PROCESS_CPUTIME_ID) passed
timer_settime01.c:174: PASS: timer_settime(CLOCK_THREAD_CPUTIME_ID) passed
timer_settime01.c:174: PASS: timer_settime(CLOCK_BOOTTIME) passed
timer_settime01.c:164: FAIL: timer_gettime(CLOCK_BOOTTIME_ALARM) 
reported bad values (50000000: 624015649): SUCCESS (0)
timer_settime01.c:174: PASS: timer_settime(CLOCK_BOOTTIME_ALARM) passed
timer_settime01.c:164: FAIL: timer_gettime(CLOCK_REALTIME_ALARM) 
reported bad values (50000000: 354248401): SUCCESS (0)
timer_settime01.c:174: PASS: timer_settime(CLOCK_REALTIME_ALARM) passed
timer_settime01.c:174: PASS: timer_settime(CLOCK_TAI) passed
timer_settime01.c:100: INFO: Testing for using absolute time:
timer_settime01.c:174: PASS: timer_settime(CLOCK_REALTIME) passed
timer_settime01.c:174: PASS: timer_settime(CLOCK_MONOTONIC) passed
timer_settime01.c:174: PASS: timer_settime(CLOCK_PROCESS_CPUTIME_ID) passed
timer_settime01.c:174: PASS: timer_settime(CLOCK_THREAD_CPUTIME_ID) passed
timer_settime01.c:174: PASS: timer_settime(CLOCK_BOOTTIME) passed
timer_settime01.c:164: FAIL: timer_gettime(CLOCK_BOOTTIME_ALARM) 
reported bad values (0: 176017000): SUCCESS (0)
timer_settime01.c:174: PASS: timer_settime(CLOCK_BOOTTIME_ALARM) passed
timer_settime01.c:164: FAIL: timer_gettime(CLOCK_REALTIME_ALARM) 
reported bad values (0: 856247000): SUCCESS (0)
timer_settime01.c:174: PASS: timer_settime(CLOCK_REALTIME_ALARM) passed
timer_settime01.c:174: PASS: timer_settime(CLOCK_TAI) passed

> 

[LTP] [PATCH V3] syscalls/timer_settime01: Make sure the timer fires

[Viresh Kumar]

On 24-07-20, 13:07, Yang Xu wrote:
> Yes, only _ALARM fails.

Right. So this seems this is rather a kernel issue. And I am not
really sure how to handle these.

Cyril, Any ideas ? Do we even try to support 3.10 here ?

-- 
viresh

[LTP] [PATCH V3] syscalls/timer_settime01: Make sure the timer fires

[Yang Xu]

Hi Viresh


> On 24-07-20, 13:07, Yang Xu wrote:
>> Yes, only _ALARM fails.
> 
> Right. So this seems this is rather a kernel issue. And I am not
> really sure how to handle these.
> 
> Cyril, Any ideas ? Do we even try to support 3.10 here ?

ltp supports kernel 3.10. minimal supported kernel and (g)libc version 
discussion is here[1].  I don't know this is a backport bug or a 
upstream kernel bug(has fixed). I will figure out this on next week.

[1]https://github.com/linux-test-project/ltp/issues/657
> 

[LTP] [PATCH V3] syscalls/timer_settime01: Make sure the timer fires

[Cyril Hrubis]

Hi!
> >> When tesing timer_settime01 on 3.10.0-1136.el7.x86_64, this case fails
> >> whether we use any type clock.
> >>
> >> timer_settime01.c:174: PASS: timer_settime(CLOCK_BOOTTIME) passed
> >> timer_settime01.c:164: FAIL: timer_gettime(CLOCK_BOOTTIME_ALARM) reported
> >> bad values (0: 678013000): SUCCESS (0)
> >> timer_settime01.c:174: PASS: timer_settime(CLOCK_BOOTTIME_ALARM) passed
> >> timer_settime01.c:164: FAIL: timer_gettime(CLOCK_REALTIME_ALARM) reported
> >> bad values (0: 358240000): SUCCESS (0)
> >> timer_settime01.c:174: PASS: timer_settime(CLOCK_REALTIME_ALARM) passed
> >> timer_settime01.c:174: PASS: timer_settime(CLOCK_TAI) passed
> > 
> > Can you share the complete test log? I am not sure if only the _ALARM
> > cocks are failing or all. You are getting values in the order of
> > 300-700 ms, while the max value can't be greater than 50 ms. So seems
> > like a kernel issue to me. Over that, both _ALARM type clocks weren't
> > supported before 3.11 and looks like your kernel version is 3.10.
> Yes, only _ALARM fails. I  only find a kernel patch (commit 
> 11ffa9d6065f344  timerfd: Add alarm timers) introduced alarm clock types 
> for timefd in kernel 3.11 and  a kernel patch (commit 9a7adcf5c6dea63d 
> timers: Posix interface for alarm-timers)  in kernel 3.1. It seems my 
> kernel version has supported this two alarm clock, but not sure why this 
> case fails.

This is on RHEL kernel that has backported the _ALARM support right? So
this may as well be case of badly bacported patch...

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH V3] syscalls/timer_settime01: Make sure the timer fires

[Yang Xu]

HI Cyril


> Hi!
>>>> When tesing timer_settime01 on 3.10.0-1136.el7.x86_64, this case fails
>>>> whether we use any type clock.
>>>>
>>>> timer_settime01.c:174: PASS: timer_settime(CLOCK_BOOTTIME) passed
>>>> timer_settime01.c:164: FAIL: timer_gettime(CLOCK_BOOTTIME_ALARM) reported
>>>> bad values (0: 678013000): SUCCESS (0)
>>>> timer_settime01.c:174: PASS: timer_settime(CLOCK_BOOTTIME_ALARM) passed
>>>> timer_settime01.c:164: FAIL: timer_gettime(CLOCK_REALTIME_ALARM) reported
>>>> bad values (0: 358240000): SUCCESS (0)
>>>> timer_settime01.c:174: PASS: timer_settime(CLOCK_REALTIME_ALARM) passed
>>>> timer_settime01.c:174: PASS: timer_settime(CLOCK_TAI) passed
>>>
>>> Can you share the complete test log? I am not sure if only the _ALARM
>>> cocks are failing or all. You are getting values in the order of
>>> 300-700 ms, while the max value can't be greater than 50 ms. So seems
>>> like a kernel issue to me. Over that, both _ALARM type clocks weren't
>>> supported before 3.11 and looks like your kernel version is 3.10.
>> Yes, only _ALARM fails. I  only find a kernel patch (commit
>> 11ffa9d6065f344  timerfd: Add alarm timers) introduced alarm clock types
>> for timefd in kernel 3.11 and  a kernel patch (commit 9a7adcf5c6dea63d
>> timers: Posix interface for alarm-timers)  in kernel 3.1. It seems my
>> kernel version has supported this two alarm clock, but not sure why this
>> case fails.
> 
> This is on RHEL kernel that has backported the _ALARM support right? So
> this may as well be case of badly bacported patch...
Just double check. My previous word may  make you feel confused.
On RHEL7 kernel, timer_* includes ALARM supports because kernel has 
upported it since 3.1 and timers_fd doesn't introduced ALARM 
support(REHL7 doesn't backport this patch). IMO, this case doesn't 
call/use timerfd_* function. We only need timer_*  ALARM suuport to run 
this case. so it doesn't need to backport ALARM support. Or, I miss some 
thing?
>