[LTP] [PATCH] Add clock_settime04 test
Martin Doucha
mdoucha@suse.cz
Fri Jun 27 18:35:27 CEST 2025
Hi,
Cyril already reviewed clock usage so I'll add some suggestions for
other improvements.
On 27. 06. 25 13:17, Andrea Cervesato wrote:
> From: Andrea Cervesato <andrea.cervesato@suse.com>
>
> Test that changing the value of the CLOCK_REALTIME clock via
> clock_settime(2) shall have no effect on a thread that is blocked
> on a relative clock_nanosleep().
>
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
> Both clock_settime_7-1 and clock_settime_8-1 are affected by the
> same bug which is probably related to the way tests are written.
>
> sleep() is used by the parent and is not reliable. It can oversleep
> or undersleep according to system overload or signals which are
> received. And we never check for its return value. Using
> clock_nanosleep would make parent more reliable in this case.
>
> At the same time, the test is taking for granted a certain
> synchronization between child and parent, which is not always true
> in case of system overload.
>
> My suggestion is to rewrite the test using LTP, which has better
> timing handling (see tst_timer.h).
> ---
> runtest/syscalls | 1 +
> testcases/kernel/syscalls/clock_settime/.gitignore | 1 +
> .../syscalls/clock_settime/clock_settime04.c | 170 +++++++++++++++++++++
> 3 files changed, 172 insertions(+)
>
> diff --git a/runtest/syscalls b/runtest/syscalls
> index d5582ca8da01f11adba75a56d22cb6e5c4996c68..2dc734c5537f1d5477f4c98d08b9717fb89ac05c 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -112,6 +112,7 @@ leapsec01 leapsec01
> clock_settime01 clock_settime01
> clock_settime02 clock_settime02
> clock_settime03 clock_settime03
> +clock_settime04 clock_settime04
>
> clone01 clone01
> clone02 clone02
> diff --git a/testcases/kernel/syscalls/clock_settime/.gitignore b/testcases/kernel/syscalls/clock_settime/.gitignore
> index b66169b3eb7b4d8c8ea95e9e689b612d8da37b11..8bcc83d6fc9162087e99193a00b8d3d784d4737d 100644
> --- a/testcases/kernel/syscalls/clock_settime/.gitignore
> +++ b/testcases/kernel/syscalls/clock_settime/.gitignore
> @@ -1,3 +1,4 @@
> clock_settime01
> clock_settime02
> clock_settime03
> +clock_settime04
> diff --git a/testcases/kernel/syscalls/clock_settime/clock_settime04.c b/testcases/kernel/syscalls/clock_settime/clock_settime04.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..6050fb3c6dabe81116d4a3697605d8137691458a
> --- /dev/null
> +++ b/testcases/kernel/syscalls/clock_settime/clock_settime04.c
> @@ -0,0 +1,170 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2025 Andrea Cervesato <andrea.cervesato@suse.com>
> + */
> +
> +/*\
> + * Verify that changing the value of the CLOCK_REALTIME clock via
> + * clock_settime() shall have no effect on a thread that is blocked on
> + * absolute clock_nanosleep().
> + */
> +
> +#include "tst_test.h"
> +#include "tst_timer.h"
> +#include "time64_variants.h"
> +
> +#define SEC_TO_US(x) (x * 1000 * 1000)
> +
> +#define CHILD_SLEEP_US SEC_TO_US(5)
> +#define PARENT_SLEEP_US SEC_TO_US(2)
> +#define DELTA_US SEC_TO_US(1)
> +
> +static struct tst_ts *begin, *sleep_child, *sleep_parent, *end;
> +
> +static struct time64_variants variants[] = {
> + {
> + .clock_gettime = libc_clock_gettime,
> + .clock_settime = libc_clock_settime,
> + .clock_nanosleep = libc_clock_nanosleep,
> + .ts_type = TST_LIBC_TIMESPEC,
> + .desc = "vDSO or syscall with libc spec"
> + },
> +
> +#if (__NR_clock_nanosleep != __LTP__NR_INVALID_SYSCALL)
> + {
> + .clock_gettime = sys_clock_gettime,
> + .clock_settime = sys_clock_settime,
> + .clock_nanosleep = sys_clock_nanosleep,
> + .ts_type = TST_KERN_OLD_TIMESPEC,
> + .desc = "syscall with old kernel spec"
> + },
> +#endif
> +
> +#if (__NR_clock_nanosleep_time64 != __LTP__NR_INVALID_SYSCALL)
> + {
> + .clock_gettime = sys_clock_gettime64,
> + .clock_settime = sys_clock_settime64,
> + .clock_nanosleep = sys_clock_nanosleep64,
> + .ts_type = TST_KERN_TIMESPEC,
> + .desc = "syscall time64 with kernel spec"
> + },
> +#endif
> +};
> +
> +static void child_nanosleep(struct time64_variants *tv, unsigned int tc_index)
> +{
> + long long delta;
> +
> + if (tc_index) {
> + tst_res(TINFO, "Using absolute time sleep");
> +
> + *sleep_child = tst_ts_add_us(*begin, CHILD_SLEEP_US);
You could pass the flags for clock_nanosleep() from caller instead of
passing tc_index. Then you need only the TINFO and sleep_child init in
the condition and the code from TST_CHECKPOINT_WAKE() onward will be the
same for both cases.
if (flags & TIMER_ABSTIME) {
tst_res(...);
*sleep_child = ...;
} else {
tst_res(...);
tst_ts_set_sec(...);
tst_ts_set_nsec(...);
*sleep_child = ...;
}
TST_CHECKPOINT_WAKE(0);
...
> +
> + TST_CHECKPOINT_WAKE(0);
> +
> + TEST(tv->clock_nanosleep(CLOCK_REALTIME, TIMER_ABSTIME,
> + tst_ts_get(sleep_child), NULL));
> + if (TST_RET)
> + tst_brk(TBROK | TERRNO, "clock_nanosleep() error");
> + } else {
> + tst_res(TINFO, "Using relative time sleep");
> +
> + tst_ts_set_sec(sleep_child, 0);
> + tst_ts_set_nsec(sleep_child, 0);
> +
> + *sleep_child = tst_ts_add_us(*sleep_child, CHILD_SLEEP_US);
> +
> + TST_CHECKPOINT_WAKE(0);
> +
> + TEST(tv->clock_nanosleep(CLOCK_REALTIME, 0,
> + tst_ts_get(sleep_child), NULL));
> + if (TST_RET)
> + tst_brk(TBROK | TERRNO, "clock_nanosleep() error");
> +
> + /* normalize to absolute time so we can compare times later on */
> + *sleep_child = tst_ts_add_us(*begin, CHILD_SLEEP_US - PARENT_SLEEP_US);
> + }
> +
> + TEST(tv->clock_gettime(CLOCK_REALTIME, tst_ts_get(end)));
> + if (TST_RET == -1)
> + tst_brk(TBROK | TERRNO, "clock_gettime() error");
> +
> + if (tst_ts_lt(*end, *sleep_child)) {
If you check only tst_ts_abs_diff_us(*begin, *end), you don't need to
conditionally modify sleep_child above.
> + tst_res(TFAIL, "clock_settime() didn't sleep enough. "
> + "end=%lld < sleep=%lld", tst_ts_get_sec(*end),
> + tst_ts_get_sec(*sleep_child));
> + return;
> + }
> +
> + delta = tst_ts_abs_diff_us(*sleep_child, *end);
> + if (delta > DELTA_US) {
> + tst_res(TFAIL, "clock_settime() affected child sleep. "
> + "end=%lld <= sleep=%lld", tst_ts_get_nsec(*end),
> + tst_ts_get_nsec(*sleep_child));
> + return;
> + }
> +
> + tst_res(TPASS, "clock_settime() didn't affect child sleep "
> + "(delta time: %lld us)", delta);
> +}
> +
> +static void run(unsigned int tc_index)
> +{
> + struct time64_variants *tv = &variants[tst_variant];
> +
> + TEST(tv->clock_gettime(CLOCK_REALTIME, tst_ts_get(begin)));
> + if (TST_RET == -1)
> + tst_brk(TBROK | TERRNO, "clock_gettime() error");
Any non-zero value should trigger TBROK.
> +
> + if (!SAFE_FORK()) {
> + child_nanosleep(tv, tc_index);
> + exit(0);
> + }
> +
> + *sleep_parent = tst_ts_add_us(*begin, PARENT_SLEEP_US);
> +
> + TST_CHECKPOINT_WAIT(0);
> +
> + TEST(tv->clock_nanosleep(CLOCK_REALTIME, TIMER_ABSTIME,
> + tst_ts_get(sleep_parent), NULL));
> + if (TST_RET)
> + tst_brk(TBROK | TERRNO, "clock_nanosleep() error");
> +
> + TEST(tv->clock_settime(CLOCK_REALTIME, tst_ts_get(begin)));
> + if (TST_RET)
> + tst_brk(TBROK | TERRNO, "clock_settime() error");
> +
> + tst_reap_children();
> +
> + /* restore initial clock */
> + *end = tst_ts_sub_us(*begin, PARENT_SLEEP_US);
> +
> + TEST(tv->clock_settime(CLOCK_REALTIME, tst_ts_get(end)));
> + if (TST_RET)
> + tst_brk(TBROK | TERRNO, "clock_settime() error");
> +}
> +
> +static void setup(void)
> +{
> + begin->type = end->type = sleep_child->type = sleep_parent->type =
> + variants[tst_variant].ts_type;
> +
> + tst_res(TINFO, "Testing variant: %s", variants[tst_variant].desc);
> +}
> +
> +static struct tst_test test = {
> + .test = run,
> + .setup = setup,
> + .tcnt = 2,
> + .needs_root = 1,
> + .forks_child = 1,
> + .needs_checkpoints = 1,
> + .test_variants = ARRAY_SIZE(variants),
> + .bufs = (struct tst_buffers []) {
> + {&begin, .size = sizeof(struct tst_ts)},
> + {&sleep_child, .size = sizeof(struct tst_ts)},
> + {&sleep_parent, .size = sizeof(struct tst_ts)},
> + {&end, .size = sizeof(struct tst_ts)},
> + {},
> + }
> +};
>
> ---
> base-commit: df591113afeb31107bc45bd5ba28a99b556d1028
> change-id: 20250620-clock_nanosleep05-60b3cfba52fe
>
> Best regards,
--
Martin Doucha mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic
More information about the ltp
mailing list