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.
[1] https://marc.info/?l=linux-arm-kernel&m=155110381930196&w=2
Changes: -------- v2: - Rebased on 5.1-rc5. - Addressed review comments.
Cc: Catalin Marinas catalin.marinas@arm.com Cc: Will Deacon will.deacon@arm.com 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: Greentime Hu green.hu@gmail.com Cc: Vincent Chen deanbo422@gmail.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 (5): arm64: Fix vDSO clock_getres() powerpc: Fix vDSO clock_getres() s390: Fix vDSO clock_getres() nds32: Fix vDSO clock_getres() kselftest: Extend vDSO selftest to clock_getres
arch/arm64/include/asm/vdso_datapage.h | 1 + arch/arm64/kernel/asm-offsets.c | 2 +- arch/arm64/kernel/vdso.c | 2 + arch/arm64/kernel/vdso/gettimeofday.S | 22 ++-- arch/nds32/include/asm/vdso_datapage.h | 1 + arch/nds32/kernel/vdso.c | 1 + arch/nds32/kernel/vdso/gettimeofday.c | 4 +- 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 | 108 ++++++++++++++++++ 19 files changed, 159 insertions(+), 29 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 arm64 vdso implementation of clock_getres keeping a copy of hrtimer_resolution in vdso data and using that directly.
Cc: Catalin Marinas catalin.marinas@arm.com Cc: Will Deacon will.deacon@arm.com Signed-off-by: Vincenzo Frascino vincenzo.frascino@arm.com --- arch/arm64/include/asm/vdso_datapage.h | 1 + arch/arm64/kernel/asm-offsets.c | 2 +- arch/arm64/kernel/vdso.c | 2 ++ arch/arm64/kernel/vdso/gettimeofday.S | 22 +++++++++++----------- 4 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/include/asm/vdso_datapage.h b/arch/arm64/include/asm/vdso_datapage.h index 2b9a63771eda..f89263c8e11a 100644 --- a/arch/arm64/include/asm/vdso_datapage.h +++ b/arch/arm64/include/asm/vdso_datapage.h @@ -38,6 +38,7 @@ struct vdso_data { __u32 tz_minuteswest; /* Whacky timezone stuff */ __u32 tz_dsttime; __u32 use_syscall; + __u32 hrtimer_res; };
#endif /* !__ASSEMBLY__ */ diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c index 7f40dcbdd51d..e10e2a5d9ddc 100644 --- a/arch/arm64/kernel/asm-offsets.c +++ b/arch/arm64/kernel/asm-offsets.c @@ -94,7 +94,7 @@ int main(void) DEFINE(CLOCK_REALTIME, CLOCK_REALTIME); DEFINE(CLOCK_MONOTONIC, CLOCK_MONOTONIC); DEFINE(CLOCK_MONOTONIC_RAW, CLOCK_MONOTONIC_RAW); - DEFINE(CLOCK_REALTIME_RES, MONOTONIC_RES_NSEC); + DEFINE(CLOCK_REALTIME_RES, offsetof(struct vdso_data, hrtimer_res)); DEFINE(CLOCK_REALTIME_COARSE, CLOCK_REALTIME_COARSE); DEFINE(CLOCK_MONOTONIC_COARSE,CLOCK_MONOTONIC_COARSE); DEFINE(CLOCK_COARSE_RES, LOW_RES_NSEC); diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c index 2d419006ad43..5f5759d51c33 100644 --- a/arch/arm64/kernel/vdso.c +++ b/arch/arm64/kernel/vdso.c @@ -245,6 +245,8 @@ void update_vsyscall(struct timekeeper *tk) vdso_data->cs_shift = tk->tkr_mono.shift; }
+ vdso_data->hrtimer_res = hrtimer_resolution; + smp_wmb(); ++vdso_data->tb_seq_count; } diff --git a/arch/arm64/kernel/vdso/gettimeofday.S b/arch/arm64/kernel/vdso/gettimeofday.S index c39872a7b03c..e2e9dfe9ba4a 100644 --- a/arch/arm64/kernel/vdso/gettimeofday.S +++ b/arch/arm64/kernel/vdso/gettimeofday.S @@ -296,32 +296,32 @@ ENDPROC(__kernel_clock_gettime) /* int __kernel_clock_getres(clockid_t clock_id, struct timespec *res); */ ENTRY(__kernel_clock_getres) .cfi_startproc + adr vdso_data, _vdso_data cmp w0, #CLOCK_REALTIME ccmp w0, #CLOCK_MONOTONIC, #0x4, ne ccmp w0, #CLOCK_MONOTONIC_RAW, #0x4, ne - b.ne 1f + b.ne 2f
- ldr x2, 5f - b 2f -1: +1: /* Get hrtimer_res */ + ldr x2, [vdso_data, #CLOCK_REALTIME_RES] + b 3f +2: cmp w0, #CLOCK_REALTIME_COARSE ccmp w0, #CLOCK_MONOTONIC_COARSE, #0x4, ne - b.ne 4f + b.ne 5f ldr x2, 6f -2: - cbz x1, 3f +3: + cbz x1, 4f stp xzr, x2, [x1]
-3: /* res == NULL. */ +4: /* res == NULL. */ mov w0, wzr ret
-4: /* Syscall fallback. */ +5: /* Syscall fallback. */ mov x8, #__NR_clock_getres svc #0 ret -5: - .quad CLOCK_REALTIME_RES 6: .quad CLOCK_COARSE_RES .cfi_endproc
On Tue, Apr 16, 2019 at 05:14:30PM +0100, Vincenzo Frascino wrote:
diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c index 2d419006ad43..5f5759d51c33 100644 --- a/arch/arm64/kernel/vdso.c +++ b/arch/arm64/kernel/vdso.c @@ -245,6 +245,8 @@ void update_vsyscall(struct timekeeper *tk) vdso_data->cs_shift = tk->tkr_mono.shift; }
- vdso_data->hrtimer_res = hrtimer_resolution;
This should be a WRITE_ONCE(), just in case.
- smp_wmb(); ++vdso_data->tb_seq_count;
} diff --git a/arch/arm64/kernel/vdso/gettimeofday.S b/arch/arm64/kernel/vdso/gettimeofday.S index c39872a7b03c..e2e9dfe9ba4a 100644 --- a/arch/arm64/kernel/vdso/gettimeofday.S +++ b/arch/arm64/kernel/vdso/gettimeofday.S @@ -296,32 +296,32 @@ ENDPROC(__kernel_clock_gettime) /* int __kernel_clock_getres(clockid_t clock_id, struct timespec *res); */ ENTRY(__kernel_clock_getres) .cfi_startproc
- adr vdso_data, _vdso_data cmp w0, #CLOCK_REALTIME ccmp w0, #CLOCK_MONOTONIC, #0x4, ne ccmp w0, #CLOCK_MONOTONIC_RAW, #0x4, ne
- b.ne 1f
- b.ne 2f
- ldr x2, 5f
- b 2f
-1: +1: /* Get hrtimer_res */
- ldr x2, [vdso_data, #CLOCK_REALTIME_RES]
And here we need an "ldr w2, ..." since hrtimer_res is u32.
With the above (which Will can fix up):
Reviewed-by: Catalin Marinas catalin.marinas@arm.com
On Tue, Apr 16, 2019 at 05:24:33PM +0100, Catalin Marinas wrote:
On Tue, Apr 16, 2019 at 05:14:30PM +0100, Vincenzo Frascino wrote:
diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c index 2d419006ad43..5f5759d51c33 100644 --- a/arch/arm64/kernel/vdso.c +++ b/arch/arm64/kernel/vdso.c @@ -245,6 +245,8 @@ void update_vsyscall(struct timekeeper *tk) vdso_data->cs_shift = tk->tkr_mono.shift; }
- vdso_data->hrtimer_res = hrtimer_resolution;
This should be a WRITE_ONCE(), just in case.
- smp_wmb(); ++vdso_data->tb_seq_count;
} diff --git a/arch/arm64/kernel/vdso/gettimeofday.S b/arch/arm64/kernel/vdso/gettimeofday.S index c39872a7b03c..e2e9dfe9ba4a 100644 --- a/arch/arm64/kernel/vdso/gettimeofday.S +++ b/arch/arm64/kernel/vdso/gettimeofday.S @@ -296,32 +296,32 @@ ENDPROC(__kernel_clock_gettime) /* int __kernel_clock_getres(clockid_t clock_id, struct timespec *res); */ ENTRY(__kernel_clock_getres) .cfi_startproc
- adr vdso_data, _vdso_data cmp w0, #CLOCK_REALTIME ccmp w0, #CLOCK_MONOTONIC, #0x4, ne ccmp w0, #CLOCK_MONOTONIC_RAW, #0x4, ne
- b.ne 1f
- b.ne 2f
- ldr x2, 5f
- b 2f
-1: +1: /* Get hrtimer_res */
- ldr x2, [vdso_data, #CLOCK_REALTIME_RES]
And here we need an "ldr w2, ..." since hrtimer_res is u32.
With the above (which Will can fix up):
Reviewed-by: Catalin Marinas catalin.marinas@arm.com
Applied, with the above and a few extra cleanups.
Will
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.
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 --- 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 86a61e5f8285..52e4b98a8492 100644 --- a/arch/powerpc/kernel/asm-offsets.c +++ b/arch/powerpc/kernel/asm-offsets.c @@ -383,6 +383,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); @@ -413,7 +414,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 bc0503ef9c9c..62c04a6746d8 100644 --- a/arch/powerpc/kernel/time.c +++ b/arch/powerpc/kernel/time.c @@ -955,6 +955,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
Le 16/04/2019 à 18:14, Vincenzo Frascino a écrit :
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.
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
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 86a61e5f8285..52e4b98a8492 100644 --- a/arch/powerpc/kernel/asm-offsets.c +++ b/arch/powerpc/kernel/asm-offsets.c @@ -383,6 +383,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);
@@ -413,7 +414,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 bc0503ef9c9c..62c04a6746d8 100644 --- a/arch/powerpc/kernel/time.c +++ b/arch/powerpc/kernel/time.c @@ -955,6 +955,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 --- 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 164bec175628..36db4a9ee703 100644 --- a/arch/s390/kernel/asm-offsets.c +++ b/arch/s390/kernel/asm-offsets.c @@ -75,6 +75,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); @@ -86,7 +87,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
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 nds32 vdso implementation of clock_getres keeping a copy of hrtimer_resolution in vdso data and using that directly.
Cc: Greentime Hu green.hu@gmail.com Cc: Vincent Chen deanbo422@gmail.com Signed-off-by: Vincenzo Frascino vincenzo.frascino@arm.com --- arch/nds32/include/asm/vdso_datapage.h | 1 + arch/nds32/kernel/vdso.c | 1 + arch/nds32/kernel/vdso/gettimeofday.c | 4 +++- 3 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/nds32/include/asm/vdso_datapage.h b/arch/nds32/include/asm/vdso_datapage.h index 79db5a12ca5e..34d80548297f 100644 --- a/arch/nds32/include/asm/vdso_datapage.h +++ b/arch/nds32/include/asm/vdso_datapage.h @@ -20,6 +20,7 @@ struct vdso_data { u32 xtime_clock_sec; /* CLOCK_REALTIME - seconds */ u32 cs_mult; /* clocksource multiplier */ u32 cs_shift; /* Cycle to nanosecond divisor (power of two) */ + u32 hrtimer_res; /* hrtimer resolution */
u64 cs_cycle_last; /* last cycle value */ u64 cs_mask; /* clocksource mask */ diff --git a/arch/nds32/kernel/vdso.c b/arch/nds32/kernel/vdso.c index 016f15891f6d..90bcae6f8554 100644 --- a/arch/nds32/kernel/vdso.c +++ b/arch/nds32/kernel/vdso.c @@ -220,6 +220,7 @@ void update_vsyscall(struct timekeeper *tk) vdso_data->xtime_coarse_sec = tk->xtime_sec; vdso_data->xtime_coarse_nsec = tk->tkr_mono.xtime_nsec >> tk->tkr_mono.shift; + vdso_data->hrtimer_res = hrtimer_resolution; vdso_write_end(vdso_data); }
diff --git a/arch/nds32/kernel/vdso/gettimeofday.c b/arch/nds32/kernel/vdso/gettimeofday.c index 038721af40e3..b02581891c33 100644 --- a/arch/nds32/kernel/vdso/gettimeofday.c +++ b/arch/nds32/kernel/vdso/gettimeofday.c @@ -208,6 +208,8 @@ static notrace int clock_getres_fallback(clockid_t _clk_id,
notrace int __vdso_clock_getres(clockid_t clk_id, struct timespec *res) { + struct vdso_data *vdata = __get_datapage(); + if (res == NULL) return 0; switch (clk_id) { @@ -215,7 +217,7 @@ notrace int __vdso_clock_getres(clockid_t clk_id, struct timespec *res) case CLOCK_MONOTONIC: case CLOCK_MONOTONIC_RAW: res->tv_sec = 0; - res->tv_nsec = CLOCK_REALTIME_RES; + res->tv_nsec = vdata->hrtimer_res; break; case CLOCK_REALTIME_COARSE: case CLOCK_MONOTONIC_COARSE:
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 --- tools/testing/selftests/vDSO/Makefile | 2 + .../selftests/vDSO/vdso_clock_getres.c | 108 ++++++++++++++++++ 2 files changed, 110 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..b1b9652972eb --- /dev/null +++ b/tools/testing/selftests/vDSO/vdso_clock_getres.c @@ -0,0 +1,108 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * 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", +}; + +/* + * Macro to call clock_getres in vdso and by system call + * with different values for clock_id. + */ +#define vdso_test_clock(clock_id) \ +do { \ + 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_SKIP; \ + } else { \ + printf(" [PASS]\n"); \ + } \ +} while (0) + +int main(int argc, char **argv) +{ + +#if _POSIX_TIMERS > 0 + +#ifdef CLOCK_REALTIME + vdso_test_clock(CLOCK_REALTIME); +#endif + +#ifdef CLOCK_BOOTTIME + vdso_test_clock(CLOCK_BOOTTIME); +#endif + +#ifdef CLOCK_TAI + vdso_test_clock(CLOCK_TAI); +#endif + +#ifdef CLOCK_REALTIME_COARSE + vdso_test_clock(CLOCK_REALTIME_COARSE); +#endif + +#ifdef CLOCK_MONOTONIC + vdso_test_clock(CLOCK_MONOTONIC); +#endif + +#ifdef CLOCK_MONOTONIC_RAW + vdso_test_clock(CLOCK_MONOTONIC_RAW); +#endif + +#ifdef CLOCK_MONOTONIC_COARSE + vdso_test_clock(CLOCK_MONOTONIC_COARSE); +#endif + +#endif + + return 0; +}
On Tue, Apr 16, 2019 at 05:14:34PM +0100, Vincenzo Frascino wrote:
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
tools/testing/selftests/vDSO/Makefile | 2 + .../selftests/vDSO/vdso_clock_getres.c | 108 ++++++++++++++++++ 2 files changed, 110 insertions(+) create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
Assuming this will go via Shuah's tree.
Will
Hi Shuah,
I did not see this patch in 5.2-rc1 and I was wondering if there is anything I can do to help with upstraming it.
Please let me know.
Thanks, Vincenzo
On 16/04/2019 17:14, Vincenzo Frascino wrote:
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
tools/testing/selftests/vDSO/Makefile | 2 + .../selftests/vDSO/vdso_clock_getres.c | 108 ++++++++++++++++++ 2 files changed, 110 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..b1b9652972eb --- /dev/null +++ b/tools/testing/selftests/vDSO/vdso_clock_getres.c @@ -0,0 +1,108 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- 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",
+};
+/*
- Macro to call clock_getres in vdso and by system call
- with different values for clock_id.
- */
+#define vdso_test_clock(clock_id) \ +do { \
- 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_SKIP; \- } else { \
printf(" [PASS]\n"); \- } \
+} while (0)
+int main(int argc, char **argv) +{
+#if _POSIX_TIMERS > 0
+#ifdef CLOCK_REALTIME
- vdso_test_clock(CLOCK_REALTIME);
+#endif
+#ifdef CLOCK_BOOTTIME
- vdso_test_clock(CLOCK_BOOTTIME);
+#endif
+#ifdef CLOCK_TAI
- vdso_test_clock(CLOCK_TAI);
+#endif
+#ifdef CLOCK_REALTIME_COARSE
- vdso_test_clock(CLOCK_REALTIME_COARSE);
+#endif
+#ifdef CLOCK_MONOTONIC
- vdso_test_clock(CLOCK_MONOTONIC);
+#endif
+#ifdef CLOCK_MONOTONIC_RAW
- vdso_test_clock(CLOCK_MONOTONIC_RAW);
+#endif
+#ifdef CLOCK_MONOTONIC_COARSE
- vdso_test_clock(CLOCK_MONOTONIC_COARSE);
+#endif
+#endif
- return 0;
+}
On 5/21/19 10:11 AM, Vincenzo Frascino wrote:
Hi Shuah,
I did not see this patch in 5.2-rc1 and I was wondering if there is anything I can do to help with upstraming it.
Thanks for the ping.
Sorry about this. I saw that this part of a series and thought it depends on other patches and should go through another tree.
Totally missed that Will suggested that I take it though selftests.
My bad. I will get this in - I hope there are no dependencies with other trees.
thanks, -- Shuah
On 5/21/19 10:24 AM, shuah wrote:
On 5/21/19 10:11 AM, Vincenzo Frascino wrote:
Hi Shuah,
I did not see this patch in 5.2-rc1 and I was wondering if there is anything I can do to help with upstraming it.
Thanks for the ping.
Sorry about this. I saw that this part of a series and thought it depends on other patches and should go through another tree.
Totally missed that Will suggested that I take it though selftests.
My bad. I will get this in - I hope there are no dependencies with other trees.
thanks, -- Shuah
Can you take a look at the macro in this patch. It has checkpatch warn:
WARNING: Macros with flow control statements should be avoided
thanks, -- Shuah
Hi Shuah,
On 21/05/2019 18:33, shuah wrote:
On 5/21/19 10:24 AM, shuah wrote:
On 5/21/19 10:11 AM, Vincenzo Frascino wrote:
Hi Shuah,
I did not see this patch in 5.2-rc1 and I was wondering if there is anything I can do to help with upstraming it.
Thanks for the ping.
Sorry about this. I saw that this part of a series and thought it depends on other patches and should go through another tree.
Totally missed that Will suggested that I take it though selftests.
My bad. I will get this in - I hope there are no dependencies with other trees.
thanks, -- Shuah
Can you take a look at the macro in this patch. It has checkpatch warn:
WARNING: Macros with flow control statements should be avoided
Ok, I will fix this in v3.
thanks, -- Shuah
linux-kselftest-mirror@lists.linaro.org
-
Catalin Marinas -
Christophe Leroy -
shuah -
Vincenzo Frascino -
Will Deacon