Message ID | cc75beb4074b62e94b8ac92cba17af41b8f5fbdc.1591864369.git.viresh.kumar@linaro.org |
---|---|
State | Superseded |
Headers | show |
Series | [V2,1/2] libs: Import vdso parsing lib from kernel tree | expand |
On Thu, Jun 11, 2020 at 10:34 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > +static void find_vdso_helpers(void) > +{ > + /* > + * Some vDSO exports its clock_gettime() implementation with a different > + * name and version from other architectures, so we need to handle it as > + * a special case. > + */ > +#if defined(__powerpc__) || defined(__powerpc64__) > + const char *version = "LINUX_2.6.15"; > + const char *name = "__kernel_clock_gettime"; > +#elif defined(__aarch64__) > + const char *version = "LINUX_2.6.39"; > + const char *name = "__kernel_clock_gettime"; > +#elif defined(__s390__) > + const char *version = "LINUX_2.6.29"; > + const char *name = "__kernel_clock_gettime"; > +#else > + const char *version = "LINUX_2.6"; > + const char *name = "__vdso_clock_gettime"; > +#endif I see that risc-v uses version="LINUX_4.15", and nds32 uses "LINUX_4", the other ones look correct. > + ret = tv->gettime(clks[i], tst_ts_get(&ts)); > + if (ret) { > + if (errno != ENOSYS) { > + tst_res(TFAIL | TERRNO, "%s: clock_gettime() failed (%d)", > + tst_clock_name(clks[i]), j); > + } > + continue; > + } Is this a test case failure, or does it still succeed after skipping a call? When any of the variants (in particular vdso_clock_gettime64) don't exist, I think you just need to continue without printing anything. Note that older kernels before v5.1 don't have the clock_gettime64 syscall, while riscv and future architectures as well as kernels with CONFIG_COMPAT_32BIT_TIME=n don't have clock_gettime(), and some architectures don't have a vdso, or only the time32 version of it. Arnd Arnd
On 11-06-20, 15:05, Arnd Bergmann wrote: > On Thu, Jun 11, 2020 at 10:34 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > +static void find_vdso_helpers(void) > > +{ > > + /* > > + * Some vDSO exports its clock_gettime() implementation with a different > > + * name and version from other architectures, so we need to handle it as > > + * a special case. > > + */ > > +#if defined(__powerpc__) || defined(__powerpc64__) > > + const char *version = "LINUX_2.6.15"; > > + const char *name = "__kernel_clock_gettime"; > > +#elif defined(__aarch64__) > > + const char *version = "LINUX_2.6.39"; > > + const char *name = "__kernel_clock_gettime"; > > +#elif defined(__s390__) > > + const char *version = "LINUX_2.6.29"; > > + const char *name = "__kernel_clock_gettime"; > > +#else > > + const char *version = "LINUX_2.6"; > > + const char *name = "__vdso_clock_gettime"; > > +#endif > > I see that risc-v uses version="LINUX_4.15", and nds32 uses "LINUX_4", > the other ones look correct. My bad, I only looked at man page of vdso for clock_gettime(), while I looked at kernel source for clock_gettime64() :( > > + ret = tv->gettime(clks[i], tst_ts_get(&ts)); > > + if (ret) { > > + if (errno != ENOSYS) { > > + tst_res(TFAIL | TERRNO, "%s: clock_gettime() failed (%d)", > > + tst_clock_name(clks[i]), j); > > + } > > + continue; > > + } > > Is this a test case failure, or does it still succeed after skipping a call? "continue" takes us to the next variant for the current test loop (out of 10000 loops). So we don't exit here, though this reports a failure and that will be visible in the output. But because we continue here, we will also see a TPASS at the end for the same clock. So if the test was running for CLOCK_REALTIME, then we will see both FAIL and PASS in output. I didn't wanted to abandon the test in such a case and so kept it like that. > When any of the variants (in particular vdso_clock_gettime64) don't > exist, I think you just need to continue without printing anything. That is exactly why we are looking for ENOSYS here. The other code (which you removed) explicitly sets the error to ENOSYS if clock_gettime64() or clock_gettime() vdso isn't available. And so we won't print an error here. Though the setup routine will print only once if the vdso wasn't available, as general information. > Note that older kernels before v5.1 don't have the clock_gettime64 > syscall, while riscv and future architectures as well as kernels with > CONFIG_COMPAT_32BIT_TIME=n don't have clock_gettime(), > and some architectures don't have a vdso, or only the time32 version > of it.
On Fri, Jun 12, 2020 at 9:40 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 11-06-20, 15:05, Arnd Bergmann wrote: > > When any of the variants (in particular vdso_clock_gettime64) don't > > exist, I think you just need to continue without printing anything. > > That is exactly why we are looking for ENOSYS here. The other code > (which you removed) explicitly sets the error to ENOSYS if > clock_gettime64() or clock_gettime() vdso isn't available. And so we > won't print an error here. Though the setup routine will print only > once if the vdso wasn't available, as general information. Ok, got it, I misread the != ENOSYS check for ==ENOSYS. Arnd
diff --git a/runtest/syscalls b/runtest/syscalls index f9a6397560fa..d7c3cbed611a 100644 --- a/runtest/syscalls +++ b/runtest/syscalls @@ -96,6 +96,7 @@ clock_nanosleep04 clock_nanosleep04 clock_gettime01 clock_gettime01 clock_gettime02 clock_gettime02 clock_gettime03 clock_gettime03 +clock_gettime04 clock_gettime04 leapsec01 leapsec01 clock_settime01 clock_settime01 diff --git a/testcases/kernel/syscalls/clock_gettime/.gitignore b/testcases/kernel/syscalls/clock_gettime/.gitignore index 9d06613b6f41..304eedab68c6 100644 --- a/testcases/kernel/syscalls/clock_gettime/.gitignore +++ b/testcases/kernel/syscalls/clock_gettime/.gitignore @@ -1,4 +1,5 @@ clock_gettime01 clock_gettime02 clock_gettime03 +clock_gettime04 leapsec01 diff --git a/testcases/kernel/syscalls/clock_gettime/Makefile b/testcases/kernel/syscalls/clock_gettime/Makefile index 79f671f1c597..1c1cbd7a8853 100644 --- a/testcases/kernel/syscalls/clock_gettime/Makefile +++ b/testcases/kernel/syscalls/clock_gettime/Makefile @@ -3,8 +3,11 @@ top_srcdir ?= ../../../.. +LTPLIBS = ltpvdso + include $(top_srcdir)/include/mk/testcases.mk LDLIBS+=-lrt +clock_gettime04: LDLIBS += -lltpvdso -include $(top_srcdir)/include/mk/generic_leaf_target.mk \ No newline at end of file +include $(top_srcdir)/include/mk/generic_leaf_target.mk diff --git a/testcases/kernel/syscalls/clock_gettime/clock_gettime04.c b/testcases/kernel/syscalls/clock_gettime/clock_gettime04.c new file mode 100644 index 000000000000..a70288ce0cb9 --- /dev/null +++ b/testcases/kernel/syscalls/clock_gettime/clock_gettime04.c @@ -0,0 +1,197 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2020 Linaro Limited. All rights reserved. + * Author: Viresh Kumar<viresh.kumar@linaro.org> + * + * Check time difference between successive readings and report a bug if + * difference found to be over 5 ms. + */ + +#include "config.h" +#include "parse_vdso.h" +#include "tst_timer.h" +#include "tst_safe_clocks.h" + +#include <sys/auxv.h> + +clockid_t clks[] = { + CLOCK_REALTIME, + CLOCK_REALTIME_COARSE, + CLOCK_MONOTONIC, + CLOCK_MONOTONIC_COARSE, + CLOCK_MONOTONIC_RAW, + CLOCK_BOOTTIME, +}; + +typedef int (*gettime_t)(clockid_t clk_id, void *ts); +static gettime_t ptr_vdso_gettime, ptr_vdso_gettime64; + +static inline int _vdso_gettime(gettime_t vdso, clockid_t clk_id, void *ts) +{ + if (!vdso) { + errno = ENOSYS; + return -1; + } + + return vdso(clk_id, ts); +} + +static inline int vdso_gettime(clockid_t clk_id, void *ts) +{ + return _vdso_gettime(ptr_vdso_gettime, clk_id, ts); +} + +static inline int vdso_gettime64(clockid_t clk_id, void *ts) +{ + return _vdso_gettime(ptr_vdso_gettime64, clk_id, ts); +} + +static void find_vdso_helpers(void) +{ + /* + * Some vDSO exports its clock_gettime() implementation with a different + * name and version from other architectures, so we need to handle it as + * a special case. + */ +#if defined(__powerpc__) || defined(__powerpc64__) + const char *version = "LINUX_2.6.15"; + const char *name = "__kernel_clock_gettime"; +#elif defined(__aarch64__) + const char *version = "LINUX_2.6.39"; + const char *name = "__kernel_clock_gettime"; +#elif defined(__s390__) + const char *version = "LINUX_2.6.29"; + const char *name = "__kernel_clock_gettime"; +#else + const char *version = "LINUX_2.6"; + const char *name = "__vdso_clock_gettime"; +#endif + + const char *version64 = "LINUX_2.6"; + const char *name64 = "__vdso_clock_gettime64"; + + unsigned long sysinfo_ehdr = getauxval(AT_SYSINFO_EHDR); + + if (!sysinfo_ehdr) { + tst_res(TINFO, "Couldn't find AT_SYSINFO_EHDR"); + return; + } + + vdso_init_from_sysinfo_ehdr(sysinfo_ehdr); + + ptr_vdso_gettime = vdso_sym(version, name); + if (!ptr_vdso_gettime) + tst_res(TINFO, "Couldn't find vdso_gettime()"); + + ptr_vdso_gettime64 = vdso_sym(version64, name64); + if (!ptr_vdso_gettime64) + tst_res(TINFO, "Couldn't find vdso_gettime64()"); +} + +static inline int my_gettimeofday(clockid_t clk_id, void *ts) +{ + struct timeval tval; + + if (clk_id != CLOCK_REALTIME) + tst_brk(TBROK, "%s: Invalid clk_id, exiting", tst_clock_name(clk_id)); + + if (gettimeofday(&tval, NULL) < 0) + tst_brk(TBROK | TERRNO, "gettimeofday() failed"); + + /* + * The array defines the type to TST_LIBC_TIMESPEC and so we can cast + * this into struct timespec. + */ + *((struct timespec *)ts) = tst_timespec_from_us(tst_timeval_to_us(tval)); + return 0; +} + +static struct test_variants { + int (*gettime)(clockid_t clk_id, void *ts); + enum tst_ts_type type; + char *desc; +} variants[] = { + { .gettime = libc_clock_gettime, .type = TST_LIBC_TIMESPEC, .desc = "vDSO or syscall with libc spec"}, + +#if (__NR_clock_gettime != __LTP__NR_INVALID_SYSCALL) + { .gettime = sys_clock_gettime, .type = TST_KERN_OLD_TIMESPEC, .desc = "syscall with old kernel spec"}, + { .gettime = vdso_gettime, .type = TST_KERN_OLD_TIMESPEC, .desc = "vDSO with old kernel spec"}, +#endif + +#if (__NR_clock_gettime64 != __LTP__NR_INVALID_SYSCALL) + { .gettime = sys_clock_gettime64, .type = TST_KERN_TIMESPEC, .desc = "syscall time64 with kernel spec"}, + { .gettime = vdso_gettime64, .type = TST_KERN_TIMESPEC, .desc = "vDSO time64 with kernel spec"}, +#endif + { .gettime = my_gettimeofday, .type = TST_LIBC_TIMESPEC, .desc = "gettimeofday"}, +}; + +static void run(unsigned int i) +{ + struct tst_ts ts; + long long start, end = 0, diff, slack; + struct test_variants *tv; + int count = 10000, ret; + unsigned int j; + + do { + for (j = 0; j < ARRAY_SIZE(variants); j++) { + /* Refresh time in start */ + start = end; + + tv = &variants[j]; + ts.type = tv->type; + + /* Do gettimeofday() test only for CLOCK_REALTIME */ + if (tv->gettime == my_gettimeofday && clks[i] != CLOCK_REALTIME) + continue; + + ret = tv->gettime(clks[i], tst_ts_get(&ts)); + if (ret) { + if (errno != ENOSYS) { + tst_res(TFAIL | TERRNO, "%s: clock_gettime() failed (%d)", + tst_clock_name(clks[i]), j); + } + continue; + } + + end = tst_ts_to_ns(ts); + + /* Skip comparison on first traversal */ + if (count == 10000 && !j) + continue; + + /* + * gettimeofday() doesn't capture time less than 1 us, + * add 999 to it. + */ + if (tv->gettime == my_gettimeofday) + slack = 999; + else + slack = 0; + + diff = end + slack - start; + if (diff < 0) { + tst_res(TFAIL, "%s: Time travelled backwards (%d): %lld ns", + tst_clock_name(clks[i]), j, diff); + return; + } + + diff /= 1000000; + + if (diff >= 5) { + tst_res(TFAIL, "%s: Difference between successive readings greater than 5 ms (%d): %lld", + tst_clock_name(clks[i]), j, diff); + return; + } + } + } while (--count); + + tst_res(TPASS, "%s: Difference between successive readings is reasonable", + tst_clock_name(clks[i])); +} + +static struct tst_test test = { + .test = run, + .setup = find_vdso_helpers, + .tcnt = ARRAY_SIZE(clks), +};
An issue was reported recently where a bug was found during successive reading of 64 bit time on arm32 platforms. Add a test for that. https://github.com/richfelker/musl-cross-make/issues/96 Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- V2: - Test vDSOs as well and overall improved test. runtest/syscalls | 1 + .../kernel/syscalls/clock_gettime/.gitignore | 1 + .../kernel/syscalls/clock_gettime/Makefile | 5 +- .../syscalls/clock_gettime/clock_gettime04.c | 197 ++++++++++++++++++ 4 files changed, 203 insertions(+), 1 deletion(-) create mode 100644 testcases/kernel/syscalls/clock_gettime/clock_gettime04.c