clock_getres in the vDSO library has to preserve the same behaviour of posix_get_hrtimer_res().
In particular, posix_get_hrtimer_res() does: sec = 0; ns = hrtimer_resolution; and hrtimer_resolution depends on the enablement of the high resolution timers that can happen either at compile or at run time.
A possible fix is to change the vdso implementation of clock_getres, keeping a copy of hrtimer_resolution in vdso data and using that directly [1].
This patchset implements the proposed fix for arm64, powerpc, s390, nds32 and adds a test to verify that the syscall and the vdso library implementation of clock_getres return the same values.
Even if these patches are unified by the same topic, there is no dependency between them, hence they can be merged singularly by each arch maintainer.
Note: arm64 and nds32 respective fixes have been merged in 5.2-rc1, hence they have been removed from this series.
[1] https://marc.info/?l=linux-arm-kernel&m=155110381930196&w=2
Changes: -------- v3: - Rebased on 5.2-rc1. - Addressed review comments. v2: - Rebased on 5.1-rc5. - Addressed review comments.
Cc: Christophe Leroy christophe.leroy@c-s.fr Cc: Benjamin Herrenschmidt benh@kernel.crashing.org Cc: Paul Mackerras paulus@samba.org Cc: Michael Ellerman mpe@ellerman.id.au Cc: Martin Schwidefsky schwidefsky@de.ibm.com Cc: Heiko Carstens heiko.carstens@de.ibm.com Cc: Shuah Khan shuah@kernel.org Cc: Thomas Gleixner tglx@linutronix.de Cc: Arnd Bergmann arnd@arndb.de Signed-off-by: Vincenzo Frascino vincenzo.frascino@arm.com
Vincenzo Frascino (3): powerpc: Fix vDSO clock_getres() s390: Fix vDSO clock_getres() kselftest: Extend vDSO selftest to clock_getres
arch/powerpc/include/asm/vdso_datapage.h | 2 + arch/powerpc/kernel/asm-offsets.c | 2 +- arch/powerpc/kernel/time.c | 1 + arch/powerpc/kernel/vdso32/gettimeofday.S | 7 +- arch/powerpc/kernel/vdso64/gettimeofday.S | 7 +- arch/s390/include/asm/vdso.h | 1 + arch/s390/kernel/asm-offsets.c | 2 +- arch/s390/kernel/time.c | 1 + arch/s390/kernel/vdso32/clock_getres.S | 12 +- arch/s390/kernel/vdso64/clock_getres.S | 10 +- tools/testing/selftests/vDSO/Makefile | 2 + .../selftests/vDSO/vdso_clock_getres.c | 137 ++++++++++++++++++ 12 files changed, 168 insertions(+), 16 deletions(-) create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
clock_getres in the vDSO library has to preserve the same behaviour of posix_get_hrtimer_res().
In particular, posix_get_hrtimer_res() does: sec = 0; ns = hrtimer_resolution; and hrtimer_resolution depends on the enablement of the high resolution timers that can happen either at compile or at run time.
Fix the powerpc vdso implementation of clock_getres keeping a copy of hrtimer_resolution in vdso data and using that directly.
Fixes: a7f290dad32e ("[PATCH] powerpc: Merge vdso's and add vdso support to 32 bits kernel") Cc: stable@vger.kernel.org Cc: Benjamin Herrenschmidt benh@kernel.crashing.org Cc: Paul Mackerras paulus@samba.org Cc: Michael Ellerman mpe@ellerman.id.au Signed-off-by: Vincenzo Frascino vincenzo.frascino@arm.com Reviewed-by: Christophe Leroy christophe.leroy@c-s.fr ---
Note: This patch is independent from the others in this series, hence it can be merged singularly by the powerpc maintainers.
arch/powerpc/include/asm/vdso_datapage.h | 2 ++ arch/powerpc/kernel/asm-offsets.c | 2 +- arch/powerpc/kernel/time.c | 1 + arch/powerpc/kernel/vdso32/gettimeofday.S | 7 +++++-- arch/powerpc/kernel/vdso64/gettimeofday.S | 7 +++++-- 5 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/vdso_datapage.h b/arch/powerpc/include/asm/vdso_datapage.h index bbc06bd72b1f..4333b9a473dc 100644 --- a/arch/powerpc/include/asm/vdso_datapage.h +++ b/arch/powerpc/include/asm/vdso_datapage.h @@ -86,6 +86,7 @@ struct vdso_data { __s32 wtom_clock_nsec; /* Wall to monotonic clock nsec */ __s64 wtom_clock_sec; /* Wall to monotonic clock sec */ struct timespec stamp_xtime; /* xtime as at tb_orig_stamp */ + __u32 hrtimer_res; /* hrtimer resolution */ __u32 syscall_map_64[SYSCALL_MAP_SIZE]; /* map of syscalls */ __u32 syscall_map_32[SYSCALL_MAP_SIZE]; /* map of syscalls */ }; @@ -107,6 +108,7 @@ struct vdso_data { __s32 wtom_clock_nsec; struct timespec stamp_xtime; /* xtime as at tb_orig_stamp */ __u32 stamp_sec_fraction; /* fractional seconds of stamp_xtime */ + __u32 hrtimer_res; /* hrtimer resolution */ __u32 syscall_map_32[SYSCALL_MAP_SIZE]; /* map of syscalls */ __u32 dcache_block_size; /* L1 d-cache block size */ __u32 icache_block_size; /* L1 i-cache block size */ diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c index 8e02444e9d3d..dfc40f29f2b9 100644 --- a/arch/powerpc/kernel/asm-offsets.c +++ b/arch/powerpc/kernel/asm-offsets.c @@ -389,6 +389,7 @@ int main(void) OFFSET(WTOM_CLOCK_NSEC, vdso_data, wtom_clock_nsec); OFFSET(STAMP_XTIME, vdso_data, stamp_xtime); OFFSET(STAMP_SEC_FRAC, vdso_data, stamp_sec_fraction); + OFFSET(CLOCK_REALTIME_RES, vdso_data, hrtimer_res); OFFSET(CFG_ICACHE_BLOCKSZ, vdso_data, icache_block_size); OFFSET(CFG_DCACHE_BLOCKSZ, vdso_data, dcache_block_size); OFFSET(CFG_ICACHE_LOGBLOCKSZ, vdso_data, icache_log_block_size); @@ -419,7 +420,6 @@ int main(void) DEFINE(CLOCK_REALTIME_COARSE, CLOCK_REALTIME_COARSE); DEFINE(CLOCK_MONOTONIC_COARSE, CLOCK_MONOTONIC_COARSE); DEFINE(NSEC_PER_SEC, NSEC_PER_SEC); - DEFINE(CLOCK_REALTIME_RES, MONOTONIC_RES_NSEC);
#ifdef CONFIG_BUG DEFINE(BUG_ENTRY_SIZE, sizeof(struct bug_entry)); diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c index 325d60633dfa..4ea4e9d7a58e 100644 --- a/arch/powerpc/kernel/time.c +++ b/arch/powerpc/kernel/time.c @@ -963,6 +963,7 @@ void update_vsyscall(struct timekeeper *tk) vdso_data->wtom_clock_nsec = tk->wall_to_monotonic.tv_nsec; vdso_data->stamp_xtime = xt; vdso_data->stamp_sec_fraction = frac_sec; + vdso_data->hrtimer_res = hrtimer_resolution; smp_wmb(); ++(vdso_data->tb_update_count); } diff --git a/arch/powerpc/kernel/vdso32/gettimeofday.S b/arch/powerpc/kernel/vdso32/gettimeofday.S index afd516b572f8..2b5f9e83c610 100644 --- a/arch/powerpc/kernel/vdso32/gettimeofday.S +++ b/arch/powerpc/kernel/vdso32/gettimeofday.S @@ -160,12 +160,15 @@ V_FUNCTION_BEGIN(__kernel_clock_getres) cror cr0*4+eq,cr0*4+eq,cr1*4+eq bne cr0,99f
+ mflr r12 + .cfi_register lr,r12 + bl __get_datapage@local + lwz r5,CLOCK_REALTIME_RES(r3) + mtlr r12 li r3,0 cmpli cr0,r4,0 crclr cr0*4+so beqlr - lis r5,CLOCK_REALTIME_RES@h - ori r5,r5,CLOCK_REALTIME_RES@l stw r3,TSPC32_TV_SEC(r4) stw r5,TSPC32_TV_NSEC(r4) blr diff --git a/arch/powerpc/kernel/vdso64/gettimeofday.S b/arch/powerpc/kernel/vdso64/gettimeofday.S index 1f324c28705b..f07730f73d5e 100644 --- a/arch/powerpc/kernel/vdso64/gettimeofday.S +++ b/arch/powerpc/kernel/vdso64/gettimeofday.S @@ -190,12 +190,15 @@ V_FUNCTION_BEGIN(__kernel_clock_getres) cror cr0*4+eq,cr0*4+eq,cr1*4+eq bne cr0,99f
+ mflr r12 + .cfi_register lr,r12 + bl V_LOCAL_FUNC(__get_datapage) + lwz r5,CLOCK_REALTIME_RES(r3) + mtlr r12 li r3,0 cmpldi cr0,r4,0 crclr cr0*4+so beqlr - lis r5,CLOCK_REALTIME_RES@h - ori r5,r5,CLOCK_REALTIME_RES@l std r3,TSPC64_TV_SEC(r4) std r5,TSPC64_TV_NSEC(r4) blr
clock_getres in the vDSO library has to preserve the same behaviour of posix_get_hrtimer_res().
In particular, posix_get_hrtimer_res() does: sec = 0; ns = hrtimer_resolution; and hrtimer_resolution depends on the enablement of the high resolution timers that can happen either at compile or at run time.
Fix the s390 vdso implementation of clock_getres keeping a copy of hrtimer_resolution in vdso data and using that directly.
Cc: Martin Schwidefsky schwidefsky@de.ibm.com Cc: Heiko Carstens heiko.carstens@de.ibm.com Signed-off-by: Vincenzo Frascino vincenzo.frascino@arm.com Acked-by: Martin Schwidefsky schwidefsky@de.ibm.com ---
Note: This patch is independent from the others in this series, hence it can be merged singularly by the s390 maintainers.
arch/s390/include/asm/vdso.h | 1 + arch/s390/kernel/asm-offsets.c | 2 +- arch/s390/kernel/time.c | 1 + arch/s390/kernel/vdso32/clock_getres.S | 12 +++++++----- arch/s390/kernel/vdso64/clock_getres.S | 10 +++++----- 5 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/arch/s390/include/asm/vdso.h b/arch/s390/include/asm/vdso.h index 169d7604eb80..f3ba84fa9bd1 100644 --- a/arch/s390/include/asm/vdso.h +++ b/arch/s390/include/asm/vdso.h @@ -36,6 +36,7 @@ struct vdso_data { __u32 tk_shift; /* Shift used for xtime_nsec 0x60 */ __u32 ts_dir; /* TOD steering direction 0x64 */ __u64 ts_end; /* TOD steering end 0x68 */ + __u32 hrtimer_res; /* hrtimer resolution 0x70 */ };
struct vdso_per_cpu_data { diff --git a/arch/s390/kernel/asm-offsets.c b/arch/s390/kernel/asm-offsets.c index 41ac4ad21311..4a229a60b24a 100644 --- a/arch/s390/kernel/asm-offsets.c +++ b/arch/s390/kernel/asm-offsets.c @@ -76,6 +76,7 @@ int main(void) OFFSET(__VDSO_TK_SHIFT, vdso_data, tk_shift); OFFSET(__VDSO_TS_DIR, vdso_data, ts_dir); OFFSET(__VDSO_TS_END, vdso_data, ts_end); + OFFSET(__VDSO_CLOCK_REALTIME_RES, vdso_data, hrtimer_res); OFFSET(__VDSO_ECTG_BASE, vdso_per_cpu_data, ectg_timer_base); OFFSET(__VDSO_ECTG_USER, vdso_per_cpu_data, ectg_user_time); OFFSET(__VDSO_CPU_NR, vdso_per_cpu_data, cpu_nr); @@ -87,7 +88,6 @@ int main(void) DEFINE(__CLOCK_REALTIME_COARSE, CLOCK_REALTIME_COARSE); DEFINE(__CLOCK_MONOTONIC_COARSE, CLOCK_MONOTONIC_COARSE); DEFINE(__CLOCK_THREAD_CPUTIME_ID, CLOCK_THREAD_CPUTIME_ID); - DEFINE(__CLOCK_REALTIME_RES, MONOTONIC_RES_NSEC); DEFINE(__CLOCK_COARSE_RES, LOW_RES_NSEC); BLANK(); /* idle data offsets */ diff --git a/arch/s390/kernel/time.c b/arch/s390/kernel/time.c index e8766beee5ad..8ea9db599d38 100644 --- a/arch/s390/kernel/time.c +++ b/arch/s390/kernel/time.c @@ -310,6 +310,7 @@ void update_vsyscall(struct timekeeper *tk)
vdso_data->tk_mult = tk->tkr_mono.mult; vdso_data->tk_shift = tk->tkr_mono.shift; + vdso_data->hrtimer_res = hrtimer_resolution; smp_wmb(); ++vdso_data->tb_update_count; } diff --git a/arch/s390/kernel/vdso32/clock_getres.S b/arch/s390/kernel/vdso32/clock_getres.S index eaf9cf1417f6..fecd7684c645 100644 --- a/arch/s390/kernel/vdso32/clock_getres.S +++ b/arch/s390/kernel/vdso32/clock_getres.S @@ -18,20 +18,22 @@ __kernel_clock_getres: CFI_STARTPROC basr %r1,0 - la %r1,4f-.(%r1) +10: al %r1,4f-10b(%r1) + l %r0,__VDSO_CLOCK_REALTIME_RES(%r1) chi %r2,__CLOCK_REALTIME je 0f chi %r2,__CLOCK_MONOTONIC je 0f - la %r1,5f-4f(%r1) + basr %r1,0 + la %r1,5f-.(%r1) + l %r0,0(%r1) chi %r2,__CLOCK_REALTIME_COARSE je 0f chi %r2,__CLOCK_MONOTONIC_COARSE jne 3f 0: ltr %r3,%r3 jz 2f /* res == NULL */ -1: l %r0,0(%r1) - xc 0(4,%r3),0(%r3) /* set tp->tv_sec to zero */ +1: xc 0(4,%r3),0(%r3) /* set tp->tv_sec to zero */ st %r0,4(%r3) /* store tp->tv_usec */ 2: lhi %r2,0 br %r14 @@ -39,6 +41,6 @@ __kernel_clock_getres: svc 0 br %r14 CFI_ENDPROC -4: .long __CLOCK_REALTIME_RES +4: .long _vdso_data - 10b 5: .long __CLOCK_COARSE_RES .size __kernel_clock_getres,.-__kernel_clock_getres diff --git a/arch/s390/kernel/vdso64/clock_getres.S b/arch/s390/kernel/vdso64/clock_getres.S index 081435398e0a..022b58c980db 100644 --- a/arch/s390/kernel/vdso64/clock_getres.S +++ b/arch/s390/kernel/vdso64/clock_getres.S @@ -17,12 +17,14 @@ .type __kernel_clock_getres,@function __kernel_clock_getres: CFI_STARTPROC - larl %r1,4f + larl %r1,3f + lg %r0,0(%r1) cghi %r2,__CLOCK_REALTIME_COARSE je 0f cghi %r2,__CLOCK_MONOTONIC_COARSE je 0f - larl %r1,3f + larl %r1,_vdso_data + l %r0,__VDSO_CLOCK_REALTIME_RES(%r1) cghi %r2,__CLOCK_REALTIME je 0f cghi %r2,__CLOCK_MONOTONIC @@ -36,7 +38,6 @@ __kernel_clock_getres: jz 2f 0: ltgr %r3,%r3 jz 1f /* res == NULL */ - lg %r0,0(%r1) xc 0(8,%r3),0(%r3) /* set tp->tv_sec to zero */ stg %r0,8(%r3) /* store tp->tv_usec */ 1: lghi %r2,0 @@ -45,6 +46,5 @@ __kernel_clock_getres: svc 0 br %r14 CFI_ENDPROC -3: .quad __CLOCK_REALTIME_RES -4: .quad __CLOCK_COARSE_RES +3: .quad __CLOCK_COARSE_RES .size __kernel_clock_getres,.-__kernel_clock_getres
The current version of the multiarch vDSO selftest verifies only gettimeofday.
Extend the vDSO selftest to clock_getres, to verify that the syscall and the vDSO library function return the same information.
The extension has been used to verify the hrtimer_resoltion fix.
Cc: Shuah Khan shuah@kernel.org Signed-off-by: Vincenzo Frascino vincenzo.frascino@arm.com ---
Note: This patch is independent from the others in this series, hence it can be merged singularly by the kselftest maintainers.
tools/testing/selftests/vDSO/Makefile | 2 + .../selftests/vDSO/vdso_clock_getres.c | 137 ++++++++++++++++++ 2 files changed, 139 insertions(+) create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile index 9e03d61f52fd..d5c5bfdf1ac1 100644 --- a/tools/testing/selftests/vDSO/Makefile +++ b/tools/testing/selftests/vDSO/Makefile @@ -5,6 +5,7 @@ uname_M := $(shell uname -m 2>/dev/null || echo not) ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/)
TEST_GEN_PROGS := $(OUTPUT)/vdso_test +TEST_GEN_PROGS += $(OUTPUT)/vdso_clock_getres ifeq ($(ARCH),x86) TEST_GEN_PROGS += $(OUTPUT)/vdso_standalone_test_x86 endif @@ -18,6 +19,7 @@ endif
all: $(TEST_GEN_PROGS) $(OUTPUT)/vdso_test: parse_vdso.c vdso_test.c +$(OUTPUT)/vdso_clock_getres: vdso_clock_getres.c $(OUTPUT)/vdso_standalone_test_x86: vdso_standalone_test_x86.c parse_vdso.c $(CC) $(CFLAGS) $(CFLAGS_vdso_standalone_test_x86) \ vdso_standalone_test_x86.c parse_vdso.c \ diff --git a/tools/testing/selftests/vDSO/vdso_clock_getres.c b/tools/testing/selftests/vDSO/vdso_clock_getres.c new file mode 100644 index 000000000000..341a9bc34ffc --- /dev/null +++ b/tools/testing/selftests/vDSO/vdso_clock_getres.c @@ -0,0 +1,137 @@ +// SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note +/* + * vdso_clock_getres.c: Sample code to test clock_getres. + * Copyright (c) 2019 Arm Ltd. + * + * Compile with: + * gcc -std=gnu99 vdso_clock_getres.c + * + * Tested on ARM, ARM64, MIPS32, x86 (32-bit and 64-bit), + * Power (32-bit and 64-bit), S390x (32-bit and 64-bit). + * Might work on other architectures. + */ + +#define _GNU_SOURCE +#include <elf.h> +#include <err.h> +#include <fcntl.h> +#include <stdint.h> +#include <stdio.h> +#include <stdlib.h> +#include <time.h> +#include <sys/auxv.h> +#include <sys/mman.h> +#include <sys/time.h> +#include <unistd.h> +#include <sys/syscall.h> + +#include "../kselftest.h" + +static long syscall_clock_getres(clockid_t _clkid, struct timespec *_ts) +{ + long ret; + + ret = syscall(SYS_clock_getres, _clkid, _ts); + + return ret; +} + +const char *vdso_clock_name[12] = { + "CLOCK_REALTIME", + "CLOCK_MONOTONIC", + "CLOCK_PROCESS_CPUTIME_ID", + "CLOCK_THREAD_CPUTIME_ID", + "CLOCK_MONOTONIC_RAW", + "CLOCK_REALTIME_COARSE", + "CLOCK_MONOTONIC_COARSE", + "CLOCK_BOOTTIME", + "CLOCK_REALTIME_ALARM", + "CLOCK_BOOTTIME_ALARM", + "CLOCK_SGI_CYCLE", + "CLOCK_TAI", +}; + +/* + * This function calls clock_getres in vdso and by system call + * with different values for clock_id. + * + * Example of output: + * + * clock_id: CLOCK_REALTIME [PASS] + * clock_id: CLOCK_BOOTTIME [PASS] + * clock_id: CLOCK_TAI [PASS] + * clock_id: CLOCK_REALTIME_COARSE [PASS] + * clock_id: CLOCK_MONOTONIC [PASS] + * clock_id: CLOCK_MONOTONIC_RAW [PASS] + * clock_id: CLOCK_MONOTONIC_COARSE [PASS] + */ +static inline int vdso_test_clock(unsigned int clock_id) +{ + struct timespec x, y; + + printf("clock_id: %s", vdso_clock_name[clock_id]); + clock_getres(clock_id, &x); + syscall_clock_getres(clock_id, &y); + + if ((x.tv_sec != y.tv_sec) || (x.tv_sec != y.tv_sec)) { + printf(" [FAIL]\n"); + return KSFT_FAIL; + } + + printf(" [PASS]\n"); + return 0; +} + +int main(int argc, char **argv) +{ + int ret; + +#if _POSIX_TIMERS > 0 + +#ifdef CLOCK_REALTIME + ret = vdso_test_clock(CLOCK_REALTIME); + if (ret) + goto out; +#endif + +#ifdef CLOCK_BOOTTIME + ret = vdso_test_clock(CLOCK_BOOTTIME); + if (ret) + goto out; +#endif + +#ifdef CLOCK_TAI + ret = vdso_test_clock(CLOCK_TAI); + if (ret) + goto out; +#endif + +#ifdef CLOCK_REALTIME_COARSE + ret = vdso_test_clock(CLOCK_REALTIME_COARSE); + if (ret) + goto out; +#endif + +#ifdef CLOCK_MONOTONIC + ret = vdso_test_clock(CLOCK_MONOTONIC); + if (ret) + goto out; +#endif + +#ifdef CLOCK_MONOTONIC_RAW + ret = vdso_test_clock(CLOCK_MONOTONIC_RAW); + if (ret) + goto out; +#endif + +#ifdef CLOCK_MONOTONIC_COARSE + ret = vdso_test_clock(CLOCK_MONOTONIC_COARSE); + if (ret) + goto out; +#endif + +#endif + +out: + return ret; +}
Le 22/05/2019 à 13:07, Vincenzo Frascino a écrit :
The current version of the multiarch vDSO selftest verifies only gettimeofday.
Extend the vDSO selftest to clock_getres, to verify that the syscall and the vDSO library function return the same information.
The extension has been used to verify the hrtimer_resoltion fix.
Cc: Shuah Khan shuah@kernel.org Signed-off-by: Vincenzo Frascino vincenzo.frascino@arm.com
Note: This patch is independent from the others in this series, hence it can be merged singularly by the kselftest maintainers.
tools/testing/selftests/vDSO/Makefile | 2 + .../selftests/vDSO/vdso_clock_getres.c | 137 ++++++++++++++++++ 2 files changed, 139 insertions(+) create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile index 9e03d61f52fd..d5c5bfdf1ac1 100644 --- a/tools/testing/selftests/vDSO/Makefile +++ b/tools/testing/selftests/vDSO/Makefile @@ -5,6 +5,7 @@ uname_M := $(shell uname -m 2>/dev/null || echo not) ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/) TEST_GEN_PROGS := $(OUTPUT)/vdso_test +TEST_GEN_PROGS += $(OUTPUT)/vdso_clock_getres ifeq ($(ARCH),x86) TEST_GEN_PROGS += $(OUTPUT)/vdso_standalone_test_x86 endif @@ -18,6 +19,7 @@ endif all: $(TEST_GEN_PROGS) $(OUTPUT)/vdso_test: parse_vdso.c vdso_test.c +$(OUTPUT)/vdso_clock_getres: vdso_clock_getres.c $(OUTPUT)/vdso_standalone_test_x86: vdso_standalone_test_x86.c parse_vdso.c $(CC) $(CFLAGS) $(CFLAGS_vdso_standalone_test_x86) \ vdso_standalone_test_x86.c parse_vdso.c \ diff --git a/tools/testing/selftests/vDSO/vdso_clock_getres.c b/tools/testing/selftests/vDSO/vdso_clock_getres.c new file mode 100644 index 000000000000..341a9bc34ffc --- /dev/null +++ b/tools/testing/selftests/vDSO/vdso_clock_getres.c @@ -0,0 +1,137 @@ +// SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note +/*
- vdso_clock_getres.c: Sample code to test clock_getres.
- Copyright (c) 2019 Arm Ltd.
- Compile with:
- gcc -std=gnu99 vdso_clock_getres.c
- Tested on ARM, ARM64, MIPS32, x86 (32-bit and 64-bit),
- Power (32-bit and 64-bit), S390x (32-bit and 64-bit).
- Might work on other architectures.
- */
+#define _GNU_SOURCE +#include <elf.h> +#include <err.h> +#include <fcntl.h> +#include <stdint.h> +#include <stdio.h> +#include <stdlib.h> +#include <time.h> +#include <sys/auxv.h> +#include <sys/mman.h> +#include <sys/time.h> +#include <unistd.h> +#include <sys/syscall.h>
+#include "../kselftest.h"
+static long syscall_clock_getres(clockid_t _clkid, struct timespec *_ts) +{
- long ret;
- ret = syscall(SYS_clock_getres, _clkid, _ts);
- return ret;
+}
+const char *vdso_clock_name[12] = {
- "CLOCK_REALTIME",
- "CLOCK_MONOTONIC",
- "CLOCK_PROCESS_CPUTIME_ID",
- "CLOCK_THREAD_CPUTIME_ID",
- "CLOCK_MONOTONIC_RAW",
- "CLOCK_REALTIME_COARSE",
- "CLOCK_MONOTONIC_COARSE",
- "CLOCK_BOOTTIME",
- "CLOCK_REALTIME_ALARM",
- "CLOCK_BOOTTIME_ALARM",
- "CLOCK_SGI_CYCLE",
- "CLOCK_TAI",
+};
+/*
- This function calls clock_getres in vdso and by system call
- with different values for clock_id.
- Example of output:
- clock_id: CLOCK_REALTIME [PASS]
- clock_id: CLOCK_BOOTTIME [PASS]
- clock_id: CLOCK_TAI [PASS]
- clock_id: CLOCK_REALTIME_COARSE [PASS]
- clock_id: CLOCK_MONOTONIC [PASS]
- clock_id: CLOCK_MONOTONIC_RAW [PASS]
- clock_id: CLOCK_MONOTONIC_COARSE [PASS]
- */
+static inline int vdso_test_clock(unsigned int clock_id) +{
- struct timespec x, y;
- printf("clock_id: %s", vdso_clock_name[clock_id]);
- clock_getres(clock_id, &x);
- syscall_clock_getres(clock_id, &y);
- if ((x.tv_sec != y.tv_sec) || (x.tv_sec != y.tv_sec)) {
printf(" [FAIL]\n");return KSFT_FAIL;- }
- printf(" [PASS]\n");
- return 0;
+}
+int main(int argc, char **argv) +{
- int ret;
+#if _POSIX_TIMERS > 0
+#ifdef CLOCK_REALTIME
Why do you need that #ifdef and all the ones below ?
CLOCK_REALTIME (and others) is defined in include/uapi/linux/time.h, so it should be there when you build the test, shouldn't it ?
- ret = vdso_test_clock(CLOCK_REALTIME);
- if (ret)
goto out;
Why that goto ? Nothing is done at out, so a 'return ret' would be better I think.
And do we really want to stop at first failure ? Wouldn't it be better to run all the tests regardless ?
Christophe
+#endif
+#ifdef CLOCK_BOOTTIME
- ret = vdso_test_clock(CLOCK_BOOTTIME);
- if (ret)
goto out;+#endif
+#ifdef CLOCK_TAI
- ret = vdso_test_clock(CLOCK_TAI);
- if (ret)
goto out;+#endif
+#ifdef CLOCK_REALTIME_COARSE
- ret = vdso_test_clock(CLOCK_REALTIME_COARSE);
- if (ret)
goto out;+#endif
+#ifdef CLOCK_MONOTONIC
- ret = vdso_test_clock(CLOCK_MONOTONIC);
- if (ret)
goto out;+#endif
+#ifdef CLOCK_MONOTONIC_RAW
- ret = vdso_test_clock(CLOCK_MONOTONIC_RAW);
- if (ret)
goto out;+#endif
+#ifdef CLOCK_MONOTONIC_COARSE
- ret = vdso_test_clock(CLOCK_MONOTONIC_COARSE);
- if (ret)
goto out;+#endif
+#endif
+out:
- return ret;
+}
Hi Christophe,
thank you for your review.
On 22/05/2019 12:50, Christophe Leroy wrote:
Le 22/05/2019 à 13:07, Vincenzo Frascino a écrit :
The current version of the multiarch vDSO selftest verifies only gettimeofday.
Extend the vDSO selftest to clock_getres, to verify that the syscall and the vDSO library function return the same information.
The extension has been used to verify the hrtimer_resoltion fix.
Cc: Shuah Khan shuah@kernel.org Signed-off-by: Vincenzo Frascino vincenzo.frascino@arm.com
Note: This patch is independent from the others in this series, hence it can be merged singularly by the kselftest maintainers.
tools/testing/selftests/vDSO/Makefile | 2 + .../selftests/vDSO/vdso_clock_getres.c | 137 ++++++++++++++++++ 2 files changed, 139 insertions(+) create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile index 9e03d61f52fd..d5c5bfdf1ac1 100644 --- a/tools/testing/selftests/vDSO/Makefile +++ b/tools/testing/selftests/vDSO/Makefile @@ -5,6 +5,7 @@ uname_M := $(shell uname -m 2>/dev/null || echo not) ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/) TEST_GEN_PROGS := $(OUTPUT)/vdso_test +TEST_GEN_PROGS += $(OUTPUT)/vdso_clock_getres ifeq ($(ARCH),x86) TEST_GEN_PROGS += $(OUTPUT)/vdso_standalone_test_x86 endif @@ -18,6 +19,7 @@ endif all: $(TEST_GEN_PROGS) $(OUTPUT)/vdso_test: parse_vdso.c vdso_test.c +$(OUTPUT)/vdso_clock_getres: vdso_clock_getres.c $(OUTPUT)/vdso_standalone_test_x86: vdso_standalone_test_x86.c parse_vdso.c $(CC) $(CFLAGS) $(CFLAGS_vdso_standalone_test_x86) \ vdso_standalone_test_x86.c parse_vdso.c \ diff --git a/tools/testing/selftests/vDSO/vdso_clock_getres.c b/tools/testing/selftests/vDSO/vdso_clock_getres.c new file mode 100644 index 000000000000..341a9bc34ffc --- /dev/null +++ b/tools/testing/selftests/vDSO/vdso_clock_getres.c @@ -0,0 +1,137 @@ +// SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note +/*
- vdso_clock_getres.c: Sample code to test clock_getres.
- Copyright (c) 2019 Arm Ltd.
- Compile with:
- gcc -std=gnu99 vdso_clock_getres.c
- Tested on ARM, ARM64, MIPS32, x86 (32-bit and 64-bit),
- Power (32-bit and 64-bit), S390x (32-bit and 64-bit).
- Might work on other architectures.
- */
+#define _GNU_SOURCE +#include <elf.h> +#include <err.h> +#include <fcntl.h> +#include <stdint.h> +#include <stdio.h> +#include <stdlib.h> +#include <time.h> +#include <sys/auxv.h> +#include <sys/mman.h> +#include <sys/time.h> +#include <unistd.h> +#include <sys/syscall.h>
+#include "../kselftest.h"
+static long syscall_clock_getres(clockid_t _clkid, struct timespec *_ts) +{
- long ret;
- ret = syscall(SYS_clock_getres, _clkid, _ts);
- return ret;
+}
+const char *vdso_clock_name[12] = {
- "CLOCK_REALTIME",
- "CLOCK_MONOTONIC",
- "CLOCK_PROCESS_CPUTIME_ID",
- "CLOCK_THREAD_CPUTIME_ID",
- "CLOCK_MONOTONIC_RAW",
- "CLOCK_REALTIME_COARSE",
- "CLOCK_MONOTONIC_COARSE",
- "CLOCK_BOOTTIME",
- "CLOCK_REALTIME_ALARM",
- "CLOCK_BOOTTIME_ALARM",
- "CLOCK_SGI_CYCLE",
- "CLOCK_TAI",
+};
+/*
- This function calls clock_getres in vdso and by system call
- with different values for clock_id.
- Example of output:
- clock_id: CLOCK_REALTIME [PASS]
- clock_id: CLOCK_BOOTTIME [PASS]
- clock_id: CLOCK_TAI [PASS]
- clock_id: CLOCK_REALTIME_COARSE [PASS]
- clock_id: CLOCK_MONOTONIC [PASS]
- clock_id: CLOCK_MONOTONIC_RAW [PASS]
- clock_id: CLOCK_MONOTONIC_COARSE [PASS]
- */
+static inline int vdso_test_clock(unsigned int clock_id) +{
- struct timespec x, y;
- printf("clock_id: %s", vdso_clock_name[clock_id]);
- clock_getres(clock_id, &x);
- syscall_clock_getres(clock_id, &y);
- if ((x.tv_sec != y.tv_sec) || (x.tv_sec != y.tv_sec)) {
printf(" [FAIL]\n");return KSFT_FAIL;- }
- printf(" [PASS]\n");
- return 0;
+}
+int main(int argc, char **argv) +{
- int ret;
+#if _POSIX_TIMERS > 0
+#ifdef CLOCK_REALTIME
Why do you need that #ifdef and all the ones below ?
CLOCK_REALTIME (and others) is defined in include/uapi/linux/time.h, so it should be there when you build the test, shouldn't it ?
In implementing this test I followed what the man page for clock_gettime(2) defines in terms of availability of the timers. Since I do not know how old are the userspace headers, I think it is a good idea checking that the clocks are defined before trying to use them.
- ret = vdso_test_clock(CLOCK_REALTIME);
- if (ret)
goto out;Why that goto ? Nothing is done at out, so a 'return ret' would be better I think.
Agree, thanks for pointing this out. Will fix in v4.
And do we really want to stop at first failure ? Wouldn't it be better to run all the tests regardless ?
The test is supposed to fail if one of the sub-tests fails, hence once the first fails doesn't seem convenient running the others, because we already know the result.
Christophe
+#endif
+#ifdef CLOCK_BOOTTIME
- ret = vdso_test_clock(CLOCK_BOOTTIME);
- if (ret)
goto out;+#endif
+#ifdef CLOCK_TAI
- ret = vdso_test_clock(CLOCK_TAI);
- if (ret)
goto out;+#endif
+#ifdef CLOCK_REALTIME_COARSE
- ret = vdso_test_clock(CLOCK_REALTIME_COARSE);
- if (ret)
goto out;+#endif
+#ifdef CLOCK_MONOTONIC
- ret = vdso_test_clock(CLOCK_MONOTONIC);
- if (ret)
goto out;+#endif
+#ifdef CLOCK_MONOTONIC_RAW
- ret = vdso_test_clock(CLOCK_MONOTONIC_RAW);
- if (ret)
goto out;+#endif
+#ifdef CLOCK_MONOTONIC_COARSE
- ret = vdso_test_clock(CLOCK_MONOTONIC_COARSE);
- if (ret)
goto out;+#endif
+#endif
+out:
- return ret;
+}
Hi Christophe,
thank you for your review.
On 22/05/2019 12:50, Christophe Leroy wrote:
Le 22/05/2019 à 13:07, Vincenzo Frascino a écrit :
The current version of the multiarch vDSO selftest verifies only gettimeofday.
Extend the vDSO selftest to clock_getres, to verify that the syscall and the vDSO library function return the same information.
The extension has been used to verify the hrtimer_resoltion fix.
Cc: Shuah Khan shuah@kernel.org Signed-off-by: Vincenzo Frascino vincenzo.frascino@arm.com
Note: This patch is independent from the others in this series, hence it can be merged singularly by the kselftest maintainers.
tools/testing/selftests/vDSO/Makefile | 2 + .../selftests/vDSO/vdso_clock_getres.c | 137 ++++++++++++++++++ 2 files changed, 139 insertions(+) create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile index 9e03d61f52fd..d5c5bfdf1ac1 100644 --- a/tools/testing/selftests/vDSO/Makefile +++ b/tools/testing/selftests/vDSO/Makefile @@ -5,6 +5,7 @@ uname_M := $(shell uname -m 2>/dev/null || echo not) ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/) TEST_GEN_PROGS := $(OUTPUT)/vdso_test +TEST_GEN_PROGS += $(OUTPUT)/vdso_clock_getres ifeq ($(ARCH),x86) TEST_GEN_PROGS += $(OUTPUT)/vdso_standalone_test_x86 endif @@ -18,6 +19,7 @@ endif all: $(TEST_GEN_PROGS) $(OUTPUT)/vdso_test: parse_vdso.c vdso_test.c +$(OUTPUT)/vdso_clock_getres: vdso_clock_getres.c $(OUTPUT)/vdso_standalone_test_x86: vdso_standalone_test_x86.c parse_vdso.c $(CC) $(CFLAGS) $(CFLAGS_vdso_standalone_test_x86) \ vdso_standalone_test_x86.c parse_vdso.c \ diff --git a/tools/testing/selftests/vDSO/vdso_clock_getres.c b/tools/testing/selftests/vDSO/vdso_clock_getres.c new file mode 100644 index 000000000000..341a9bc34ffc --- /dev/null +++ b/tools/testing/selftests/vDSO/vdso_clock_getres.c @@ -0,0 +1,137 @@ +// SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note +/*
- vdso_clock_getres.c: Sample code to test clock_getres.
- Copyright (c) 2019 Arm Ltd.
- Compile with:
- gcc -std=gnu99 vdso_clock_getres.c
- Tested on ARM, ARM64, MIPS32, x86 (32-bit and 64-bit),
- Power (32-bit and 64-bit), S390x (32-bit and 64-bit).
- Might work on other architectures.
- */
+#define _GNU_SOURCE +#include <elf.h> +#include <err.h> +#include <fcntl.h> +#include <stdint.h> +#include <stdio.h> +#include <stdlib.h> +#include <time.h> +#include <sys/auxv.h> +#include <sys/mman.h> +#include <sys/time.h> +#include <unistd.h> +#include <sys/syscall.h>
+#include "../kselftest.h"
+static long syscall_clock_getres(clockid_t _clkid, struct timespec *_ts) +{
- long ret;
- ret = syscall(SYS_clock_getres, _clkid, _ts);
- return ret;
+}
+const char *vdso_clock_name[12] = {
- "CLOCK_REALTIME",
- "CLOCK_MONOTONIC",
- "CLOCK_PROCESS_CPUTIME_ID",
- "CLOCK_THREAD_CPUTIME_ID",
- "CLOCK_MONOTONIC_RAW",
- "CLOCK_REALTIME_COARSE",
- "CLOCK_MONOTONIC_COARSE",
- "CLOCK_BOOTTIME",
- "CLOCK_REALTIME_ALARM",
- "CLOCK_BOOTTIME_ALARM",
- "CLOCK_SGI_CYCLE",
- "CLOCK_TAI",
+};
+/*
- This function calls clock_getres in vdso and by system call
- with different values for clock_id.
- Example of output:
- clock_id: CLOCK_REALTIME [PASS]
- clock_id: CLOCK_BOOTTIME [PASS]
- clock_id: CLOCK_TAI [PASS]
- clock_id: CLOCK_REALTIME_COARSE [PASS]
- clock_id: CLOCK_MONOTONIC [PASS]
- clock_id: CLOCK_MONOTONIC_RAW [PASS]
- clock_id: CLOCK_MONOTONIC_COARSE [PASS]
- */
+static inline int vdso_test_clock(unsigned int clock_id) +{
- struct timespec x, y;
- printf("clock_id: %s", vdso_clock_name[clock_id]);
- clock_getres(clock_id, &x);
- syscall_clock_getres(clock_id, &y);
- if ((x.tv_sec != y.tv_sec) || (x.tv_sec != y.tv_sec)) {
printf(" [FAIL]\n");return KSFT_FAIL;- }
- printf(" [PASS]\n");
- return 0;
+}
+int main(int argc, char **argv) +{
- int ret;
+#if _POSIX_TIMERS > 0
+#ifdef CLOCK_REALTIME
Why do you need that #ifdef and all the ones below ?
CLOCK_REALTIME (and others) is defined in include/uapi/linux/time.h, so it should be there when you build the test, shouldn't it ?
In implementing this test I tried to follow what the man page for clock_gettime(2) defines in terms of availability of the timers. Since I do not know how old are the userspace headers, I think it is a good idea checking that the clocks are defined before trying to use them.
- ret = vdso_test_clock(CLOCK_REALTIME);
- if (ret)
goto out;Why that goto ? Nothing is done at out, so a 'return ret' would be better I think.
Agree, thanks for pointing this out. Will fix in v4.
And do we really want to stop at first failure ? Wouldn't it be better to run all the tests regardless ?
The test is supposed to fail if one of the sub-tests fails, hence once the first fails doesn't seem convenient to run the others, because we already know the result.
Christophe
+#endif
+#ifdef CLOCK_BOOTTIME
- ret = vdso_test_clock(CLOCK_BOOTTIME);
- if (ret)
goto out;+#endif
+#ifdef CLOCK_TAI
- ret = vdso_test_clock(CLOCK_TAI);
- if (ret)
goto out;+#endif
+#ifdef CLOCK_REALTIME_COARSE
- ret = vdso_test_clock(CLOCK_REALTIME_COARSE);
- if (ret)
goto out;+#endif
+#ifdef CLOCK_MONOTONIC
- ret = vdso_test_clock(CLOCK_MONOTONIC);
- if (ret)
goto out;+#endif
+#ifdef CLOCK_MONOTONIC_RAW
- ret = vdso_test_clock(CLOCK_MONOTONIC_RAW);
- if (ret)
goto out;+#endif
+#ifdef CLOCK_MONOTONIC_COARSE
- ret = vdso_test_clock(CLOCK_MONOTONIC_COARSE);
- if (ret)
goto out;+#endif
+#endif
+out:
- return ret;
+}
linux-kselftest-mirror@lists.linaro.org
-
Christophe Leroy -
Vincenzo Frascino