[PATCH 0/5] Fix vDSO 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. 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. [1] https://marc.info/?l=linux-arm-kernel&m=155110381930196&w=2 Cc: Catalin Marinas Cc: Will Deacon Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: Michael Ellerman Cc: Martin Schwidefsky Cc: Heiko Carstens Cc: Greentime Hu Cc: Vincent Chen Cc: Shuah Khan Cc: Thomas Gleixner Cc: Arnd Bergmann Signed-off-by: Vincenzo Frascino 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 | 25 ++-- 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 | 22 ++-- arch/powerpc/kernel/vdso64/gettimeofday.S | 22 ++-- 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| 17 ++- arch/s390/kernel/vdso64/clock_getres.S| 15 ++- tools/testing/selftests/vDSO/Makefile | 2 + .../selftests/vDSO/vdso_clock_getres.c| 107 ++ 19 files changed, 191 insertions(+), 39 deletions(-) create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c -- 2.21.0
[PATCH 1/5] arm64: Fix vDSO 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 arm64 vdso implementation of clock_getres keeping a copy of
hrtimer_resolution in vdso data and using that directly.
Cc: Catalin Marinas
Cc: Will Deacon
Signed-off-by: Vincenzo Frascino
---
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 | 25 ++---
4 files changed, 18 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..47ba72345739 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..7a2cd2f8e13a 100644
--- a/arch/arm64/kernel/vdso/gettimeofday.S
+++ b/arch/arm64/kernel/vdso/gettimeofday.S
@@ -296,32 +296,35 @@ 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
ccmpw0, #CLOCK_MONOTONIC, #0x4, ne
ccmpw0, #CLOCK_MONOTONIC_RAW, #0x4, ne
- b.ne1f
+ b.ne2f
- ldr x2, 5f
- b 2f
-1:
+1: /* Get hrtimer_res */
+ seqcnt_acquire
+ syscall_check fail=5f
+ ldr x2, [vdso_data, #CLOCK_REALTIME_RES]
+ seqcnt_check fail=1b
+ b 3f
+2:
cmp w0, #CLOCK_REALTIME_COARSE
ccmpw0, #CLOCK_MONOTONIC_COARSE, #0x4, ne
- b.ne4f
+ b.ne5f
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
--
2.21.0
[PATCH 2/5] powerpc: Fix vDSO 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 powerpc vdso implementation of clock_getres keeping a copy of
hrtimer_resolution in vdso data and using that directly.
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: Michael Ellerman
Signed-off-by: Vincenzo Frascino
---
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 | 22 +++---
arch/powerpc/kernel/vdso64/gettimeofday.S | 22 +++---
5 files changed, 34 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/include/asm/vdso_datapage.h
b/arch/powerpc/include/asm/vdso_datapage.h
index 1afe90ade595..4ae43fc77fe9 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;
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_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 1e0bc5955a40..b21630079496 100644
--- a/arch/powerpc/kernel/vdso32/gettimeofday.S
+++ b/arch/powerpc/kernel/vdso32/gettimeofday.S
@@ -160,14 +160,21 @@ V_FUNCTION_BEGIN(__kernel_clock_getres)
crorcr0*4+eq,cr0*4+eq,cr1*4+eq
bne cr0,99f
- li r3,0
- cmpli cr0,r4,0
+ mflrr12
+ .cfi_register lr,r12
+ mr r11,r4
+ bl __get_datapage@local
+ lwz r5,CLOCK_REALTIME_RES(r3)
+ li r4,0
+ cmplwi r11,0 /* check if res is NULL */
+ beq 1f
+
+ stw r4,TSPC32_TV_SEC(r11)
+ stw r5,TSPC32_TV_NSEC(r11)
+
+1: mtlrr12
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)
+ li r3,0
blr
/*
@@ -175,6 +182,7 @@ V_FUNCTION_BEGIN(__kernel_clock_getres)
*/
99:
li r0,__NR_clock_getres
+ .cfi_restore lr
sc
blr
.cfi_endproc
diff --git a/arch/powerpc/kernel/vdso64/gettimeofday.S
b/arch/powerpc/kernel/vdso64/gettimeofday.S
index a4ed9edfd5f0..a7e49bddd475 100644
--- a/arch/powerpc/kernel/vdso64/gettimeofday.S
+++ b/arch/pow
[PATCH 3/5] s390: Fix vDSO 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 s390 vdso implementation of clock_getres keeping a copy of
hrtimer_resolution in vdso data and using that directly.
Cc: Martin Schwidefsky
Cc: Heiko Carstens
Signed-off-by: Vincenzo Frascino
---
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 | 17 -
arch/s390/kernel/vdso64/clock_getres.S | 15 ++-
5 files changed, 25 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_nsec0x60 */
__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..00fc50ad228c 100644
--- a/arch/s390/kernel/vdso32/clock_getres.S
+++ b/arch/s390/kernel/vdso32/clock_getres.S
@@ -18,20 +18,27 @@
__kernel_clock_getres:
CFI_STARTPROC
basr%r1,0
- la %r1,4f-.(%r1)
+10:al %r1,4f-10b(%r1)
+11:l %r4,__VDSO_UPD_COUNT+4(%r1) /* load update counter */
+ tml %r4,0x0001 /* pending update ? loop */
+ jnz 11b
+ l %r0,__VDSO_CLOCK_REALTIME_RES(%r1)
+ cl %r4,__VDSO_UPD_COUNT+4(%r1) /* check update counter */
+ jne 11b
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 +46,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..1400a8df9802 100644
--- a/arch/s390/kernel/vdso64/clock_getres.S
+++ b/arch/s390/kernel/vdso64/clock_getres.S
@@ -17,12 +17,19 @@
.type __kernel_clock_getres,@function
__kernel_clock_getres:
CFI_STARTPROC
- larl%r1,4f
+ larl%r1,3
[PATCH 4/5] nds32: Fix vDSO 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
Cc: Vincent Chen
Signed-off-by: Vincenzo Frascino
---
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:
--
2.21.0
[PATCH 5/5] kselftest: Extend vDSO selftest to 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
Signed-off-by: Vincenzo Frascino
---
tools/testing/selftests/vDSO/Makefile | 2 +
.../selftests/vDSO/vdso_clock_getres.c| 107 ++
2 files changed, 109 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 ..589949f6ca90
--- /dev/null
+++ b/tools/testing/selftests/vDSO/vdso_clock_getres.c
@@ -0,0 +1,107 @@
+// 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.
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#define _GNU_SOURCE
+#include
+#include
+
+#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;
+}
--
2.21.0
Re: [PATCH 2/5] powerpc: Fix vDSO clock_getres()
Hi Christophe,
thank you for your review.
On 02/04/2019 06:54, Christophe Leroy wrote:
>
>
> On 04/01/2019 11:51 AM, Vincenzo Frascino wrote:
>> 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
>> Cc: Paul Mackerras
>> Cc: Michael Ellerman
>> Signed-off-by: Vincenzo Frascino
>> ---
>> arch/powerpc/include/asm/vdso_datapage.h | 2 ++
>
> Conflicts with commit b5b4453e7912 ("powerpc/vdso64: Fix CLOCK_MONOTONIC
> inconsistencies across Y2038")
>
Thanks for pointing this out, I will rebase my code on top of the latest version
before reissuing v2.
...
--
Regards,
Vincenzo
Re: [PATCH 2/5] powerpc: Fix vDSO clock_getres()
On 02/04/2019 07:14, Christophe Leroy wrote: > > > On 04/01/2019 11:51 AM, Vincenzo Frascino wrote: >> 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 >> Cc: Paul Mackerras >> Cc: Michael Ellerman >> Signed-off-by: Vincenzo Frascino >> --- >> 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 | 22 +++--- >> arch/powerpc/kernel/vdso64/gettimeofday.S | 22 +++--- >> 5 files changed, 34 insertions(+), 15 deletions(-) >> > > [...] > >> diff --git a/arch/powerpc/kernel/vdso32/gettimeofday.S >> b/arch/powerpc/kernel/vdso32/gettimeofday.S >> index 1e0bc5955a40..b21630079496 100644 >> --- a/arch/powerpc/kernel/vdso32/gettimeofday.S >> +++ b/arch/powerpc/kernel/vdso32/gettimeofday.S >> @@ -160,14 +160,21 @@ V_FUNCTION_BEGIN(__kernel_clock_getres) >> crorcr0*4+eq,cr0*4+eq,cr1*4+eq >> bne cr0,99f >> >> -li r3,0 >> -cmpli cr0,r4,0 >> +mflrr12 >> + .cfi_register lr,r12 >> +mr r11,r4 >> +bl __get_datapage@local >> +lwz r5,CLOCK_REALTIME_RES(r3) >> +li r4,0 >> +cmplwi r11,0 /* check if res is NULL */ >> +beq 1f >> + >> +stw r4,TSPC32_TV_SEC(r11) >> +stw r5,TSPC32_TV_NSEC(r11) >> + >> +1: mtlrr12 >> 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) >> +li r3,0 >> blr > > The above can be done simpler, see below > > @@ -160,12 +160,15 @@ V_FUNCTION_BEGIN(__kernel_clock_getres) > crorcr0*4+eq,cr0*4+eq,cr1*4+eq > bne cr0,99f > > + mflrr12 > + .cfi_register lr,r12 > + bl __get_datapage@local > + lwz r5,CLOCK_REALTIME_RES(r3) > + mtlrr12 > 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 > Thank you for this, I will update my code accordingly before posting v2. > Christophe > >> >> /* >> @@ -175,6 +182,7 @@ V_FUNCTION_BEGIN(__kernel_clock_getres) >> */ >> 99: >> li r0,__NR_clock_getres >> + .cfi_restore lr >> sc >> blr >> .cfi_endproc >> diff --git a/arch/powerpc/kernel/vdso64/gettimeofday.S >> b/arch/powerpc/kernel/vdso64/gettimeofday.S >> index a4ed9edfd5f0..a7e49bddd475 100644 >> --- a/arch/powerpc/kernel/vdso64/gettimeofday.S >> +++ b/arch/powerpc/kernel/vdso64/gettimeofday.S >> @@ -190,14 +190,21 @@ V_FUNCTION_BEGIN(__kernel_clock_getres) >> crorcr0*4+eq,cr0*4+eq,cr1*4+eq >> bne cr0,99f >> >> -li r3,0 >> -cmpldi cr0,r4,0 >> +mflrr12 >> + .cfi_register lr,r12 >> +mr r11, r4 >> +bl V_LOCAL_FUNC(__get_datapage) >> +lwz r5,CLOCK_REALTIME_RES(r3) >> +li r4,0 >> +cmpldi r11,0 /* check if res is NULL */ >> +beq 1f >> + >> +std r4,TSPC64_TV_SEC(r11) >> +std r5,TSPC64_TV_NSEC(r11) >> + >> +1: mtlrr12 >> 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) >> +li r3,0 >> blr > > The same type of simplification applies here too. > > Christophe > > >> >> /* >> @@ -205,6 +212,7 @@ V_FUNCTION_BEGIN(__kernel_clock_getres) >> */ >> 99: >> li r0,__NR_clock_getres >> + .cfi_restore lr >> sc >> blr >> .cfi_endproc >> -- Regards, Vincenzo
Re: [PATCH 3/5] s390: Fix vDSO clock_getres()
On 03/04/2019 11:06, Thomas Gleixner wrote: > On Wed, 3 Apr 2019, Martin Schwidefsky wrote: > >> On Mon, 1 Apr 2019 12:51:50 +0100 >> Vincenzo Frascino wrote: >> >>> 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 >>> Cc: Heiko Carstens >>> Signed-off-by: Vincenzo Frascino >>> --- >>> 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 | 17 - >>> arch/s390/kernel/vdso64/clock_getres.S | 15 ++- >>> 5 files changed, 25 insertions(+), 11 deletions(-) >> >> I tried this patch and in principle this works. In that regard >> Acked-by: Martin Schwidefsky >> >> But I wonder if the loop to check the update counter is really >> necessary. The hrtimer_resolution value can only changes once with >> the first call to hrtimer_switch_to_hres(). With the TOD clock >> as the only clock available on s390 we always have the ability >> to do hrtimer. It then all depends on the highres=[on|off] kernel >> parameter what value we get with clock_getres(). > > Yes, it's not changing after boot anymore. > > Thanks, > > tglx > Ok, I will remove the loop from both the implementations and post it with v2. -- Regards, Vincenzo
Re: [PATCH 1/5] arm64: Fix vDSO clock_getres()
Hi Catalin,
On 15/04/2019 18:35, Catalin Marinas wrote:
> On Mon, Apr 01, 2019 at 12:51:48PM +0100, Vincenzo Frascino wrote:
>> diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
>> index 2d419006ad43..47ba72345739 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..7a2cd2f8e13a 100644
>> --- a/arch/arm64/kernel/vdso/gettimeofday.S
>> +++ b/arch/arm64/kernel/vdso/gettimeofday.S
>> @@ -296,32 +296,35 @@ 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
>> ccmpw0, #CLOCK_MONOTONIC, #0x4, ne
>> ccmpw0, #CLOCK_MONOTONIC_RAW, #0x4, ne
>> -b.ne1f
>> +b.ne2f
>>
>> -ldr x2, 5f
>> -b 2f
>> -1:
>> +1: /* Get hrtimer_res */
>> +seqcnt_acquire
>> +syscall_check fail=5f
>> +ldr x2, [vdso_data, #CLOCK_REALTIME_RES]
>> +seqcnt_check fail=1b
>> +b 3f
>> +2:
>
> We talked briefly but I'm still confused why we need the fallback to the
> syscall here if archdata.vdso_direct is false. Is it because if the
> timer driver code sets vdso_direct to false, we don't don't support
> highres timers? If my understanding is correct, you may want to move the
> hrtimer_res setting in update_vsyscall() to the !use_syscall block.
>
Ok, so let me try to provide more details on what I mentioned yesterday:
- clock_getres syscall follows the rules of what defined in posix-timers.c
- based on the clock_id that, for this purpose, can be separated in coarse and
non-coarse calls either posix_get_coarse_res() or posix_get_hrtimer_res().
- if clock id is set to a coarse clock and posix_get_coarse_res() is invoked,
happens what follows:
static int posix_get_coarse_res(const clockid_t which_clock,
struct timespec64 *tp)
{
*tp = ktime_to_timespec64(KTIME_LOW_RES);
return 0;
}
Note that since CONFIG_1HZ seems not supported (jiffies.h) by the kernel in this
case we do not need rounding in our vDSO implementation.
- if clock id is set to non-coarse and posix_get_hrtimer_res() is invoked,
happens the following:
static int posix_get_hrtimer_res(clockid_t which_clock, struct timespec64 *tp)
{
tp->tv_sec = 0;
tp->tv_nsec = hrtimer_resolution;
return 0;
}
hrtimer_resolution can be high res or low res depending on the call of
hrtimer_switch_to_hres(). For us the only way to preserve the correct value is
to keep it in the vdso data page.
- The assembly code mimics exactly the same behaviour detailed above, with one
difference: the one related to the use_syscall parameter which is specific to
arm64.
The use_syscall parameter is set by arm_arch_timer and consumed by
update_vsyscall(). To mirror what update_vsyscall does in update_vsyscall() I
check "syscall_check fail=5f" in clock_getres vdso function.
Said that, even if functionally it is the same thing, I think it is logically
more correct to have hrtimer_res setting inside the !use_syscall block, hence I
am going to change it in the next iteration.
Please let me know your thoughts.
--
Regards,
Vincenzo
[PATCH v2 0/5] Fix vDSO 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. 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 Cc: Will Deacon Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: Michael Ellerman Cc: Martin Schwidefsky Cc: Heiko Carstens Cc: Greentime Hu Cc: Vincent Chen Cc: Shuah Khan Cc: Thomas Gleixner Cc: Arnd Bergmann Signed-off-by: Vincenzo Frascino 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 -- 2.21.0
[PATCH v2 1/5] arm64: Fix vDSO 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 arm64 vdso implementation of clock_getres keeping a copy of
hrtimer_resolution in vdso data and using that directly.
Cc: Catalin Marinas
Cc: Will Deacon
Signed-off-by: Vincenzo Frascino
---
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
ccmpw0, #CLOCK_MONOTONIC, #0x4, ne
ccmpw0, #CLOCK_MONOTONIC_RAW, #0x4, ne
- b.ne1f
+ b.ne2f
- 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
ccmpw0, #CLOCK_MONOTONIC_COARSE, #0x4, ne
- b.ne4f
+ b.ne5f
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
--
2.21.0
[PATCH v2 2/5] powerpc: Fix vDSO 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 powerpc vdso implementation of clock_getres keeping a copy of
hrtimer_resolution in vdso data and using that directly.
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: Michael Ellerman
Signed-off-by: Vincenzo Frascino
---
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)
crorcr0*4+eq,cr0*4+eq,cr1*4+eq
bne cr0,99f
+ mflrr12
+ .cfi_register lr,r12
+ bl __get_datapage@local
+ lwz r5,CLOCK_REALTIME_RES(r3)
+ mtlrr12
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)
crorcr0*4+eq,cr0*4+eq,cr1*4+eq
bne cr0,99f
+ mflrr12
+ .cfi_register lr,r12
+ bl V_LOCAL_FUNC(__get_datapage)
+ lwz r5,CLOCK_REALTIME_RES(r3)
+ mtlrr12
li r3,0
cmpldi cr0,r4,0
crclr cr0*
[PATCH v2 3/5] s390: Fix vDSO 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 s390 vdso implementation of clock_getres keeping a copy of
hrtimer_resolution in vdso data and using that directly.
Cc: Martin Schwidefsky
Cc: Heiko Carstens
Signed-off-by: Vincenzo Frascino
Acked-by: Martin Schwidefsky
---
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_nsec0x60 */
__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_R
[PATCH v2 4/5] nds32: Fix vDSO 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
Cc: Vincent Chen
Signed-off-by: Vincenzo Frascino
---
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:
--
2.21.0
[PATCH v2 5/5] kselftest: Extend vDSO selftest to 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
Signed-off-by: Vincenzo Frascino
---
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 ..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
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#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;
+}
--
2.21.0
Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
Hi Michael, I wanted to check with you if you had time to have a look at my new version (v5) of the patches with the fixed test, and if they are ready to be merged or if there is anything else I can do. Thanks and Regards, Vincenzo On 28/05/2019 12:57, Vincenzo Frascino wrote: > Hi Michael, > > thank you for your reply. > > On 28/05/2019 07:19, Michael Ellerman wrote: >> Vincenzo Frascino writes: >> >>> 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. >> >> This is passing for me even without patch 1 applied, shouldn't it fail >> without the fix? What am I missing? >> > > This is correct, because during the refactoring process I missed an "n" :) > > if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_sec·!=·y.tv_sec)) > > Should be: > > if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_nsec·!=·y.tv_nsec)) > > My mistake, I am going to fix the test and re-post v5 of this set. > > Without my patch if you pass "highres=off" to the kernel (as a command line > parameter) it leads to a broken implementation of clock_getres since the value > of CLOCK_REALTIME_RES does not change at runtime. > > Expected result (with highres=off): > > # uname -r > 5.2.0-rc2 > # ./vdso_clock_getres > clock_id: CLOCK_REALTIME [FAIL] > clock_id: CLOCK_BOOTTIME [PASS] > clock_id: CLOCK_TAI [PASS] > clock_id: CLOCK_REALTIME_COARSE [PASS] > clock_id: CLOCK_MONOTONIC [FAIL] > clock_id: CLOCK_MONOTONIC_RAW [PASS] > clock_id: CLOCK_MONOTONIC_COARSE [PASS] > > The reason of this behavior is that the only clocks supported by getres on > powerpc are CLOCK_REALTIME and CLOCK_MONOTONIC, the rest on the clocks use > always syscalls. > >> # uname -r >> 5.2.0-rc2-gcc-8.2.0 >> >> # ./vdso_clock_getres >> 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] >> >> cheers >> >>> Cc: Shuah Khan >>> Signed-off-by: Vincenzo Frascino >>> --- >>> >>> 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| 124 ++ >>> 2 files changed, 126 insertions(+) >>> create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c >
Re: [PATCH 15/22] arch: vdso: consolidate gettime prototypes
Hi Arnd, On 11/8/23 12:58, Arnd Bergmann wrote: > From: Arnd Bergmann > > The VDSO functions are defined as globals in the kernel sources but intended > to be called from userspace, so there is no need to declare them in a kernel > side header. > > Without a prototype, this now causes warnings such as > > arch/mips/vdso/vgettimeofday.c:14:5: error: no previous prototype for > '__vdso_clock_gettime' [-Werror=missing-prototypes] > arch/mips/vdso/vgettimeofday.c:28:5: error: no previous prototype for > '__vdso_gettimeofday' [-Werror=missing-prototypes] > arch/mips/vdso/vgettimeofday.c:36:5: error: no previous prototype for > '__vdso_clock_getres' [-Werror=missing-prototypes] > arch/mips/vdso/vgettimeofday.c:42:5: error: no previous prototype for > '__vdso_clock_gettime64' [-Werror=missing-prototypes] > arch/sparc/vdso/vclock_gettime.c:254:1: error: no previous prototype for > '__vdso_clock_gettime' [-Werror=missing-prototypes] > arch/sparc/vdso/vclock_gettime.c:282:1: error: no previous prototype for > '__vdso_clock_gettime_stick' [-Werror=missing-prototypes] > arch/sparc/vdso/vclock_gettime.c:307:1: error: no previous prototype for > '__vdso_gettimeofday' [-Werror=missing-prototypes] > arch/sparc/vdso/vclock_gettime.c:343:1: error: no previous prototype for > '__vdso_gettimeofday_stick' [-Werror=missing-prototypes] > > Most architectures have already added workarounds for these by adding > declarations somewhere, but since these are all compatible, we should > really just have one copy, with an #ifdef check for the 32-bit vs > 64-bit variant and use that everywhere. > I agree, it is a good idea to have a single header for this purpose. > Unfortunately, the sparc version is currently incompatible since > that never added support for __vdso_clock_gettime64() in 32-bit > userland. For the moment, I'm leaving this one out, as I can't > easily test it and it requires a larger rework. > > Signed-off-by: Arnd Bergmann Reviewed-by: Vincenzo Frascino > --- > arch/arm/include/asm/vdso.h | 5 - > arch/arm/vdso/vgettimeofday.c| 1 + > arch/arm64/kernel/vdso32/vgettimeofday.c | 1 + > arch/csky/kernel/vdso/vgettimeofday.c| 11 +-- > arch/loongarch/vdso/vgettimeofday.c | 7 +-- > arch/mips/vdso/vgettimeofday.c | 1 + > arch/riscv/kernel/vdso/vgettimeofday.c | 7 +-- > arch/x86/entry/vdso/vclock_gettime.c | 10 +- > arch/x86/include/asm/vdso/gettimeofday.h | 2 -- > arch/x86/um/vdso/um_vdso.c | 1 + > include/vdso/gettime.h | 23 +++ > 11 files changed, 31 insertions(+), 38 deletions(-) > create mode 100644 include/vdso/gettime.h > > diff --git a/arch/arm/include/asm/vdso.h b/arch/arm/include/asm/vdso.h > index 422c3afa806a..5b85889f82ee 100644 > --- a/arch/arm/include/asm/vdso.h > +++ b/arch/arm/include/asm/vdso.h > @@ -24,11 +24,6 @@ static inline void arm_install_vdso(struct mm_struct *mm, > unsigned long addr) > > #endif /* CONFIG_VDSO */ > > -int __vdso_clock_gettime(clockid_t clock, struct old_timespec32 *ts); > -int __vdso_clock_gettime64(clockid_t clock, struct __kernel_timespec *ts); > -int __vdso_gettimeofday(struct __kernel_old_timeval *tv, struct timezone > *tz); > -int __vdso_clock_getres(clockid_t clock_id, struct old_timespec32 *res); > - > #endif /* __ASSEMBLY__ */ > > #endif /* __KERNEL__ */ > diff --git a/arch/arm/vdso/vgettimeofday.c b/arch/arm/vdso/vgettimeofday.c > index a003beacac76..3554aa35f1ba 100644 > --- a/arch/arm/vdso/vgettimeofday.c > +++ b/arch/arm/vdso/vgettimeofday.c > @@ -8,6 +8,7 @@ > #include > #include > #include > +#include > > int __vdso_clock_gettime(clockid_t clock, >struct old_timespec32 *ts) > diff --git a/arch/arm64/kernel/vdso32/vgettimeofday.c > b/arch/arm64/kernel/vdso32/vgettimeofday.c > index 5acff29c5991..e23c7f4ef26b 100644 > --- a/arch/arm64/kernel/vdso32/vgettimeofday.c > +++ b/arch/arm64/kernel/vdso32/vgettimeofday.c > @@ -5,6 +5,7 @@ > * Copyright (C) 2018 ARM Limited > * > */ > +#include > > int __vdso_clock_gettime(clockid_t clock, >struct old_timespec32 *ts) > diff --git a/arch/csky/kernel/vdso/vgettimeofday.c > b/arch/csky/kernel/vdso/vgettimeofday.c > index c4831145eed5..55af30e83752 100644 > --- a/arch/csky/kernel/vdso/vgettimeofday.c > +++ b/arch/csky/kernel/vdso/vgettimeofday.c > @@ -2,36 +2,27 @@ > > #include > #include > +#include > > extern > -int __vdso_clock_gettime(clockid_t clock, > -
Re: [PATCH v2] vdso: Improve cmd_vdso_check to check all dynamic relocations
Hi Fangrui, Apologize for the delay, I totally missed that you had a new version of your patch since it was threaded with the old one. On 12/21/22 23:51, Fangrui Song wrote: > The actual intention is that no dynamic relocation exists. However, some > GNU ld ports produce unneeded R_*_NONE. (If a port fails to determine > the exact .rel[a].dyn size, the trailing zeros become R_*_NONE > relocations. E.g. ld's powerpc port recently fixed > https://sourceware.org/bugzilla/show_bug.cgi?id=29540) R_*_NONE are > generally no-op in the dynamic loaders. So just ignore them. > > With the change, we can remove ARCH_REL_TYPE_ABS. ARCH_REL_TYPE_ABS is a > bit misnomer as ports may check RELAVETIVE/GLOB_DAT/JUMP_SLOT which are > not called "absolute relocations". (The patch is motivated by the arm64 > port missing R_AARCH64_RELATIVE.) It makes sense to update the name, it started as "absolute relocations" but then it evolved into something else. A part that, did you perform any testing with the generated vDSO libraries? > > Signed-off-by: Fangrui Song > Reviewed-by: Christophe Leroy > --- > Change from v1: > * rebase after 8ac3b5cd3e0521d92f9755e90d140382fc292510 (lib/vdso: use "grep > -E" > instead of "egrep") > * change the commit message to mention an example GNU ld bug; no longer say > the > patch fixes a deprecated egrep use > --- > arch/arm/vdso/Makefile | 3 --- > arch/arm64/kernel/vdso/Makefile | 3 --- > arch/arm64/kernel/vdso32/Makefile | 3 --- > arch/csky/kernel/vdso/Makefile | 3 --- > arch/loongarch/vdso/Makefile | 3 --- > arch/mips/vdso/Makefile | 3 --- > arch/powerpc/kernel/vdso/Makefile | 1 - > arch/riscv/kernel/vdso/Makefile | 3 --- > arch/s390/kernel/vdso32/Makefile | 2 -- > arch/s390/kernel/vdso64/Makefile | 2 -- > arch/x86/entry/vdso/Makefile | 4 > lib/vdso/Makefile | 13 - > 12 files changed, 4 insertions(+), 39 deletions(-) > > diff --git a/arch/arm/vdso/Makefile b/arch/arm/vdso/Makefile > index a7ec06ce3785..e58197bba776 100644 > --- a/arch/arm/vdso/Makefile > +++ b/arch/arm/vdso/Makefile > @@ -1,8 +1,5 @@ > # SPDX-License-Identifier: GPL-2.0 > > -# Absolute relocation type $(ARCH_REL_TYPE_ABS) needs to be defined before > -# the inclusion of generic Makefile. > -ARCH_REL_TYPE_ABS := R_ARM_JUMP_SLOT|R_ARM_GLOB_DAT|R_ARM_ABS32 I would still add a comment here to say why we are including the generic Makefile to prevent that it gets accidentally removed (similar thing for every architecture touched by this patch). With that: Reviewed-by: Vincenzo Frascino # for vDSO, aarch64 Tested-by: Vincenzo Frascino # for aarch64 > include $(srctree)/lib/vdso/Makefile > > hostprogs := vdsomunge > diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile > index beaf9586338f..1f2427b13410 100644 > --- a/arch/arm64/kernel/vdso/Makefile > +++ b/arch/arm64/kernel/vdso/Makefile > @@ -6,9 +6,6 @@ > # Heavily based on the vDSO Makefiles for other archs. > # > > -# Absolute relocation type $(ARCH_REL_TYPE_ABS) needs to be defined before > -# the inclusion of generic Makefile. > -ARCH_REL_TYPE_ABS := R_AARCH64_JUMP_SLOT|R_AARCH64_GLOB_DAT|R_AARCH64_ABS64 > include $(srctree)/lib/vdso/Makefile > > obj-vdso := vgettimeofday.o note.o sigreturn.o > diff --git a/arch/arm64/kernel/vdso32/Makefile > b/arch/arm64/kernel/vdso32/Makefile > index f59bd1a4ead6..d014162c5c71 100644 > --- a/arch/arm64/kernel/vdso32/Makefile > +++ b/arch/arm64/kernel/vdso32/Makefile > @@ -3,9 +3,6 @@ > # Makefile for vdso32 > # > > -# Absolute relocation type $(ARCH_REL_TYPE_ABS) needs to be defined before > -# the inclusion of generic Makefile. > -ARCH_REL_TYPE_ABS := R_ARM_JUMP_SLOT|R_ARM_GLOB_DAT|R_ARM_ABS32 > include $(srctree)/lib/vdso/Makefile > > # Same as cc-*option, but using CC_COMPAT instead of CC > diff --git a/arch/csky/kernel/vdso/Makefile b/arch/csky/kernel/vdso/Makefile > index 0b6909f10667..86c8c4de1b0f 100644 > --- a/arch/csky/kernel/vdso/Makefile > +++ b/arch/csky/kernel/vdso/Makefile > @@ -1,8 +1,5 @@ > # SPDX-License-Identifier: GPL-2.0-only > > -# Absolute relocation type $(ARCH_REL_TYPE_ABS) needs to be defined before > -# the inclusion of generic Makefile. > -ARCH_REL_TYPE_ABS := R_CKCORE_ADDR32|R_CKCORE_JUMP_SLOT > include $(srctree)/lib/vdso/Makefile > > # Symbols present in the vdso > diff --git a/arch/loongarch/vdso/Makefile b/arch/loongarch/vdso/Makefile > index d89e2ac75f7b..1b2e0f149f55 100644 > --- a/arch/loongarch/vdso/Makefile > +++ b/arch/loongarch/vdso/Makefile > @@ -1,9 +1,6 @@ > # SPDX-License-Identifier: GPL-2.0 > #
Re: [v2 PATCH 0/3] arch: mm, vdso: consolidate PAGE_SIZE definition
On 06/03/2024 14:14, Arnd Bergmann wrote: > From: Arnd Bergmann > > Naresh noticed that the newly added usage of the PAGE_SIZE macro in > include/vdso/datapage.h introduced a build regression. I had an older > patch that I revived to have this defined through Kconfig rather than > through including asm/page.h, which is not allowed in vdso code. > > The vdso patch series now has a temporary workaround, but I still want to > get this into v6.9 so we can place the hack with CONFIG_PAGE_SIZE > in the vdso. > > I've applied this to the asm-generic tree already, please let me know if > there are still remaining issues. It's really close to the merge window > already, so I'd probably give this a few more days before I send a pull > request, or defer it to v6.10 if anything goes wrong. > > Sorry for the delay, I was still waiting to resolve the m68k question, > but there were no further replies in the end, so I kept my original > version. > > Changes from v1: > > - improve Kconfig help texts > - remove an extraneous line in hexagon > > Arnd > Thanks Arnd, looks good to me. Reviewed-by: Vincenzo Frascino
Re: [PATCH RESEND v1 1/4] lib/vdso: Mark do_hres_timens() and do_coarse_timens() __always_inline()
On 3/31/21 5:48 PM, Christophe Leroy wrote:
> In the same spirit as commit c966533f8c6c ("lib/vdso: Mark do_hres()
> and do_coarse() as __always_inline"), mark do_hres_timens() and
> do_coarse_timens() __always_inline.
>
> The measurement below in on a non timens process, ie on the fastest path.
>
> On powerpc32, without the patch:
>
> clock-gettime-monotonic-raw:vdso: 1155 nsec/call
> clock-gettime-monotonic-coarse:vdso: 813 nsec/call
> clock-gettime-monotonic:vdso: 1076 nsec/call
>
> With the patch:
>
> clock-gettime-monotonic-raw:vdso: 1100 nsec/call
> clock-gettime-monotonic-coarse:vdso: 667 nsec/call
> clock-gettime-monotonic:vdso: 1025 nsec/call
>
> Signed-off-by: Christophe Leroy
Reviewed-by: Vincenzo Frascino
> ---
> lib/vdso/gettimeofday.c | 16
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/lib/vdso/gettimeofday.c b/lib/vdso/gettimeofday.c
> index 2919f1698140..c6f6dee08746 100644
> --- a/lib/vdso/gettimeofday.c
> +++ b/lib/vdso/gettimeofday.c
> @@ -46,8 +46,8 @@ static inline bool vdso_cycles_ok(u64 cycles)
> #endif
>
> #ifdef CONFIG_TIME_NS
> -static int do_hres_timens(const struct vdso_data *vdns, clockid_t clk,
> - struct __kernel_timespec *ts)
> +static __always_inline int do_hres_timens(const struct vdso_data *vdns,
> clockid_t clk,
> + struct __kernel_timespec *ts)
> {
> const struct vdso_data *vd = __arch_get_timens_vdso_data();
> const struct timens_offset *offs = &vdns->offset[clk];
> @@ -97,8 +97,8 @@ static __always_inline const struct vdso_data
> *__arch_get_timens_vdso_data(void)
> return NULL;
> }
>
> -static int do_hres_timens(const struct vdso_data *vdns, clockid_t clk,
> - struct __kernel_timespec *ts)
> +static __always_inline int do_hres_timens(const struct vdso_data *vdns,
> clockid_t clk,
> + struct __kernel_timespec *ts)
> {
> return -EINVAL;
> }
> @@ -159,8 +159,8 @@ static __always_inline int do_hres(const struct vdso_data
> *vd, clockid_t clk,
> }
>
> #ifdef CONFIG_TIME_NS
> -static int do_coarse_timens(const struct vdso_data *vdns, clockid_t clk,
> - struct __kernel_timespec *ts)
> +static __always_inline int do_coarse_timens(const struct vdso_data *vdns,
> clockid_t clk,
> + struct __kernel_timespec *ts)
> {
> const struct vdso_data *vd = __arch_get_timens_vdso_data();
> const struct vdso_timestamp *vdso_ts = &vd->basetime[clk];
> @@ -188,8 +188,8 @@ static int do_coarse_timens(const struct vdso_data *vdns,
> clockid_t clk,
> return 0;
> }
> #else
> -static int do_coarse_timens(const struct vdso_data *vdns, clockid_t clk,
> - struct __kernel_timespec *ts)
> +static __always_inline int do_coarse_timens(const struct vdso_data *vdns,
> clockid_t clk,
> + struct __kernel_timespec *ts)
> {
> return -1;
> }
>
--
Regards,
Vincenzo
Re: [PATCH RESEND v1 2/4] lib/vdso: Add vdso_data pointer as input to __arch_get_timens_vdso_data()
On 3/31/21 5:48 PM, Christophe Leroy wrote:
> For the same reason as commit e876f0b69dc9 ("lib/vdso: Allow
> architectures to provide the vdso data pointer"), powerpc wants to
> avoid calculation of relative position to code.
>
> As the timens_vdso_data is next page to vdso_data, provide
> vdso_data pointer to __arch_get_timens_vdso_data() in order
> to ease the calculation on powerpc in following patches.
>
> Signed-off-by: Christophe Leroy
Reviewed-by: Vincenzo Frascino
> ---
> arch/arm64/include/asm/vdso/compat_gettimeofday.h | 3 ++-
> arch/arm64/include/asm/vdso/gettimeofday.h| 2 +-
> arch/s390/include/asm/vdso/gettimeofday.h | 3 ++-
> arch/x86/include/asm/vdso/gettimeofday.h | 3 ++-
> lib/vdso/gettimeofday.c | 15 +--
> 5 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm64/include/asm/vdso/compat_gettimeofday.h
> b/arch/arm64/include/asm/vdso/compat_gettimeofday.h
> index 7508b0ac1d21..ecb6fd4c3c64 100644
> --- a/arch/arm64/include/asm/vdso/compat_gettimeofday.h
> +++ b/arch/arm64/include/asm/vdso/compat_gettimeofday.h
> @@ -155,7 +155,8 @@ static __always_inline const struct vdso_data
> *__arch_get_vdso_data(void)
> }
>
> #ifdef CONFIG_TIME_NS
> -static __always_inline const struct vdso_data
> *__arch_get_timens_vdso_data(void)
> +static __always_inline
> +const struct vdso_data *__arch_get_timens_vdso_data(const struct vdso_data
> *vd)
> {
> const struct vdso_data *ret;
>
> diff --git a/arch/arm64/include/asm/vdso/gettimeofday.h
> b/arch/arm64/include/asm/vdso/gettimeofday.h
> index 631ab1281633..de86230a9436 100644
> --- a/arch/arm64/include/asm/vdso/gettimeofday.h
> +++ b/arch/arm64/include/asm/vdso/gettimeofday.h
> @@ -100,7 +100,7 @@ const struct vdso_data *__arch_get_vdso_data(void)
>
> #ifdef CONFIG_TIME_NS
> static __always_inline
> -const struct vdso_data *__arch_get_timens_vdso_data(void)
> +const struct vdso_data *__arch_get_timens_vdso_data(const struct vdso_data
> *vd)
> {
> return _timens_data;
> }
> diff --git a/arch/s390/include/asm/vdso/gettimeofday.h
> b/arch/s390/include/asm/vdso/gettimeofday.h
> index ed89ef742530..383c53c3 100644
> --- a/arch/s390/include/asm/vdso/gettimeofday.h
> +++ b/arch/s390/include/asm/vdso/gettimeofday.h
> @@ -68,7 +68,8 @@ long clock_getres_fallback(clockid_t clkid, struct
> __kernel_timespec *ts)
> }
>
> #ifdef CONFIG_TIME_NS
> -static __always_inline const struct vdso_data
> *__arch_get_timens_vdso_data(void)
> +static __always_inline
> +const struct vdso_data *__arch_get_timens_vdso_data(const struct vdso_data
> *vd)
> {
> return _timens_data;
> }
> diff --git a/arch/x86/include/asm/vdso/gettimeofday.h
> b/arch/x86/include/asm/vdso/gettimeofday.h
> index df01d7349d79..1936f21ed8cd 100644
> --- a/arch/x86/include/asm/vdso/gettimeofday.h
> +++ b/arch/x86/include/asm/vdso/gettimeofday.h
> @@ -58,7 +58,8 @@ extern struct ms_hyperv_tsc_page hvclock_page
> #endif
>
> #ifdef CONFIG_TIME_NS
> -static __always_inline const struct vdso_data
> *__arch_get_timens_vdso_data(void)
> +static __always_inline
> +const struct vdso_data *__arch_get_timens_vdso_data(const struct vdso_data
> *vd)
> {
> return __timens_vdso_data;
> }
> diff --git a/lib/vdso/gettimeofday.c b/lib/vdso/gettimeofday.c
> index c6f6dee08746..ce2f69552003 100644
> --- a/lib/vdso/gettimeofday.c
> +++ b/lib/vdso/gettimeofday.c
> @@ -49,13 +49,15 @@ static inline bool vdso_cycles_ok(u64 cycles)
> static __always_inline int do_hres_timens(const struct vdso_data *vdns,
> clockid_t clk,
> struct __kernel_timespec *ts)
> {
> - const struct vdso_data *vd = __arch_get_timens_vdso_data();
> + const struct vdso_data *vd;
> const struct timens_offset *offs = &vdns->offset[clk];
> const struct vdso_timestamp *vdso_ts;
> u64 cycles, last, ns;
> u32 seq;
> s64 sec;
>
> + vd = vdns - (clk == CLOCK_MONOTONIC_RAW ? CS_RAW : CS_HRES_COARSE);
> + vd = __arch_get_timens_vdso_data(vd);
> if (clk != CLOCK_MONOTONIC_RAW)
> vd = &vd[CS_HRES_COARSE];
> else
> @@ -92,7 +94,8 @@ static __always_inline int do_hres_timens(const struct
> vdso_data *vdns, clockid_
> return 0;
> }
> #else
> -static __always_inline const struct vdso_data
> *__arch_get_timens_vdso_data(void)
> +static __always_inline
> +const struct vdso_data *__arch_get_timens_vdso_data(const struct vdso_data
> *vd)
> {
> return NULL;
> }
> @@ -162,7 +165,7 @@ static _
Re: [PATCH RESEND v1 3/4] powerpc/vdso: Separate vvar vma from vdso
On 3/31/21 5:48 PM, Christophe Leroy wrote:
> From: Dmitry Safonov
>
> Since commit 511157ab641e ("powerpc/vdso: Move vdso datapage up front")
> VVAR page is in front of the VDSO area. In result it breaks CRIU
> (Checkpoint Restore In Userspace) [1], where CRIU expects that "[vdso]"
> from /proc/../maps points at ELF/vdso image, rather than at VVAR data page.
> Laurent made a patch to keep CRIU working (by reading aux vector).
> But I think it still makes sence to separate two mappings into different
> VMAs. It will also make ppc64 less "special" for userspace and as
> a side-bonus will make VVAR page un-writable by debugger (which previously
> would COW page and can be unexpected).
>
> I opportunistically Cc stable on it: I understand that usually such
> stuff isn't a stable material, but that will allow us in CRIU have
> one workaround less that is needed just for one release (v5.11) on
> one platform (ppc64), which we otherwise have to maintain.
> I wouldn't go as far as to say that the commit 511157ab641e is ABI
> regression as no other userspace got broken, but I'd really appreciate
> if it gets backported to v5.11 after v5.12 is released, so as not
> to complicate already non-simple CRIU-vdso code. Thanks!
>
> Cc: Andrei Vagin
> Cc: Andy Lutomirski
> Cc: Benjamin Herrenschmidt
> Cc: Christophe Leroy
> Cc: Laurent Dufour
> Cc: Michael Ellerman
> Cc: Paul Mackerras
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: sta...@vger.kernel.org # v5.11
> [1]: https://github.com/checkpoint-restore/criu/issues/1417
> Signed-off-by: Dmitry Safonov
> Tested-by: Christophe Leroy
> Signed-off-by: Christophe Leroy
Reviewed-by: Vincenzo Frascino # vDSO parts.
> ---
> arch/powerpc/include/asm/mmu_context.h | 2 +-
> arch/powerpc/kernel/vdso.c | 54 +++---
> 2 files changed, 40 insertions(+), 16 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/mmu_context.h
> b/arch/powerpc/include/asm/mmu_context.h
> index 652ce85f9410..4bc45d3ed8b0 100644
> --- a/arch/powerpc/include/asm/mmu_context.h
> +++ b/arch/powerpc/include/asm/mmu_context.h
> @@ -263,7 +263,7 @@ extern void arch_exit_mmap(struct mm_struct *mm);
> static inline void arch_unmap(struct mm_struct *mm,
> unsigned long start, unsigned long end)
> {
> - unsigned long vdso_base = (unsigned long)mm->context.vdso - PAGE_SIZE;
> + unsigned long vdso_base = (unsigned long)mm->context.vdso;
>
> if (start <= vdso_base && vdso_base < end)
> mm->context.vdso = NULL;
> diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
> index e839a906fdf2..b14907209822 100644
> --- a/arch/powerpc/kernel/vdso.c
> +++ b/arch/powerpc/kernel/vdso.c
> @@ -55,10 +55,10 @@ static int vdso_mremap(const struct vm_special_mapping
> *sm, struct vm_area_struc
> {
> unsigned long new_size = new_vma->vm_end - new_vma->vm_start;
>
> - if (new_size != text_size + PAGE_SIZE)
> + if (new_size != text_size)
> return -EINVAL;
>
> - current->mm->context.vdso = (void __user *)new_vma->vm_start +
> PAGE_SIZE;
> + current->mm->context.vdso = (void __user *)new_vma->vm_start;
>
> return 0;
> }
> @@ -73,6 +73,10 @@ static int vdso64_mremap(const struct vm_special_mapping
> *sm, struct vm_area_str
> return vdso_mremap(sm, new_vma, &vdso64_end - &vdso64_start);
> }
>
> +static struct vm_special_mapping vvar_spec __ro_after_init = {
> + .name = "[vvar]",
> +};
> +
> static struct vm_special_mapping vdso32_spec __ro_after_init = {
> .name = "[vdso]",
> .mremap = vdso32_mremap,
> @@ -89,11 +93,11 @@ static struct vm_special_mapping vdso64_spec
> __ro_after_init = {
> */
> static int __arch_setup_additional_pages(struct linux_binprm *bprm, int
> uses_interp)
> {
> - struct mm_struct *mm = current->mm;
> + unsigned long vdso_size, vdso_base, mappings_size;
> struct vm_special_mapping *vdso_spec;
> + unsigned long vvar_size = PAGE_SIZE;
> + struct mm_struct *mm = current->mm;
> struct vm_area_struct *vma;
> - unsigned long vdso_size;
> - unsigned long vdso_base;
>
> if (is_32bit_task()) {
> vdso_spec = &vdso32_spec;
> @@ -110,8 +114,8 @@ static int __arch_setup_additional_pages(struct
> linux_binprm *bprm, int uses_int
> vdso_base = 0;
> }
>
> - /* Add a page to the vdso size for the data page */
> - vdso_size += PAGE_SIZE;
> + mappings_size = vds
Re: [PATCH RESEND v1 4/4] powerpc/vdso: Add support for time namespaces
On 3/31/21 5:48 PM, Christophe Leroy wrote:
> This patch adds the necessary glue to provide time namespaces.
>
> Things are mainly copied from ARM64.
>
> __arch_get_timens_vdso_data() calculates timens vdso data position
> based on the vdso data position, knowing it is the next page in vvar.
> This avoids having to redo the mflr/bcl/mflr/mtlr dance to locate
> the page relative to running code position.
>
> Signed-off-by: Christophe Leroy
Reviewed-by: Vincenzo Frascino # vDSO parts
> ---
> arch/powerpc/Kconfig | 3 +-
> arch/powerpc/include/asm/vdso/gettimeofday.h | 10 ++
> arch/powerpc/include/asm/vdso_datapage.h | 2 -
> arch/powerpc/kernel/vdso.c | 116 ---
> arch/powerpc/kernel/vdso32/vdso32.lds.S | 2 +-
> arch/powerpc/kernel/vdso64/vdso64.lds.S | 2 +-
> 6 files changed, 114 insertions(+), 21 deletions(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index c1344c05226c..71daff5f15d5 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -172,6 +172,7 @@ config PPC
> select GENERIC_CPU_AUTOPROBE
> select GENERIC_CPU_VULNERABILITIES if PPC_BARRIER_NOSPEC
> select GENERIC_EARLY_IOREMAP
> + select GENERIC_GETTIMEOFDAY
> select GENERIC_IRQ_SHOW
> select GENERIC_IRQ_SHOW_LEVEL
> select GENERIC_PCI_IOMAPif PCI
> @@ -179,7 +180,7 @@ config PPC
> select GENERIC_STRNCPY_FROM_USER
> select GENERIC_STRNLEN_USER
> select GENERIC_TIME_VSYSCALL
> - select GENERIC_GETTIMEOFDAY
> + select GENERIC_VDSO_TIME_NS
> select HAVE_ARCH_AUDITSYSCALL
> select HAVE_ARCH_HUGE_VMAP if PPC_BOOK3S_64 &&
> PPC_RADIX_MMU
> select HAVE_ARCH_JUMP_LABEL
> diff --git a/arch/powerpc/include/asm/vdso/gettimeofday.h
> b/arch/powerpc/include/asm/vdso/gettimeofday.h
> index d453e725c79f..e448df1dd071 100644
> --- a/arch/powerpc/include/asm/vdso/gettimeofday.h
> +++ b/arch/powerpc/include/asm/vdso/gettimeofday.h
> @@ -2,6 +2,8 @@
> #ifndef _ASM_POWERPC_VDSO_GETTIMEOFDAY_H
> #define _ASM_POWERPC_VDSO_GETTIMEOFDAY_H
>
> +#include
> +
> #ifdef __ASSEMBLY__
>
> #include
> @@ -153,6 +155,14 @@ static __always_inline u64 __arch_get_hw_counter(s32
> clock_mode,
>
> const struct vdso_data *__arch_get_vdso_data(void);
>
> +#ifdef CONFIG_TIME_NS
> +static __always_inline
> +const struct vdso_data *__arch_get_timens_vdso_data(const struct vdso_data
> *vd)
> +{
> + return (void *)vd + PAGE_SIZE;
> +}
> +#endif
> +
> static inline bool vdso_clocksource_ok(const struct vdso_data *vd)
> {
> return true;
> diff --git a/arch/powerpc/include/asm/vdso_datapage.h
> b/arch/powerpc/include/asm/vdso_datapage.h
> index 3f958ecf2beb..a585c8e538ff 100644
> --- a/arch/powerpc/include/asm/vdso_datapage.h
> +++ b/arch/powerpc/include/asm/vdso_datapage.h
> @@ -107,9 +107,7 @@ extern struct vdso_arch_data *vdso_data;
> bcl 20, 31, .+4
> 999:
> mflr\ptr
> -#if CONFIG_PPC_PAGE_SHIFT > 14
> addis \ptr, \ptr, (_vdso_datapage - 999b)@ha
> -#endif
> addi\ptr, \ptr, (_vdso_datapage - 999b)@l
> .endm
>
> diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
> index b14907209822..717f2c9a7573 100644
> --- a/arch/powerpc/kernel/vdso.c
> +++ b/arch/powerpc/kernel/vdso.c
> @@ -18,6 +18,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #include
> @@ -50,6 +51,12 @@ static union {
> } vdso_data_store __page_aligned_data;
> struct vdso_arch_data *vdso_data = &vdso_data_store.data;
>
> +enum vvar_pages {
> + VVAR_DATA_PAGE_OFFSET,
> + VVAR_TIMENS_PAGE_OFFSET,
> + VVAR_NR_PAGES,
> +};
> +
> static int vdso_mremap(const struct vm_special_mapping *sm, struct
> vm_area_struct *new_vma,
> unsigned long text_size)
> {
> @@ -73,8 +80,12 @@ static int vdso64_mremap(const struct vm_special_mapping
> *sm, struct vm_area_str
> return vdso_mremap(sm, new_vma, &vdso64_end - &vdso64_start);
> }
>
> +static vm_fault_t vvar_fault(const struct vm_special_mapping *sm,
> + struct vm_area_struct *vma, struct vm_fault *vmf);
> +
> static struct vm_special_mapping vvar_spec __ro_after_init = {
> .name = "[vvar]",
> + .fault = vvar_fault,
> };
>
> static struct vm_special_mapping vdso32_spec __ro_after_init = {
> @@ -87,6 +98,94 @@ static struct vm_special_mapping vdso64_spec
> __ro_after_init = {
> .mremap = vdso64_mremap,
>
[PATCH v3 0/3] Fix vDSO 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. 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 Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: Michael Ellerman Cc: Martin Schwidefsky Cc: Heiko Carstens Cc: Shuah Khan Cc: Thomas Gleixner Cc: Arnd Bergmann Signed-off-by: Vincenzo Frascino 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 -- 2.21.0
[PATCH v3 1/3] powerpc: Fix vDSO 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 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: sta...@vger.kernel.org
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: Michael Ellerman
Signed-off-by: Vincenzo Frascino
Reviewed-by: Christophe Leroy
---
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)
crorcr0*4+eq,cr0*4+eq,cr1*4+eq
bne cr0,99f
+ mflrr12
+ .cfi_register lr,r12
+ bl __get_datapage@local
+ lwz r5,CLOCK_REALTIME_RES(r3)
+ mtlrr12
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_getre
[PATCH v3 3/3] kselftest: Extend vDSO selftest to 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
Signed-off-by: Vincenzo Frascino
---
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 ..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
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#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;
+}
--
2.21.0
[PATCH v3 2/3] s390: Fix vDSO 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 s390 vdso implementation of clock_getres keeping a copy of
hrtimer_resolution in vdso data and using that directly.
Cc: Martin Schwidefsky
Cc: Heiko Carstens
Signed-off-by: Vincenzo Frascino
Acked-by: Martin Schwidefsky
---
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_nsec0x60 */
__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_MONOTONI
Re: [PATCH v3 3/3] kselftest: Extend vDSO selftest to clock_getres
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
>> Signed-off-by: Vincenzo Frascino
>> ---
>>
>> 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 ..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
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#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
Re: [PATCH v3 3/3] kselftest: Extend vDSO selftest to clock_getres
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
>> Signed-off-by: Vincenzo Frascino
>> ---
>>
>> 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 ..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
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#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
[PATCH v4 0/3] Fix vDSO 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. 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: v4: - Address review comments. v3: - Rebased on 5.2-rc1. - Address review comments. v2: - Rebased on 5.1-rc5. - Addressed review comments. Cc: Christophe Leroy Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: Michael Ellerman Cc: Martin Schwidefsky Cc: Heiko Carstens Cc: Shuah Khan Cc: Thomas Gleixner Cc: Arnd Bergmann Signed-off-by: Vincenzo Frascino 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| 124 ++ 12 files changed, 155 insertions(+), 16 deletions(-) create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c -- 2.21.0
[PATCH v4 1/3] powerpc: Fix vDSO 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 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: sta...@vger.kernel.org
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: Michael Ellerman
Signed-off-by: Vincenzo Frascino
Reviewed-by: Christophe Leroy
---
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)
crorcr0*4+eq,cr0*4+eq,cr1*4+eq
bne cr0,99f
+ mflrr12
+ .cfi_register lr,r12
+ bl __get_datapage@local
+ lwz r5,CLOCK_REALTIME_RES(r3)
+ mtlrr12
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_getre
[PATCH v4 2/3] s390: Fix vDSO 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 s390 vdso implementation of clock_getres keeping a copy of
hrtimer_resolution in vdso data and using that directly.
Cc: Martin Schwidefsky
Cc: Heiko Carstens
Signed-off-by: Vincenzo Frascino
Acked-by: Martin Schwidefsky
---
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_nsec0x60 */
__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_MONOTONI
[PATCH v4 3/3] kselftest: Extend vDSO selftest to 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
Signed-off-by: Vincenzo Frascino
---
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| 124 ++
2 files changed, 126 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 ..b62d8d4f7c38
--- /dev/null
+++ b/tools/testing/selftests/vDSO/vdso_clock_getres.c
@@ -0,0 +1,124 @@
+// 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
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#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 KSFT_PASS;
+}
+
+int main(int argc, char **argv)
+{
+ int ret;
+
+#if _POSIX_TIMERS > 0
+
+#ifdef CLOCK_REALTIME
+ ret = vdso_test_clock(CLOCK_REALTIME);
+#endif
+
+#ifdef CLOCK_BOOTTIME
+ ret += vdso_test_clock(CLOCK_BOOTTIME);
+#endif
+
+#ifdef CLOCK_TAI
+ ret += vdso_test_clock(CLOCK_TAI);
+#endif
+
+#ifdef CLOCK_REALTIME_COARSE
+ ret += vdso_test_clock(CLOCK_REALTIME_COARSE);
+#endif
+
+#ifdef CLOCK_MONOTONIC
+ ret += vdso_test_clock(CLOCK_MONOTONIC);
+#endif
+
+#ifdef CLOCK_MONOTONIC_RAW
+ ret += vdso_test_clock(CLOCK_MONOTONIC_RAW);
+#endif
+
+#ifdef CLOCK_MONOTONIC_COARSE
+ ret += vdso_test_clock(CLOCK_MONOTONIC_COARSE);
+#endif
+
+#endif
+ if (ret > 0)
+ return KSFT_FAIL;
+
+ return KSFT_PASS;
+}
--
2.21.0
Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
Hi Michael, thank you for your reply. On 28/05/2019 07:19, Michael Ellerman wrote: > Vincenzo Frascino writes: > >> 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. > > This is passing for me even without patch 1 applied, shouldn't it fail > without the fix? What am I missing? > This is correct, because during the refactoring process I missed an "n" :) if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_sec·!=·y.tv_sec)) Should be: if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_nsec·!=·y.tv_nsec)) My mistake, I am going to fix the test and re-post v5 of this set. Without my patch if you pass "highres=off" to the kernel (as a command line parameter) it leads to a broken implementation of clock_getres since the value of CLOCK_REALTIME_RES does not change at runtime. Expected result (with highres=off): # uname -r 5.2.0-rc2 # ./vdso_clock_getres clock_id: CLOCK_REALTIME [FAIL] clock_id: CLOCK_BOOTTIME [PASS] clock_id: CLOCK_TAI [PASS] clock_id: CLOCK_REALTIME_COARSE [PASS] clock_id: CLOCK_MONOTONIC [FAIL] clock_id: CLOCK_MONOTONIC_RAW [PASS] clock_id: CLOCK_MONOTONIC_COARSE [PASS] The reason of this behavior is that the only clocks supported by getres on powerpc are CLOCK_REALTIME and CLOCK_MONOTONIC, the rest on the clocks use always syscalls. > # uname -r > 5.2.0-rc2-gcc-8.2.0 > > # ./vdso_clock_getres > 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] > > cheers > >> Cc: Shuah Khan >> Signed-off-by: Vincenzo Frascino >> --- >> >> 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| 124 ++ >> 2 files changed, 126 insertions(+) >> create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c -- Regards, Vincenzo
[PATCH v5 0/3] Fix vDSO 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. 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: v5: - Rebased on 5.2-rc2 - Fixed a bug in kselftest. v4: - Address review comments. v3: - Rebased on 5.2-rc1. - Address review comments. v2: - Rebased on 5.1-rc5. - Addressed review comments. Cc: Christophe Leroy Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: Michael Ellerman Cc: Martin Schwidefsky Cc: Heiko Carstens Cc: Shuah Khan Cc: Thomas Gleixner Cc: Arnd Bergmann Signed-off-by: Vincenzo Frascino 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| 124 ++ 12 files changed, 155 insertions(+), 16 deletions(-) create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c -- 2.21.0
[PATCH v5 1/3] powerpc: Fix vDSO 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 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: sta...@vger.kernel.org
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: Michael Ellerman
Signed-off-by: Vincenzo Frascino
Reviewed-by: Christophe Leroy
---
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)
crorcr0*4+eq,cr0*4+eq,cr1*4+eq
bne cr0,99f
+ mflrr12
+ .cfi_register lr,r12
+ bl __get_datapage@local
+ lwz r5,CLOCK_REALTIME_RES(r3)
+ mtlrr12
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_getre
[PATCH v5 2/3] s390: Fix vDSO 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 s390 vdso implementation of clock_getres keeping a copy of
hrtimer_resolution in vdso data and using that directly.
Cc: Martin Schwidefsky
Cc: Heiko Carstens
Signed-off-by: Vincenzo Frascino
Acked-by: Martin Schwidefsky
---
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_nsec0x60 */
__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_MONOTONI
[PATCH v5 3/3] kselftest: Extend vDSO selftest to 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
Signed-off-by: Vincenzo Frascino
---
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| 124 ++
2 files changed, 126 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 ..15dcee16ff72
--- /dev/null
+++ b/tools/testing/selftests/vDSO/vdso_clock_getres.c
@@ -0,0 +1,124 @@
+// 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
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#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_nsec != y.tv_nsec)) {
+ printf(" [FAIL]\n");
+ return KSFT_FAIL;
+ }
+
+ printf(" [PASS]\n");
+ return KSFT_PASS;
+}
+
+int main(int argc, char **argv)
+{
+ int ret;
+
+#if _POSIX_TIMERS > 0
+
+#ifdef CLOCK_REALTIME
+ ret = vdso_test_clock(CLOCK_REALTIME);
+#endif
+
+#ifdef CLOCK_BOOTTIME
+ ret += vdso_test_clock(CLOCK_BOOTTIME);
+#endif
+
+#ifdef CLOCK_TAI
+ ret += vdso_test_clock(CLOCK_TAI);
+#endif
+
+#ifdef CLOCK_REALTIME_COARSE
+ ret += vdso_test_clock(CLOCK_REALTIME_COARSE);
+#endif
+
+#ifdef CLOCK_MONOTONIC
+ ret += vdso_test_clock(CLOCK_MONOTONIC);
+#endif
+
+#ifdef CLOCK_MONOTONIC_RAW
+ ret += vdso_test_clock(CLOCK_MONOTONIC_RAW);
+#endif
+
+#ifdef CLOCK_MONOTONIC_COARSE
+ ret += vdso_test_clock(CLOCK_MONOTONIC_COARSE);
+#endif
+
+#endif
+ if (ret > 0)
+ return KSFT_FAIL;
+
+ return KSFT_PASS;
+}
--
2.21.0
Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
Hi Christophe, On 28/05/2019 18:01, Christophe Leroy wrote: > Vincenzo Frascino a écrit : > >> Hi Michael, >> >> thank you for your reply. >> >> On 28/05/2019 07:19, Michael Ellerman wrote: >>> Vincenzo Frascino writes: >>> >>>> 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. >>> >>> This is passing for me even without patch 1 applied, shouldn't it fail >>> without the fix? What am I missing? >>> >> >> This is correct, because during the refactoring process I missed an "n" :) >> >> if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_sec·!=·y.tv_sec)) >> >> Should be: >> >> if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_nsec·!=·y.tv_nsec)) > > Maybe you'd better use timercmp() from sys/time.h > timercmp() takes "struct timeval" not "struct timespec". > Christophe > >> >> My mistake, I am going to fix the test and re-post v5 of this set. >> >> Without my patch if you pass "highres=off" to the kernel (as a command line >> parameter) it leads to a broken implementation of clock_getres since >> the value >> of CLOCK_REALTIME_RES does not change at runtime. >> >> Expected result (with highres=off): >> >> # uname -r >> 5.2.0-rc2 >> # ./vdso_clock_getres >> clock_id: CLOCK_REALTIME [FAIL] >> clock_id: CLOCK_BOOTTIME [PASS] >> clock_id: CLOCK_TAI [PASS] >> clock_id: CLOCK_REALTIME_COARSE [PASS] >> clock_id: CLOCK_MONOTONIC [FAIL] >> clock_id: CLOCK_MONOTONIC_RAW [PASS] >> clock_id: CLOCK_MONOTONIC_COARSE [PASS] >> >> The reason of this behavior is that the only clocks supported by getres on >> powerpc are CLOCK_REALTIME and CLOCK_MONOTONIC, the rest on the clocks use >> always syscalls. >> >>> # uname -r >>> 5.2.0-rc2-gcc-8.2.0 >>> >>> # ./vdso_clock_getres >>> 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] >>> >>> cheers >>> >>>> Cc: Shuah Khan >>>> Signed-off-by: Vincenzo Frascino >>>> --- >>>> >>>> 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| 124 ++ >>>> 2 files changed, 126 insertions(+) >>>> create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c >> >> -- >> Regards, >> Vincenzo > > -- Regards, Vincenzo
Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
Hi Christophe,
On 04/06/2019 14:16, Christophe Leroy wrote:
> Hi Vincenzo
>
> Le 28/05/2019 à 13:57, Vincenzo Frascino a écrit :
>> Hi Michael,
>>
>> thank you for your reply.
>>
>> On 28/05/2019 07:19, Michael Ellerman wrote:
>>> Vincenzo Frascino writes:
>>>
>>>> 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.
>>>
>>> This is passing for me even without patch 1 applied, shouldn't it fail
>>> without the fix? What am I missing?
>>>
>>
>> This is correct, because during the refactoring process I missed an "n" :)
>>
>> if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_sec·!=·y.tv_sec))
>>
>> Should be:
>>
>> if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_nsec·!=·y.tv_nsec))
>>
>> My mistake, I am going to fix the test and re-post v5 of this set.
>>
>> Without my patch if you pass "highres=off" to the kernel (as a command line
>> parameter) it leads to a broken implementation of clock_getres since the
>> value
>> of CLOCK_REALTIME_RES does not change at runtime.
>>
>> Expected result (with highres=off):
>>
>> # uname -r
>> 5.2.0-rc2
>> # ./vdso_clock_getres
>> clock_id: CLOCK_REALTIME [FAIL]
>> clock_id: CLOCK_BOOTTIME [PASS]
>> clock_id: CLOCK_TAI [PASS]
>> clock_id: CLOCK_REALTIME_COARSE [PASS]
>> clock_id: CLOCK_MONOTONIC [FAIL]
>> clock_id: CLOCK_MONOTONIC_RAW [PASS]
>> clock_id: CLOCK_MONOTONIC_COARSE [PASS]
>>
>> The reason of this behavior is that the only clocks supported by getres on
>> powerpc are CLOCK_REALTIME and CLOCK_MONOTONIC, the rest on the clocks use
>> always syscalls.
>
> vdso64 is supposed to implement CLOCK_{REALTIME/MONOTONIC}_COARSE, so I
> guess it should fail for them too ?
>
> Or is your test done on vdso32 ?
>
Based on what I can see in kernel/vdso64 in 5.2-rc3:
/*
* Exact prototype of clock_getres()
*
* int __kernel_clock_getres(clockid_t clock_id, struct timespec *res);
*
*/
V_FUNCTION_BEGIN(__kernel_clock_getres)
.cfi_startproc
/* Check for supported clock IDs */
cmpwi cr0,r3,CLOCK_REALTIME
cmpwi cr1,r3,CLOCK_MONOTONIC
crorcr0*4+eq,cr0*4+eq,cr1*4+eq
bne cr0,99f
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
/*
* syscall fallback
*/
99:
li r0,__NR_clock_getres
sc
blr
.cfi_endproc
V_FUNCTION_END(__kernel_clock_getres)
it does not seem so for what concerns vdso64. I did run again the test both on
ppc and ppc64 qemu instances and the result is the same to what I reported in
this thread.
Am I missing something?
> Christophe
>
>>
>>> # uname -r
>>> 5.2.0-rc2-gcc-8.2.0
>>>
>>> # ./vdso_clock_getres
>>> 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]
>>>
>>> cheers
>>>
>>>> Cc: Shuah Khan
>>>> Signed-off-by: Vincenzo Frascino
>>>> ---
>>>>
>>>> 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| 124 ++
>>>> 2 files changed, 126 insertions(+)
>>>> create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
>>
--
Regards,
Vincenzo
Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
On 04/06/2019 14:39, Christophe Leroy wrote:
>
>
> Le 04/06/2019 à 15:32, Vincenzo Frascino a écrit :
>> Hi Christophe,
>>
>> On 04/06/2019 14:16, Christophe Leroy wrote:
>>> Hi Vincenzo
>>>
>>> Le 28/05/2019 à 13:57, Vincenzo Frascino a écrit :
>>>> Hi Michael,
>>>>
>>>> thank you for your reply.
>>>>
>>>> On 28/05/2019 07:19, Michael Ellerman wrote:
>>>>> Vincenzo Frascino writes:
>>>>>
>>>>>> 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.
>>>>>
>>>>> This is passing for me even without patch 1 applied, shouldn't it fail
>>>>> without the fix? What am I missing?
>>>>>
>>>>
>>>> This is correct, because during the refactoring process I missed an "n" :)
>>>>
>>>> if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_sec·!=·y.tv_sec))
>>>>
>>>> Should be:
>>>>
>>>> if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_nsec·!=·y.tv_nsec))
>>>>
>>>> My mistake, I am going to fix the test and re-post v5 of this set.
>>>>
>>>> Without my patch if you pass "highres=off" to the kernel (as a command line
>>>> parameter) it leads to a broken implementation of clock_getres since the
>>>> value
>>>> of CLOCK_REALTIME_RES does not change at runtime.
>>>>
>>>> Expected result (with highres=off):
>>>>
>>>> # uname -r
>>>> 5.2.0-rc2
>>>> # ./vdso_clock_getres
>>>> clock_id: CLOCK_REALTIME [FAIL]
>>>> clock_id: CLOCK_BOOTTIME [PASS]
>>>> clock_id: CLOCK_TAI [PASS]
>>>> clock_id: CLOCK_REALTIME_COARSE [PASS]
>>>> clock_id: CLOCK_MONOTONIC [FAIL]
>>>> clock_id: CLOCK_MONOTONIC_RAW [PASS]
>>>> clock_id: CLOCK_MONOTONIC_COARSE [PASS]
>>>>
>>>> The reason of this behavior is that the only clocks supported by getres on
>>>> powerpc are CLOCK_REALTIME and CLOCK_MONOTONIC, the rest on the clocks use
>>>> always syscalls.
>>>
>>> vdso64 is supposed to implement CLOCK_{REALTIME/MONOTONIC}_COARSE, so I
>>> guess it should fail for them too ?
>>>
>>> Or is your test done on vdso32 ?
>>>
>>
>> Based on what I can see in kernel/vdso64 in 5.2-rc3:
>>
>> /*
>> * Exact prototype of clock_getres()
>> *
>> * int __kernel_clock_getres(clockid_t clock_id, struct timespec *res);
>> *
>> */
>> V_FUNCTION_BEGIN(__kernel_clock_getres)
>>.cfi_startproc
>> /* Check for supported clock IDs */
>> cmpwi cr0,r3,CLOCK_REALTIME
>> cmpwi cr1,r3,CLOCK_MONOTONIC
>> crorcr0*4+eq,cr0*4+eq,cr1*4+eq
>> bne cr0,99f
>>
>> 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
>>
>> /*
>> * syscall fallback
>> */
>> 99:
>> li r0,__NR_clock_getres
>> sc
>> blr
>>.cfi_endproc
>> V_FUNCTION_END(__kernel_clock_getres)
>>
>> it does not seem so for what concerns vdso64. I did run again the test both
>> on
>> ppc and ppc64 qemu instances and the result is the same to what I reported in
>> this thread.
>>
>> Am I missing something?
>
> I was thinking about
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5c929885f1bb
>
> but apparently clock_getres() was left aside. Should we do something
> about it ?
>
Sure, but I would like this series to be merged first (since the topic is
different). I am happy, after that, to push a separate one on top that addresses
the problem.
Please let me know if it works for you and Michael.
> Christophe
>
>>
>>> Christophe
>>>
>>>>
>>>>> # uname -r
>>>>> 5.2.0-rc2-gcc-8.2.0
>>>>>
>>>>> # ./vdso_clock_getres
>>>>> 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]
>>>>>
>>>>> cheers
>>>>>
>>>>>> Cc: Shuah Khan
>>>>>> Signed-off-by: Vincenzo Frascino
>>>>>> ---
>>>>>>
>>>>>> 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| 124 ++
>>>>>>2 files changed, 126 insertions(+)
>>>>>>create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
>>>>
>>
--
Regards,
Vincenzo
Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
On 04/06/2019 14:52, Christophe Leroy wrote:
>
>
> Le 04/06/2019 à 15:43, Vincenzo Frascino a écrit :
>> On 04/06/2019 14:39, Christophe Leroy wrote:
>>>
>>>
>>> Le 04/06/2019 à 15:32, Vincenzo Frascino a écrit :
>>>> Hi Christophe,
>>>>
>>>> On 04/06/2019 14:16, Christophe Leroy wrote:
>>>>> Hi Vincenzo
>>>>>
>>>>> Le 28/05/2019 à 13:57, Vincenzo Frascino a écrit :
>>>>>> Hi Michael,
>>>>>>
>>>>>> thank you for your reply.
>>>>>>
>>>>>> On 28/05/2019 07:19, Michael Ellerman wrote:
>>>>>>> Vincenzo Frascino writes:
>>>>>>>
>>>>>>>> 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.
>>>>>>>
>>>>>>> This is passing for me even without patch 1 applied, shouldn't it fail
>>>>>>> without the fix? What am I missing?
>>>>>>>
>>>>>>
>>>>>> This is correct, because during the refactoring process I missed an "n"
>>>>>> :)
>>>>>>
>>>>>> if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_sec·!=·y.tv_sec))
>>>>>>
>>>>>> Should be:
>>>>>>
>>>>>> if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_nsec·!=·y.tv_nsec))
>>>>>>
>>>>>> My mistake, I am going to fix the test and re-post v5 of this set.
>>>>>>
>>>>>> Without my patch if you pass "highres=off" to the kernel (as a command
>>>>>> line
>>>>>> parameter) it leads to a broken implementation of clock_getres since the
>>>>>> value
>>>>>> of CLOCK_REALTIME_RES does not change at runtime.
>>>>>>
>>>>>> Expected result (with highres=off):
>>>>>>
>>>>>> # uname -r
>>>>>> 5.2.0-rc2
>>>>>> # ./vdso_clock_getres
>>>>>> clock_id: CLOCK_REALTIME [FAIL]
>>>>>> clock_id: CLOCK_BOOTTIME [PASS]
>>>>>> clock_id: CLOCK_TAI [PASS]
>>>>>> clock_id: CLOCK_REALTIME_COARSE [PASS]
>>>>>> clock_id: CLOCK_MONOTONIC [FAIL]
>>>>>> clock_id: CLOCK_MONOTONIC_RAW [PASS]
>>>>>> clock_id: CLOCK_MONOTONIC_COARSE [PASS]
>>>>>>
>>>>>> The reason of this behavior is that the only clocks supported by getres
>>>>>> on
>>>>>> powerpc are CLOCK_REALTIME and CLOCK_MONOTONIC, the rest on the clocks
>>>>>> use
>>>>>> always syscalls.
>>>>>
>>>>> vdso64 is supposed to implement CLOCK_{REALTIME/MONOTONIC}_COARSE, so I
>>>>> guess it should fail for them too ?
>>>>>
>>>>> Or is your test done on vdso32 ?
>>>>>
>>>>
>>>> Based on what I can see in kernel/vdso64 in 5.2-rc3:
>>>>
>>>> /*
>>>>* Exact prototype of clock_getres()
>>>>*
>>>>* int __kernel_clock_getres(clockid_t clock_id, struct timespec *res);
>>>>*
>>>>*/
>>>> V_FUNCTION_BEGIN(__kernel_clock_getres)
>>>> .cfi_startproc
>>>>/* Check for supported clock IDs */
>>>>cmpwi cr0,r3,CLOCK_REALTIME
>>>>cmpwi cr1,r3,CLOCK_MONOTONIC
>>>>crorcr0*4+eq,cr0*4+eq,cr1*4+eq
>>>>bne cr0,99f
>>>>
>>>>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
>>>>
>>>>/*
>>>> * syscall fallback
>>>> */
>>>> 99:
>>>>li r0,__NR_clock_getres
>>>>sc
>>>>blr
>>>> .cfi_endproc
>>>>
Re: [PATCH] random: vDSO: Redefine PAGE_SIZE and PAGE_MASK
Hi Christophe, On 27/08/2024 11:49, Christophe Leroy wrote: ... >> >> These are still two headers outside of the vdso/ namespace. For arm64 >> we had concluded that this is never safe, and any vdso header should >> only include other vdso headers so we never pull in anything that >> e.g. depends on memory management headers that are in turn broken >> for the compat vdso. >> >> The array_size.h header is really small, so that one could >> probably just be moved into the vdso/ namespace. The minmax.h >> header is already rather complex, so it may be better to just >> open-code the usage of MIN/MAX where needed? > > It is used at two places only so yes can to that. > Could you please clarify where minmax is needed? I tried to build Jason's master tree for x86, commenting the header and it seems building fine. I might be missing something. > Same for ARRAY_SIZE(->reserved) by the way, easy to do opencode, we also have > it > only once > I have a similar issue to figure out why linux/array_size.h and uapi/linux/random.h are needed. It seems that I can build the object without them. Could you please explain? In general, the philosophy adopted to split the headers is to extract the set of functions used by vDSOs from the linux headers and place them in the vdso headers. Consequently the linux header includes to vdso one. E.g.: linux/time64.h includes vdso/time64.h. IMHO we should follow the same approach, if at all for consistency, unless there is a valid reason. ... >> >> Including uapi/linux/mman.h may still be problematic on >> some architectures if they change it in a way that is >> incompatible with compat vdso, but at least that can't >> accidentally rely on CONFIG_64BIT or something else that >> would be wrong there. > > Yes that one is tricky. Because uapi/linux/mman.h includes asm/mman.h with the > intention to include uapi/asm/mman.h but when built from the kernel in reality > you get arch/powerpc/include/asm/mman.h and I had to add some ifdefery to > kick-out kernel oddities it contains that pull additional kernel headers. > > diff --git a/arch/powerpc/include/asm/mman.h b/arch/powerpc/include/asm/mman.h > index 17a77d47ed6d..42a51a993d94 100644 > --- a/arch/powerpc/include/asm/mman.h > +++ b/arch/powerpc/include/asm/mman.h > @@ -6,7 +6,7 @@ > > #include > > -#ifdef CONFIG_PPC64 > +#if defined(CONFIG_PPC64) && !defined(BUILD_VDSO) > > #include > #include > I agree with Arnd here. uapi/linux/mman.h can cause us problems in the long run. I am attaching a patch to provide my view on how to minimize the headers included and use only the vdso/ namespace. Please, before using the code, consider that I conducted very limited testing. Note: It should apply clean on Jason's tree. Let me know your thoughts. > > Christophe -- Regards, VincenzoFrom 1933028eaeebc5c053309379c43ddea5b460c25e Mon Sep 17 00:00:00 2001 From: Vincenzo Frascino Date: Tue, 27 Aug 2024 16:44:32 +0100 Subject: [PATCH] random: VDSO: Use only headers from the vdso/ namespace The VDSO implementation includes headers from outside of the vdso/ namespace. Refactor the code to make sure that the generic library uses only the allowed namespace. Signed-off-by: Vincenzo Frascino --- arch/x86/include/asm/vdso/getrandom.h | 2 ++ arch/x86/include/asm/vdso/mman.h | 15 +++ arch/x86/include/asm/vdso/page.h | 15 +++ include/vdso/getrandom.h | 1 + include/vdso/mman.h | 7 +++ include/vdso/page.h | 7 +++ lib/vdso/getrandom.c | 20 ++-- 7 files changed, 53 insertions(+), 14 deletions(-) create mode 100644 arch/x86/include/asm/vdso/mman.h create mode 100644 arch/x86/include/asm/vdso/page.h create mode 100644 include/vdso/mman.h create mode 100644 include/vdso/page.h diff --git a/arch/x86/include/asm/vdso/getrandom.h b/arch/x86/include/asm/vdso/getrandom.h index b96e674cafde..457f237bd602 100644 --- a/arch/x86/include/asm/vdso/getrandom.h +++ b/arch/x86/include/asm/vdso/getrandom.h @@ -7,6 +7,8 @@ #ifndef __ASSEMBLY__ +#include + #include #include diff --git a/arch/x86/include/asm/vdso/mman.h b/arch/x86/include/asm/vdso/mman.h new file mode 100644 index ..4c936c9d11ab --- /dev/null +++ b/arch/x86/include/asm/vdso/mman.h @@ -0,0 +1,15 @@ + +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __ASM_VDSO_MMAN_H +#define __ASM_VDSO_MMAN_H + +#ifndef __ASSEMBLY__ + +#include + +#define VDSO_MMAP_PROT PROT_READ | PROT_WRITE +#define VDSO_MMAP_FLAGS MAP_DROPPABLE | MAP_ANONYMOUS + +#endif /* !__ASSEMBLY__ */ + +#endif /* __ASM_VDSO_MMAN_H */ diff --git a/arch/x86/include/asm/vdso/page.h b/arch/x86/include/asm/vdso/page.h new file mode 100644 index .
Re: [PATCH] random: vDSO: Redefine PAGE_SIZE and PAGE_MASK
Hi Christophe, On 27/08/2024 18:14, Christophe Leroy wrote: > > > Le 27/08/2024 à 18:05, Vincenzo Frascino a écrit : >> Hi Christophe, >> >> On 27/08/2024 11:49, Christophe Leroy wrote: >> >> ... ... >> >> Could you please clarify where minmax is needed? I tried to build Jason's >> master >> tree for x86, commenting the header and it seems building fine. I might be >> missing something. > > Without it: > > VDSO32C arch/powerpc/kernel/vdso/vgetrandom-32.o > In file included from /home/chleroy/linux-powerpc/lib/vdso/getrandom.c:11, > from : ... > > >> >>> Same for ARRAY_SIZE(->reserved) by the way, easy to do opencode, we also >>> have it >>> only once >>> >> >> I have a similar issue to figure out why linux/array_size.h and >> uapi/linux/random.h are needed. It seems that I can build the object without >> them. Could you please explain? > > Without linux/array_size.h: > > VDSO32C arch/powerpc/kernel/vdso/vgetrandom-32.o > In file included from : > /home/chleroy/linux-powerpc/lib/vdso/getrandom.c: In function > '__cvdso_getrandom_data': > /home/chleroy/linux-powerpc/lib/vdso/getrandom.c:89:40: error: implicit If this is the case, those headers should be defined for the powerpc implementation only. The generic implementation should be interpreted as the minimum common denominator in between all the architectures for what concerns the headers. -- Regards, Vincenzo
Re: [PATCH] random: vDSO: Redefine PAGE_SIZE and PAGE_MASK
Hi Christophe, On 27/08/2024 18:38, LEROY Christophe wrote: > Hi Vicenzo, > > Le 27/08/2024 à 18:05, Vincenzo Frascino a écrit : >> Hi Christophe, >> >> On 27/08/2024 11:49, Christophe Leroy wrote: >> >> ... >> >> >> >> I agree with Arnd here. uapi/linux/mman.h can cause us problems in the long >> run. >> >> I am attaching a patch to provide my view on how to minimize the headers >> included and use only the vdso/ namespace. Please, before using the code, >> consider that I conducted very limited testing. >> >> Note: It should apply clean on Jason's tree. >> >> Let me know your thoughts. >> > > Your patch looks nice, maybe a bit too much. For instance getrandom.c > can include directly asm/vdso/page.h instead of creating vdso/page.h > > Or create a vdso/page.h that only use CONFIG_PAGE_SHIFT and doesn't > include anything from architectures. > IMHO there should be only one place per architecture where PAGE_SIZE and PAGE_MASK are defined. This makes sure that if there is a problem, we do not have multiple places to look into. The indirection helps to keep consistent the namespace and allows for future extension. Similar logic has been used during my original vDSO headers definition and implementation. > We should also keep PROT_READ and PROT_WRITE in getrandom.c , that's > better for readability. Same for MAP_DROPPABLE | MAP_ANONYMOUS. I can't > see the benefit of hiding them in a header. > The idea is not to make the code unreadable but to defer to the architecture the decision of prot and flags avoiding the inclusion of headers coming from the uapi namespace. > I can't see which header provides you with min_t() or ARRAY_SIZE(). > Good point, this needs to be addressed by my patch, I will extend it, do some more testing and post it again next week. > I think you should also work on removing headers included by > arch/x86/include/asm/vdso/gettimeofday.h which is included by > include/vdso/datapage.h : > >#include >#include >#include >#include >#include >#include >#include > > As a comparison, the one from powerpc only includes the following one so > it pulls a lot less non-vdso headers: > >#include >#include >#include >#include > > Christophe This does not seem a concern, in fact I believe that the generic vDSO library should not mandate to the architecture how to organize headers. As far as the requirements are satisfied each architecture should be able to define its own naming and conventions independently. -- Regards, Vincenzo
Re: [PATCH] random: vDSO: Redefine PAGE_SIZE and PAGE_MASK
... >>> Without linux/array_size.h: >>> >>> VDSO32C arch/powerpc/kernel/vdso/vgetrandom-32.o >>> In file included from : >>> /home/chleroy/linux-powerpc/lib/vdso/getrandom.c: In function >>> '__cvdso_getrandom_data': >>> /home/chleroy/linux-powerpc/lib/vdso/getrandom.c:89:40: error: implicit >> If this is the case, those headers should be defined for the powerpc >> implementation only. The generic implementation should be interpreted as the >> minimum common denominator in between all the architectures for what concerns >> the headers. >> > > Sorry, I disagree. You can't rely on necessary headers being included > indirectly > by other arch specific headers. getrandom.c uses ARRAY_SIZE(), it must include > the header that defines ARRAY_SIZE(). > > At the moment, on x86 you get linux/array.h by change through the following > chain, that the reason why the build doesn't break: > > In file included from ./include/linux/kernel.h:16, > from ./include/linux/cpumask.h:11, > from ./arch/x86/include/asm/cpumask.h:5, > from ./arch/x86/include/asm/msr.h:11, > from ./arch/x86/include/asm/vdso/gettimeofday.h:19, > from ./include/vdso/datapage.h:164, > from arch/x86/entry/vdso/../../../../lib/vdso/getrandom.c:9, > > From my point of view you can't expect such a chain from each architecture. > --->8--- > I can't see which header provides you with min_t() or ARRAY_SIZE(). > Good point, this needs to be addressed by my patch, I will extend it, do some more testing and post it again next week. --->8--- As I explained in my other email (snippet above), my patch should address the cases of ARRAY_SIZE() and min_t() which are directly used by getrandom.c. My comment here refers to the cases not directly used by getrandom.c or the Generic vDSO library more in general (if any). Hope this clarifies. > Christophe -- Regards, Vincenzo
