[v2,1/2] syscalls/clock_settime01.c: create syscall clock_settime

Message ID 20181206190710.22471-1-rafael.tinoco@linaro.org
State New
Headers show
Series
  • [v2,1/2] syscalls/clock_settime01.c: create syscall clock_settime
Related show

Commit Message

Rafael David Tinoco Dec. 6, 2018, 7:07 p.m.
Fixes: 343

clock_settime01 creates a new test, using new API, based on existing and
older kernel/timers/clock_settime tests. It includes tests from files
clock_settime02 and clock_settime03, which will be deleted in next
commits.

Signed-off-by: Rafael David Tinoco <rafael.tinoco@linaro.org>
---
 runtest/syscalls                              |   3 +
 .../kernel/syscalls/clock_settime/.gitignore  |   2 +
 .../kernel/syscalls/clock_settime/Makefile    |   8 +
 .../syscalls/clock_settime/clock_settime01.c  | 122 +++++++++++++
 .../syscalls/clock_settime/clock_settime02.c  | 171 ++++++++++++++++++
 5 files changed, 306 insertions(+)
 create mode 100644 testcases/kernel/syscalls/clock_settime/.gitignore
 create mode 100644 testcases/kernel/syscalls/clock_settime/Makefile
 create mode 100644 testcases/kernel/syscalls/clock_settime/clock_settime01.c
 create mode 100644 testcases/kernel/syscalls/clock_settime/clock_settime02.c

Comments

Rafael David Tinoco Dec. 6, 2018, 7:11 p.m. | #1
On 12/6/18 5:07 PM, Rafael David Tinoco wrote:
> Fixes: 343
> 
> clock_settime01 creates a new test, using new API, based on existing and
> older kernel/timers/clock_settime tests. It includes tests from files
> clock_settime02 and clock_settime03, which will be deleted in next
> commits.
> 
> Signed-off-by: Rafael David Tinoco <rafael.tinoco@linaro.org>
> ---

Cyril,

Please if you accept this patch, amend the description to:

"""
clock_settime01 creates a new test, using new API, based on existing
and older kernel/timers/clock_settime02 test. clock_settime02 creates
another test based on older kernel/timers/clock_settime03 test. Both
will be deleted in the next commits.

Signed-off-by: Rafael David Tinoco <rafael.tinoco@linaro.org>
"""

I forgot to change description before sending v2. Sorry.

About the first test, best way to check good results is:

$ date ; time sudo $(find . -name clock_settime01) -I 10 2>&1 >
/dev/null 2>&1 ; date

Thu Dec  6 17:09:24 -02 2018

real	0m10.032s
user	0m2.710s
sys	0m11.911s

Thu Dec  6 17:09:34 -02 2018

Using CLOCK_MONOTONIC_RAW we can make sure to restore a good time to
CLOCK_REALTIME after the test has run (independently of how many seconds
it took or iterations it had).
Cyril Hrubis Dec. 11, 2018, 2:27 p.m. | #2
Hi!
> new file mode 100644
> index 000000000..97d720fa2
> --- /dev/null
> +++ b/testcases/kernel/syscalls/clock_settime/clock_settime01.c
> @@ -0,0 +1,122 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2018 Linaro Limited. All rights reserved.
> + * Author: Rafael David Tinoco <rafael.tinoco@linaro.org>
> + */
> +
> +/*
> + * Basic test for clock_settime(2) on REALTIME clock:
> + *
> + *      1) advance DELTA_SEC seconds
> + *      2) go backwards DELTA_SEC seconds
> + *
> + * Accept DELTA_PER deviation on both (specially going backwards).
> + */
> +
> +#include "config.h"
> +#include "tst_test.h"
> +#include "lapi/syscalls.h"
> +
> +#define DELTA_SEC 10	/* 10 seconds delta    */
> +#define DELTA_PER 0.1	/* 1 percent deviation */
> +
> +static struct timespec real_begin, mono_begin, mono_end;
> +
> +static void clock_elapsed(struct timespec *begin, struct timespec *end,
> +		struct timespec *elapsed)
> +{
> +	elapsed->tv_sec = end->tv_sec - begin->tv_sec;
> +	elapsed->tv_nsec = end->tv_nsec - begin->tv_nsec;

If you do this the elapsed may end up in non-normalized state, i.e.
elapsed->tv_nsec may end up negative.

We do have all kinds of inline functions for conversion and arimetic in
tst_timer.h so there is no point of rolling your own here.

> +}
> +
> +static void clock_return(void)
                      ^
		      I would have named this restore, but that is very
		      minor.

> +{
> +	static struct timespec elapsed, adjust;
> +
> +	clock_elapsed(&mono_begin, &mono_end, &elapsed);
> +
> +	adjust.tv_sec = real_begin.tv_sec + elapsed.tv_sec;
> +	adjust.tv_nsec = real_begin.tv_nsec + elapsed.tv_nsec;

We should normalize the addition here as well, i.e. carry over to
seconds if the number of nanoseconds gets greater than 1s.

Ideally we should add a function to add two timespec structures into the
tst_timer.h header (in a separate patch).

> +	if (clock_settime(CLOCK_REALTIME, &adjust) != 0)
> +		tst_res(TBROK | TTERRNO, "could restore realtime clock");
> +}
> +
> +static void clock_fixnow(void)
> +{
> +	if (clock_gettime(CLOCK_MONOTONIC_RAW, &mono_end) != 0)
> +		tst_res(TBROK | TTERRNO, "could not get elapsed time");

We should make sure here that both real_begin and mono_begin were set
before we attempt to restore the system time. A restore_time flag se to
1 after we successfully read both will do.

> +	clock_return();
> +}
> +
> +static void setup(void)
> +{
> +	/* save initial monotonic time to restore it when needed */
> +
> +	if (clock_gettime(CLOCK_REALTIME, &real_begin) != 0)
> +		tst_res(TBROK | TTERRNO, "could not get initial real time");
> +
> +	if (clock_gettime(CLOCK_MONOTONIC_RAW, &mono_begin) != 0)
> +		tst_res(TBROK | TTERRNO, "couldn't get initial monotonic time");
                  ^
		  This should have been tst_brk() doing tst_res() with
		  TBROK does not make much sense.

Also we do have tst_safe_clocks.h, if you include that you can use
SAFE_CLOCK_GETTIME() instead.

Overall the idea of restoring wall clock using monotonic timer is a good
one, maybe we should even move this code to a library so that all tests
that change wall clock would need just set restore_wallclock flag in the
tst_test structure...

> +}
> +
> +static void cleanup(void)
> +{
> +	clock_fixnow();
> +}
> +
> +static void verify_clock_settime(void)
> +{
> +	static struct timespec begin, change, end, elapsed;
> +
> +	/* test 01: move forward */
> +
> +	if (clock_gettime(CLOCK_REALTIME, &begin) != 0)
> +		tst_res(TBROK | TTERRNO, "could not get realtime at the begin");
> +
> +	change.tv_sec = begin.tv_sec + DELTA_SEC;
> +
> +	if (clock_settime(CLOCK_REALTIME, &change) != 0)
> +		tst_res(TBROK | TTERRNO, "could not set realtime change");
> +
> +	if (clock_gettime(CLOCK_REALTIME, &end) != 0)
> +		tst_res(TBROK | TTERRNO, "could not get realtime after change");

Here as well.

> +	clock_elapsed(&begin, &end, &elapsed);
> +
> +	if (elapsed.tv_sec < (float) (DELTA_SEC - (DELTA_SEC * DELTA_PER)))

It would be much easier to just use tst_timespec_diff_ms() and check
that the result is in reasonable range. I.e. check lower bound as well.

> +		tst_res(TFAIL, "clock_settime(2): could not advance time");
> +	else
> +		tst_res(TPASS, "clock_settime(2): was able to advance time");
> +
> +	/* test 02: move backward */
> +
> +	if (clock_gettime(CLOCK_REALTIME, &begin) != 0)
> +		tst_res(TBROK | TTERRNO, "could not get realtime at the begin");
> +
> +	change.tv_sec = begin.tv_sec - DELTA_SEC;
> +
> +	if (clock_settime(CLOCK_REALTIME, &change) != 0)
> +		tst_res(TBROK | TTERRNO, "could not set realtime change");
> +
> +	if (clock_gettime(CLOCK_REALTIME, &end) != 0)
> +		tst_res(TBROK | TTERRNO, "could not get realtime after change");
> +
> +	clock_elapsed(&begin, &end, &elapsed);
> +
> +	elapsed.tv_sec = ~elapsed.tv_sec;
> +	elapsed.tv_nsec = ~elapsed.tv_nsec;
> +
> +	if (elapsed.tv_sec < (float) (DELTA_SEC - (DELTA_SEC * DELTA_PER)))
> +		tst_res(TFAIL, "clock_settime(2): could not recede time");
> +	else
> +		tst_res(TPASS, "clock_settime(2): was able to recede time");

Here as well.

> +}
> +
> +static struct tst_test test = {
> +	.setup = setup,
> +	.test_all = verify_clock_settime,
> +	.cleanup = cleanup,
> +	.needs_root = 1,
> +};
>
> diff --git a/testcases/kernel/syscalls/clock_settime/clock_settime02.c b/testcases/kernel/syscalls/clock_settime/clock_settime02.c
> new file mode 100644
> index 000000000..710f37219
> --- /dev/null
> +++ b/testcases/kernel/syscalls/clock_settime/clock_settime02.c
> @@ -0,0 +1,171 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2018 Linaro Limited. All rights reserved.
> + * Author: Rafael David Tinoco <rafael.tinoco@linaro.org>
> + */
> +
> +/*
> + * Basic tests for errors of clock_settime(2) on different clock types.
> + */
> +
> +#include "config.h"
> +#include "tst_test.h"
> +#include "lapi/syscalls.h"
> +
> +#define DELTA_SEC 10
> +#define NSEC_PER_SEC (1000000000L)
> +#define MAX_CLOCKS 16
> +
> +static struct timespec clock_realtime_saved;
> +
> +struct test_case {
> +	clockid_t type;
> +	struct timespec newtime;
> +	int exp_err;
> +	int replace;
> +};
> +
> +struct test_case tc[] = {
> +	{				/* case 01: REALTIME: timespec NULL   */
> +	 .type = CLOCK_REALTIME,
> +	 .newtime.tv_sec = -2,
> +	 .exp_err = EFAULT,
> +	 .replace = 1,
> +	 },
> +	{				/* case 02: REALTIME: tv_sec = -1     */
> +	 .type = CLOCK_REALTIME,
> +	 .newtime.tv_sec = -1,
> +	 .exp_err = EINVAL,
> +	 .replace = 1,
> +	 },
> +	{				/* case 03: REALTIME: tv_nsec = -1    */
> +	 .type = CLOCK_REALTIME,
> +	 .newtime.tv_nsec = -1,
> +	 .exp_err = EINVAL,
> +	 .replace = 1,
> +	 },
> +	{				/* case 04: REALTIME: tv_nsec = 1s+1  */
> +	 .type = CLOCK_REALTIME,
> +	 .newtime.tv_nsec = NSEC_PER_SEC + 1,
> +	 .exp_err = EINVAL,
> +	 .replace = 1,
> +	 },
> +	{				/* case 05: MONOTONIC		      */
> +	 .type = CLOCK_MONOTONIC,
> +	 .exp_err = EINVAL,
> +	 },
> +	{				/* case 06: MAXCLOCK		      */
> +	 .type = MAX_CLOCKS,
> +	 .exp_err = EINVAL,
> +	 },
> +	{				/* case 07: MAXCLOCK+1		      */
> +	 .type = MAX_CLOCKS + 1,
> +	 .exp_err = EINVAL,
> +	 },
> +	/* Linux specific */
> +	{				/* case 08: CLOCK_MONOTONIC_COARSE    */
> +	 .type = CLOCK_MONOTONIC_COARSE,
> +	 .exp_err = EINVAL,
> +	 },
> +	{				/* case 09: CLOCK_MONOTONIC_RAW       */
> +	 .type = CLOCK_MONOTONIC_RAW,
> +	 .exp_err = EINVAL,
> +	 },
> +	{				/* case 10: CLOCK_BOOTTIME	      */
> +	 .type = CLOCK_BOOTTIME,
> +	 .exp_err = EINVAL,
> +	 },
> +	{				/* case 11: CLOCK_PROCESS_CPUTIME_ID  */
> +	 .type = CLOCK_PROCESS_CPUTIME_ID,
> +	 .exp_err = EINVAL,
> +	 },
> +	{				/* case 12: CLOCK_THREAD_CPUTIME_ID   */
> +	 .type = CLOCK_THREAD_CPUTIME_ID,
> +	 .exp_err = EINVAL,
> +	 },
> +};
> +
> +/*
> + * Some tests may cause libc to segfault when passing bad arguments.
> + */
> +static int sys_clock_settime(clockid_t clk_id, struct timespec *tp)
> +{
> +	return tst_syscall(__NR_clock_settime, clk_id, tp);
> +}
> +
> +static int sys_clock_gettime(clockid_t clk_id, struct timespec *tp)
> +{
> +	return tst_syscall(__NR_clock_gettime, clk_id, tp);
> +}
> +
> +static void cleanup(void)
> +{
> +	/* restore realtime clock */
> +
> +	if (sys_clock_settime(CLOCK_REALTIME, &clock_realtime_saved) < 0)
> +		tst_res(TBROK | TTERRNO, "clock_settime(2): could not set "
> +				"current time back");
> +}
> +
> +static void setup(void)
> +{
> +	if (sys_clock_gettime(CLOCK_REALTIME, &clock_realtime_saved) < 0)
> +		tst_res(TBROK | TTERRNO, "clock_gettime(2): could not get "
> +				"current time");
> +}
> +
> +static void verify_clock_settime(unsigned int i)
> +{
> +	struct timespec spec, *specptr;
> +
> +	if (tc[i].replace == 0) {
> +
> +		/* add 1 sec to test clock */
> +
> +		specptr = &spec;
> +		specptr->tv_sec = clock_realtime_saved.tv_sec + 1;
> +		specptr->tv_nsec = clock_realtime_saved.tv_nsec;
> +
> +	} else {
> +
> +		/* bad pointer case */
> +
> +		if (tc[i].newtime.tv_sec == -2)
> +			specptr = tst_get_bad_addr(cleanup);
> +
> +		/* use given values */
> +
> +		else {

Still curly braces missing here, but that is minor.

And maybe we just need to turn the newtime into a pointer in the tcases
structure. Then you can initialize global variable to current time +
epsion and use pointer to it or even initialize it inline as:

static struct timespec valid_time;

struct test_case tc[] = {
	{
	 .type = CLOCK_REALTIME,
	 .exp_err = EFAULT,
	},
	{
	 .type = CLOCK_REALTIME;
	 .newtime = &valid_time;
	 .exp_err = EINVAL,
	{
	 .type = CLOCK_REALTIME,
	 .newtime = &(struct timespec){},
	 .exp_err = EINVAL,
	 .replace = 1,
	},
	...
};

And we can loop in the test setup and set NULL address to the result of
tst_get_bad_addr() as well.

> +			specptr = &spec;
> +			specptr->tv_sec = tc[i].newtime.tv_sec;
> +			specptr->tv_nsec = tc[i].newtime.tv_nsec;
> +		}
> +	}
> +
> +	TEST(sys_clock_settime(tc[i].type, specptr));
> +
> +	if (TST_RET == -1) {
> +
> +		if (tc[i].exp_err == TST_ERR) {
> +
> +			tst_res(TPASS, "clock_settime(2): failed as expected");
> +
> +		} else {
> +			tst_res(TFAIL | TTERRNO, "clock_settime(2): "
> +					"failed with different error");
> +		}
> +
> +		return;
> +	}
> +
> +	tst_res(TFAIL | TTERRNO, "clock_settime(2): clock type %d failed",
> +			tc[i].type);
> +}
> +
> +static struct tst_test test = {
> +	.setup = setup,
> +	.test = verify_clock_settime,
> +	.cleanup = cleanup,
> +	.tcnt = ARRAY_SIZE(tc),
> +	.needs_root = 1,
> +};
> -- 
> 2.20.0.rc1
>
Rafael David Tinoco Dec. 11, 2018, 4:05 p.m. | #3
On 12/11/18 12:27 PM, Cyril Hrubis wrote:
> Hi!
>> new file mode 100644
>> index 000000000..97d720fa2
>> --- /dev/null
>> +++ b/testcases/kernel/syscalls/clock_settime/clock_settime01.c
>> @@ -0,0 +1,122 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (c) 2018 Linaro Limited. All rights reserved.
>> + * Author: Rafael David Tinoco <rafael.tinoco@linaro.org>
>> + */
>> +
>> +/*
>> + * Basic test for clock_settime(2) on REALTIME clock:
>> + *
>> + *      1) advance DELTA_SEC seconds
>> + *      2) go backwards DELTA_SEC seconds
>> + *
>> + * Accept DELTA_PER deviation on both (specially going backwards).
>> + */
>> +
>> +#include "config.h"
>> +#include "tst_test.h"
>> +#include "lapi/syscalls.h"
>> +
>> +#define DELTA_SEC 10	/* 10 seconds delta    */
>> +#define DELTA_PER 0.1	/* 1 percent deviation */
>> +
>> +static struct timespec real_begin, mono_begin, mono_end;
>> +
>> +static void clock_elapsed(struct timespec *begin, struct timespec *end,
>> +		struct timespec *elapsed)
>> +{
>> +	elapsed->tv_sec = end->tv_sec - begin->tv_sec;
>> +	elapsed->tv_nsec = end->tv_nsec - begin->tv_nsec;
> 
> If you do this the elapsed may end up in non-normalized state, i.e.
> elapsed->tv_nsec may end up negative.

#)... definitely, will stick to what tst_timer.h has.

> 
> We do have all kinds of inline functions for conversion and arimetic in
> tst_timer.h so there is no point of rolling your own here.
> 
>> +}
>> +
>> +static void clock_return(void)
>                       ^
> 		      I would have named this restore, but that is very
> 		      minor.
> 
>> +{
>> +	static struct timespec elapsed, adjust;
>> +
>> +	clock_elapsed(&mono_begin, &mono_end, &elapsed);
>> +
>> +	adjust.tv_sec = real_begin.tv_sec + elapsed.tv_sec;
>> +	adjust.tv_nsec = real_begin.tv_nsec + elapsed.tv_nsec;
> 
> We should normalize the addition here as well, i.e. carry over to
> seconds if the number of nanoseconds gets greater than 1s.

definitely, stupid mistake on my side, will fix.

> 
> Ideally we should add a function to add two timespec structures into the
> tst_timer.h header (in a separate patch).

Alright.

> 
>> +	if (clock_settime(CLOCK_REALTIME, &adjust) != 0)
>> +		tst_res(TBROK | TTERRNO, "could restore realtime clock");
>> +}
>> +
>> +static void clock_fixnow(void)
>> +{
>> +	if (clock_gettime(CLOCK_MONOTONIC_RAW, &mono_end) != 0)
>> +		tst_res(TBROK | TTERRNO, "could not get elapsed time");
> 
> We should make sure here that both real_begin and mono_begin were set
> before we attempt to restore the system time. A restore_time flag se to
> 1 after we successfully read both will do.

Alright.

> 
>> +	clock_return();
>> +}
>> +
>> +static void setup(void)
>> +{
>> +	/* save initial monotonic time to restore it when needed */
>> +
>> +	if (clock_gettime(CLOCK_REALTIME, &real_begin) != 0)
>> +		tst_res(TBROK | TTERRNO, "could not get initial real time");
>> +
>> +	if (clock_gettime(CLOCK_MONOTONIC_RAW, &mono_begin) != 0)
>> +		tst_res(TBROK | TTERRNO, "couldn't get initial monotonic time");
>                   ^
> 		  This should have been tst_brk() doing tst_res() with
> 		  TBROK does not make much sense.
> 
> Also we do have tst_safe_clocks.h, if you include that you can use
> SAFE_CLOCK_GETTIME() instead.
> 
> Overall the idea of restoring wall clock using monotonic timer is a good
> one, maybe we should even move this code to a library so that all tests
> that change wall clock would need just set restore_wallclock flag in the
> tst_test structure...

Will look into that.

> 
>> +}
>> +
>> +static void cleanup(void)
>> +{
>> +	clock_fixnow();
>> +}
>> +
>> +static void verify_clock_settime(void)
>> +{
>> +	static struct timespec begin, change, end, elapsed;
>> +
>> +	/* test 01: move forward */
>> +
>> +	if (clock_gettime(CLOCK_REALTIME, &begin) != 0)
>> +		tst_res(TBROK | TTERRNO, "could not get realtime at the begin");
>> +
>> +	change.tv_sec = begin.tv_sec + DELTA_SEC;
>> +
>> +	if (clock_settime(CLOCK_REALTIME, &change) != 0)
>> +		tst_res(TBROK | TTERRNO, "could not set realtime change");
>> +
>> +	if (clock_gettime(CLOCK_REALTIME, &end) != 0)
>> +		tst_res(TBROK | TTERRNO, "could not get realtime after change");
> 
> Here as well.
> 
>> +	clock_elapsed(&begin, &end, &elapsed);
>> +
>> +	if (elapsed.tv_sec < (float) (DELTA_SEC - (DELTA_SEC * DELTA_PER)))
> 
> It would be much easier to just use tst_timespec_diff_ms() and check
> that the result is in reasonable range. I.e. check lower bound as well.

Yep, I missed tst_timer.h entirely it seems =o), won't reinvent the wheel.

> 
>> +		tst_res(TFAIL, "clock_settime(2): could not advance time");
>> +	else
>> +		tst_res(TPASS, "clock_settime(2): was able to advance time");
>> +
>> +	/* test 02: move backward */
>> +
>> +	if (clock_gettime(CLOCK_REALTIME, &begin) != 0)
>> +		tst_res(TBROK | TTERRNO, "could not get realtime at the begin");
>> +
>> +	change.tv_sec = begin.tv_sec - DELTA_SEC;
>> +
>> +	if (clock_settime(CLOCK_REALTIME, &change) != 0)
>> +		tst_res(TBROK | TTERRNO, "could not set realtime change");
>> +
>> +	if (clock_gettime(CLOCK_REALTIME, &end) != 0)
>> +		tst_res(TBROK | TTERRNO, "could not get realtime after change");
>> +
>> +	clock_elapsed(&begin, &end, &elapsed);
>> +
>> +	elapsed.tv_sec = ~elapsed.tv_sec;
>> +	elapsed.tv_nsec = ~elapsed.tv_nsec;
>> +
>> +	if (elapsed.tv_sec < (float) (DELTA_SEC - (DELTA_SEC * DELTA_PER)))
>> +		tst_res(TFAIL, "clock_settime(2): could not recede time");
>> +	else
>> +		tst_res(TPASS, "clock_settime(2): was able to recede time");
> 
> Here as well.
> 
>> +}
>> +
>> +static struct tst_test test = {
>> +	.setup = setup,
>> +	.test_all = verify_clock_settime,
>> +	.cleanup = cleanup,
>> +	.needs_root = 1,
>> +};
>>
>> diff --git a/testcases/kernel/syscalls/clock_settime/clock_settime02.c b/testcases/kernel/syscalls/clock_settime/clock_settime02.c
>> new file mode 100644
>> index 000000000..710f37219
>> --- /dev/null
>> +++ b/testcases/kernel/syscalls/clock_settime/clock_settime02.c
>> @@ -0,0 +1,171 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (c) 2018 Linaro Limited. All rights reserved.
>> + * Author: Rafael David Tinoco <rafael.tinoco@linaro.org>
>> + */
>> +
>> +/*
>> + * Basic tests for errors of clock_settime(2) on different clock types.
>> + */
>> +
>> +#include "config.h"
>> +#include "tst_test.h"
>> +#include "lapi/syscalls.h"
>> +
>> +#define DELTA_SEC 10
>> +#define NSEC_PER_SEC (1000000000L)
>> +#define MAX_CLOCKS 16
>> +
>> +static struct timespec clock_realtime_saved;
>> +
>> +struct test_case {
>> +	clockid_t type;
>> +	struct timespec newtime;
>> +	int exp_err;
>> +	int replace;
>> +};
>> +
>> +struct test_case tc[] = {
>> +	{				/* case 01: REALTIME: timespec NULL   */
>> +	 .type = CLOCK_REALTIME,
>> +	 .newtime.tv_sec = -2,
>> +	 .exp_err = EFAULT,
>> +	 .replace = 1,
>> +	 },
>> +	{				/* case 02: REALTIME: tv_sec = -1     */
>> +	 .type = CLOCK_REALTIME,
>> +	 .newtime.tv_sec = -1,
>> +	 .exp_err = EINVAL,
>> +	 .replace = 1,
>> +	 },
>> +	{				/* case 03: REALTIME: tv_nsec = -1    */
>> +	 .type = CLOCK_REALTIME,
>> +	 .newtime.tv_nsec = -1,
>> +	 .exp_err = EINVAL,
>> +	 .replace = 1,
>> +	 },
>> +	{				/* case 04: REALTIME: tv_nsec = 1s+1  */
>> +	 .type = CLOCK_REALTIME,
>> +	 .newtime.tv_nsec = NSEC_PER_SEC + 1,
>> +	 .exp_err = EINVAL,
>> +	 .replace = 1,
>> +	 },
>> +	{				/* case 05: MONOTONIC		      */
>> +	 .type = CLOCK_MONOTONIC,
>> +	 .exp_err = EINVAL,
>> +	 },
>> +	{				/* case 06: MAXCLOCK		      */
>> +	 .type = MAX_CLOCKS,
>> +	 .exp_err = EINVAL,
>> +	 },
>> +	{				/* case 07: MAXCLOCK+1		      */
>> +	 .type = MAX_CLOCKS + 1,
>> +	 .exp_err = EINVAL,
>> +	 },
>> +	/* Linux specific */
>> +	{				/* case 08: CLOCK_MONOTONIC_COARSE    */
>> +	 .type = CLOCK_MONOTONIC_COARSE,
>> +	 .exp_err = EINVAL,
>> +	 },
>> +	{				/* case 09: CLOCK_MONOTONIC_RAW       */
>> +	 .type = CLOCK_MONOTONIC_RAW,
>> +	 .exp_err = EINVAL,
>> +	 },
>> +	{				/* case 10: CLOCK_BOOTTIME	      */
>> +	 .type = CLOCK_BOOTTIME,
>> +	 .exp_err = EINVAL,
>> +	 },
>> +	{				/* case 11: CLOCK_PROCESS_CPUTIME_ID  */
>> +	 .type = CLOCK_PROCESS_CPUTIME_ID,
>> +	 .exp_err = EINVAL,
>> +	 },
>> +	{				/* case 12: CLOCK_THREAD_CPUTIME_ID   */
>> +	 .type = CLOCK_THREAD_CPUTIME_ID,
>> +	 .exp_err = EINVAL,
>> +	 },
>> +};
>> +
>> +/*
>> + * Some tests may cause libc to segfault when passing bad arguments.
>> + */
>> +static int sys_clock_settime(clockid_t clk_id, struct timespec *tp)
>> +{
>> +	return tst_syscall(__NR_clock_settime, clk_id, tp);
>> +}
>> +
>> +static int sys_clock_gettime(clockid_t clk_id, struct timespec *tp)
>> +{
>> +	return tst_syscall(__NR_clock_gettime, clk_id, tp);
>> +}
>> +
>> +static void cleanup(void)
>> +{
>> +	/* restore realtime clock */
>> +
>> +	if (sys_clock_settime(CLOCK_REALTIME, &clock_realtime_saved) < 0)
>> +		tst_res(TBROK | TTERRNO, "clock_settime(2): could not set "
>> +				"current time back");
>> +}
>> +
>> +static void setup(void)
>> +{
>> +	if (sys_clock_gettime(CLOCK_REALTIME, &clock_realtime_saved) < 0)
>> +		tst_res(TBROK | TTERRNO, "clock_gettime(2): could not get "
>> +				"current time");
>> +}
>> +
>> +static void verify_clock_settime(unsigned int i)
>> +{
>> +	struct timespec spec, *specptr;
>> +
>> +	if (tc[i].replace == 0) {
>> +
>> +		/* add 1 sec to test clock */
>> +
>> +		specptr = &spec;
>> +		specptr->tv_sec = clock_realtime_saved.tv_sec + 1;
>> +		specptr->tv_nsec = clock_realtime_saved.tv_nsec;
>> +
>> +	} else {
>> +
>> +		/* bad pointer case */
>> +
>> +		if (tc[i].newtime.tv_sec == -2)
>> +			specptr = tst_get_bad_addr(cleanup);
>> +
>> +		/* use given values */
>> +
>> +		else {
> 
> Still curly braces missing here, but that is minor.
> 
> And maybe we just need to turn the newtime into a pointer in the tcases
> structure. Then you can initialize global variable to current time +
> epsion and use pointer to it or even initialize it inline as:
> 
> static struct timespec valid_time;
> 
> struct test_case tc[] = {
> 	{
> 	 .type = CLOCK_REALTIME,
> 	 .exp_err = EFAULT,
> 	},
> 	{
> 	 .type = CLOCK_REALTIME;
> 	 .newtime = &valid_time;
> 	 .exp_err = EINVAL,
> 	{
> 	 .type = CLOCK_REALTIME,
> 	 .newtime = &(struct timespec){},
> 	 .exp_err = EINVAL,
> 	 .replace = 1,
> 	},
> 	...
> };
> 
> And we can loop in the test setup and set NULL address to the result of
> tst_get_bad_addr() as well.

Good idea.

> 
>> +			specptr = &spec;
>> +			specptr->tv_sec = tc[i].newtime.tv_sec;
>> +			specptr->tv_nsec = tc[i].newtime.tv_nsec;
>> +		}
>> +	}
>> +
>> +	TEST(sys_clock_settime(tc[i].type, specptr));
>> +
>> +	if (TST_RET == -1) {
>> +
>> +		if (tc[i].exp_err == TST_ERR) {
>> +
>> +			tst_res(TPASS, "clock_settime(2): failed as expected");
>> +
>> +		} else {
>> +			tst_res(TFAIL | TTERRNO, "clock_settime(2): "
>> +					"failed with different error");
>> +		}
>> +
>> +		return;
>> +	}
>> +
>> +	tst_res(TFAIL | TTERRNO, "clock_settime(2): clock type %d failed",
>> +			tc[i].type);
>> +}
>> +
>> +static struct tst_test test = {
>> +	.setup = setup,
>> +	.test = verify_clock_settime,
>> +	.cleanup = cleanup,
>> +	.tcnt = ARRAY_SIZE(tc),
>> +	.needs_root = 1,
>> +};
>> -- 
>> 2.20.0.rc1
>>
> 

Thanks for reviewing... will re-work it.

Patch

diff --git a/runtest/syscalls b/runtest/syscalls
index ac1d2d2cd..2e38ab37e 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -79,6 +79,9 @@  clock_nanosleep01 clock_nanosleep01
 clock_nanosleep02 clock_nanosleep02
 clock_nanosleep2_01 clock_nanosleep2_01
 
+clock_settime01 clock_settime01
+clock_settime02 clock_settime02
+
 clone01 clone01
 clone02 clone02
 clone03 clone03
diff --git a/testcases/kernel/syscalls/clock_settime/.gitignore b/testcases/kernel/syscalls/clock_settime/.gitignore
new file mode 100644
index 000000000..281217550
--- /dev/null
+++ b/testcases/kernel/syscalls/clock_settime/.gitignore
@@ -0,0 +1,2 @@ 
+clock_settime01
+clock_settime02
diff --git a/testcases/kernel/syscalls/clock_settime/Makefile b/testcases/kernel/syscalls/clock_settime/Makefile
new file mode 100644
index 000000000..e6674a6b2
--- /dev/null
+++ b/testcases/kernel/syscalls/clock_settime/Makefile
@@ -0,0 +1,8 @@ 
+# Copyright (c) 2018 - Linaro Limited. All rights reserved.
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/clock_settime/clock_settime01.c b/testcases/kernel/syscalls/clock_settime/clock_settime01.c
new file mode 100644
index 000000000..97d720fa2
--- /dev/null
+++ b/testcases/kernel/syscalls/clock_settime/clock_settime01.c
@@ -0,0 +1,122 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2018 Linaro Limited. All rights reserved.
+ * Author: Rafael David Tinoco <rafael.tinoco@linaro.org>
+ */
+
+/*
+ * Basic test for clock_settime(2) on REALTIME clock:
+ *
+ *      1) advance DELTA_SEC seconds
+ *      2) go backwards DELTA_SEC seconds
+ *
+ * Accept DELTA_PER deviation on both (specially going backwards).
+ */
+
+#include "config.h"
+#include "tst_test.h"
+#include "lapi/syscalls.h"
+
+#define DELTA_SEC 10	/* 10 seconds delta    */
+#define DELTA_PER 0.1	/* 1 percent deviation */
+
+static struct timespec real_begin, mono_begin, mono_end;
+
+static void clock_elapsed(struct timespec *begin, struct timespec *end,
+		struct timespec *elapsed)
+{
+	elapsed->tv_sec = end->tv_sec - begin->tv_sec;
+	elapsed->tv_nsec = end->tv_nsec - begin->tv_nsec;
+}
+
+static void clock_return(void)
+{
+	static struct timespec elapsed, adjust;
+
+	clock_elapsed(&mono_begin, &mono_end, &elapsed);
+
+	adjust.tv_sec = real_begin.tv_sec + elapsed.tv_sec;
+	adjust.tv_nsec = real_begin.tv_nsec + elapsed.tv_nsec;
+
+	if (clock_settime(CLOCK_REALTIME, &adjust) != 0)
+		tst_res(TBROK | TTERRNO, "could restore realtime clock");
+}
+
+static void clock_fixnow(void)
+{
+	if (clock_gettime(CLOCK_MONOTONIC_RAW, &mono_end) != 0)
+		tst_res(TBROK | TTERRNO, "could not get elapsed time");
+
+	clock_return();
+}
+
+static void setup(void)
+{
+	/* save initial monotonic time to restore it when needed */
+
+	if (clock_gettime(CLOCK_REALTIME, &real_begin) != 0)
+		tst_res(TBROK | TTERRNO, "could not get initial real time");
+
+	if (clock_gettime(CLOCK_MONOTONIC_RAW, &mono_begin) != 0)
+		tst_res(TBROK | TTERRNO, "couldn't get initial monotonic time");
+}
+
+static void cleanup(void)
+{
+	clock_fixnow();
+}
+
+static void verify_clock_settime(void)
+{
+	static struct timespec begin, change, end, elapsed;
+
+	/* test 01: move forward */
+
+	if (clock_gettime(CLOCK_REALTIME, &begin) != 0)
+		tst_res(TBROK | TTERRNO, "could not get realtime at the begin");
+
+	change.tv_sec = begin.tv_sec + DELTA_SEC;
+
+	if (clock_settime(CLOCK_REALTIME, &change) != 0)
+		tst_res(TBROK | TTERRNO, "could not set realtime change");
+
+	if (clock_gettime(CLOCK_REALTIME, &end) != 0)
+		tst_res(TBROK | TTERRNO, "could not get realtime after change");
+
+	clock_elapsed(&begin, &end, &elapsed);
+
+	if (elapsed.tv_sec < (float) (DELTA_SEC - (DELTA_SEC * DELTA_PER)))
+		tst_res(TFAIL, "clock_settime(2): could not advance time");
+	else
+		tst_res(TPASS, "clock_settime(2): was able to advance time");
+
+	/* test 02: move backward */
+
+	if (clock_gettime(CLOCK_REALTIME, &begin) != 0)
+		tst_res(TBROK | TTERRNO, "could not get realtime at the begin");
+
+	change.tv_sec = begin.tv_sec - DELTA_SEC;
+
+	if (clock_settime(CLOCK_REALTIME, &change) != 0)
+		tst_res(TBROK | TTERRNO, "could not set realtime change");
+
+	if (clock_gettime(CLOCK_REALTIME, &end) != 0)
+		tst_res(TBROK | TTERRNO, "could not get realtime after change");
+
+	clock_elapsed(&begin, &end, &elapsed);
+
+	elapsed.tv_sec = ~elapsed.tv_sec;
+	elapsed.tv_nsec = ~elapsed.tv_nsec;
+
+	if (elapsed.tv_sec < (float) (DELTA_SEC - (DELTA_SEC * DELTA_PER)))
+		tst_res(TFAIL, "clock_settime(2): could not recede time");
+	else
+		tst_res(TPASS, "clock_settime(2): was able to recede time");
+}
+
+static struct tst_test test = {
+	.setup = setup,
+	.test_all = verify_clock_settime,
+	.cleanup = cleanup,
+	.needs_root = 1,
+};
diff --git a/testcases/kernel/syscalls/clock_settime/clock_settime02.c b/testcases/kernel/syscalls/clock_settime/clock_settime02.c
new file mode 100644
index 000000000..710f37219
--- /dev/null
+++ b/testcases/kernel/syscalls/clock_settime/clock_settime02.c
@@ -0,0 +1,171 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2018 Linaro Limited. All rights reserved.
+ * Author: Rafael David Tinoco <rafael.tinoco@linaro.org>
+ */
+
+/*
+ * Basic tests for errors of clock_settime(2) on different clock types.
+ */
+
+#include "config.h"
+#include "tst_test.h"
+#include "lapi/syscalls.h"
+
+#define DELTA_SEC 10
+#define NSEC_PER_SEC (1000000000L)
+#define MAX_CLOCKS 16
+
+static struct timespec clock_realtime_saved;
+
+struct test_case {
+	clockid_t type;
+	struct timespec newtime;
+	int exp_err;
+	int replace;
+};
+
+struct test_case tc[] = {
+	{				/* case 01: REALTIME: timespec NULL   */
+	 .type = CLOCK_REALTIME,
+	 .newtime.tv_sec = -2,
+	 .exp_err = EFAULT,
+	 .replace = 1,
+	 },
+	{				/* case 02: REALTIME: tv_sec = -1     */
+	 .type = CLOCK_REALTIME,
+	 .newtime.tv_sec = -1,
+	 .exp_err = EINVAL,
+	 .replace = 1,
+	 },
+	{				/* case 03: REALTIME: tv_nsec = -1    */
+	 .type = CLOCK_REALTIME,
+	 .newtime.tv_nsec = -1,
+	 .exp_err = EINVAL,
+	 .replace = 1,
+	 },
+	{				/* case 04: REALTIME: tv_nsec = 1s+1  */
+	 .type = CLOCK_REALTIME,
+	 .newtime.tv_nsec = NSEC_PER_SEC + 1,
+	 .exp_err = EINVAL,
+	 .replace = 1,
+	 },
+	{				/* case 05: MONOTONIC		      */
+	 .type = CLOCK_MONOTONIC,
+	 .exp_err = EINVAL,
+	 },
+	{				/* case 06: MAXCLOCK		      */
+	 .type = MAX_CLOCKS,
+	 .exp_err = EINVAL,
+	 },
+	{				/* case 07: MAXCLOCK+1		      */
+	 .type = MAX_CLOCKS + 1,
+	 .exp_err = EINVAL,
+	 },
+	/* Linux specific */
+	{				/* case 08: CLOCK_MONOTONIC_COARSE    */
+	 .type = CLOCK_MONOTONIC_COARSE,
+	 .exp_err = EINVAL,
+	 },
+	{				/* case 09: CLOCK_MONOTONIC_RAW       */
+	 .type = CLOCK_MONOTONIC_RAW,
+	 .exp_err = EINVAL,
+	 },
+	{				/* case 10: CLOCK_BOOTTIME	      */
+	 .type = CLOCK_BOOTTIME,
+	 .exp_err = EINVAL,
+	 },
+	{				/* case 11: CLOCK_PROCESS_CPUTIME_ID  */
+	 .type = CLOCK_PROCESS_CPUTIME_ID,
+	 .exp_err = EINVAL,
+	 },
+	{				/* case 12: CLOCK_THREAD_CPUTIME_ID   */
+	 .type = CLOCK_THREAD_CPUTIME_ID,
+	 .exp_err = EINVAL,
+	 },
+};
+
+/*
+ * Some tests may cause libc to segfault when passing bad arguments.
+ */
+static int sys_clock_settime(clockid_t clk_id, struct timespec *tp)
+{
+	return tst_syscall(__NR_clock_settime, clk_id, tp);
+}
+
+static int sys_clock_gettime(clockid_t clk_id, struct timespec *tp)
+{
+	return tst_syscall(__NR_clock_gettime, clk_id, tp);
+}
+
+static void cleanup(void)
+{
+	/* restore realtime clock */
+
+	if (sys_clock_settime(CLOCK_REALTIME, &clock_realtime_saved) < 0)
+		tst_res(TBROK | TTERRNO, "clock_settime(2): could not set "
+				"current time back");
+}
+
+static void setup(void)
+{
+	if (sys_clock_gettime(CLOCK_REALTIME, &clock_realtime_saved) < 0)
+		tst_res(TBROK | TTERRNO, "clock_gettime(2): could not get "
+				"current time");
+}
+
+static void verify_clock_settime(unsigned int i)
+{
+	struct timespec spec, *specptr;
+
+	if (tc[i].replace == 0) {
+
+		/* add 1 sec to test clock */
+
+		specptr = &spec;
+		specptr->tv_sec = clock_realtime_saved.tv_sec + 1;
+		specptr->tv_nsec = clock_realtime_saved.tv_nsec;
+
+	} else {
+
+		/* bad pointer case */
+
+		if (tc[i].newtime.tv_sec == -2)
+			specptr = tst_get_bad_addr(cleanup);
+
+		/* use given values */
+
+		else {
+			specptr = &spec;
+			specptr->tv_sec = tc[i].newtime.tv_sec;
+			specptr->tv_nsec = tc[i].newtime.tv_nsec;
+		}
+	}
+
+	TEST(sys_clock_settime(tc[i].type, specptr));
+
+	if (TST_RET == -1) {
+
+		if (tc[i].exp_err == TST_ERR) {
+
+			tst_res(TPASS, "clock_settime(2): failed as expected");
+
+		} else {
+			tst_res(TFAIL | TTERRNO, "clock_settime(2): "
+					"failed with different error");
+		}
+
+		return;
+	}
+
+	tst_res(TFAIL | TTERRNO, "clock_settime(2): clock type %d failed",
+			tc[i].type);
+}
+
+static struct tst_test test = {
+	.setup = setup,
+	.test = verify_clock_settime,
+	.cleanup = cleanup,
+	.tcnt = ARRAY_SIZE(tc),
+	.needs_root = 1,
+};