* [PATCH v4 1/3] powerpc: Fix vDSO clock_getres()
2019-05-23 11:21 [PATCH v4 0/3] Fix vDSO clock_getres() Vincenzo Frascino
@ 2019-05-23 11:21 ` Vincenzo Frascino
2019-05-29 13:14 ` Sasha Levin
2019-05-23 11:21 ` [PATCH v4 2/3] s390: " Vincenzo Frascino
2019-05-23 11:21 ` [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres Vincenzo Frascino
2 siblings, 1 reply; 16+ messages in thread
From: Vincenzo Frascino @ 2019-05-23 11:21 UTC (permalink / raw)
To: linux-arch, linuxppc-dev, linux-s390, linux-kselftest
Cc: Arnd Bergmann, Heiko Carstens, stable, Paul Mackerras,
Martin Schwidefsky, Thomas Gleixner, vincenzo.frascino,
Shuah Khan
clock_getres in the vDSO library has to preserve the same behaviour
of posix_get_hrtimer_res().
In particular, posix_get_hrtimer_res() does:
sec = 0;
ns = hrtimer_resolution;
and hrtimer_resolution depends on the enablement of the high
resolution timers that can happen either at compile or at run time.
Fix the powerpc vdso implementation of clock_getres keeping a copy of
hrtimer_resolution in vdso data and using that directly.
Fixes: a7f290dad32e ("[PATCH] powerpc: Merge vdso's and add vdso support
to 32 bits kernel")
Cc: stable@vger.kernel.org
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
Note: This patch is independent from the others in this series, hence it
can be merged singularly by the powerpc maintainers.
arch/powerpc/include/asm/vdso_datapage.h | 2 ++
arch/powerpc/kernel/asm-offsets.c | 2 +-
arch/powerpc/kernel/time.c | 1 +
arch/powerpc/kernel/vdso32/gettimeofday.S | 7 +++++--
arch/powerpc/kernel/vdso64/gettimeofday.S | 7 +++++--
5 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/vdso_datapage.h b/arch/powerpc/include/asm/vdso_datapage.h
index bbc06bd72b1f..4333b9a473dc 100644
--- a/arch/powerpc/include/asm/vdso_datapage.h
+++ b/arch/powerpc/include/asm/vdso_datapage.h
@@ -86,6 +86,7 @@ struct vdso_data {
__s32 wtom_clock_nsec; /* Wall to monotonic clock nsec */
__s64 wtom_clock_sec; /* Wall to monotonic clock sec */
struct timespec stamp_xtime; /* xtime as at tb_orig_stamp */
+ __u32 hrtimer_res; /* hrtimer resolution */
__u32 syscall_map_64[SYSCALL_MAP_SIZE]; /* map of syscalls */
__u32 syscall_map_32[SYSCALL_MAP_SIZE]; /* map of syscalls */
};
@@ -107,6 +108,7 @@ struct vdso_data {
__s32 wtom_clock_nsec;
struct timespec stamp_xtime; /* xtime as at tb_orig_stamp */
__u32 stamp_sec_fraction; /* fractional seconds of stamp_xtime */
+ __u32 hrtimer_res; /* hrtimer resolution */
__u32 syscall_map_32[SYSCALL_MAP_SIZE]; /* map of syscalls */
__u32 dcache_block_size; /* L1 d-cache block size */
__u32 icache_block_size; /* L1 i-cache block size */
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index 8e02444e9d3d..dfc40f29f2b9 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -389,6 +389,7 @@ int main(void)
OFFSET(WTOM_CLOCK_NSEC, vdso_data, wtom_clock_nsec);
OFFSET(STAMP_XTIME, vdso_data, stamp_xtime);
OFFSET(STAMP_SEC_FRAC, vdso_data, stamp_sec_fraction);
+ OFFSET(CLOCK_REALTIME_RES, vdso_data, hrtimer_res);
OFFSET(CFG_ICACHE_BLOCKSZ, vdso_data, icache_block_size);
OFFSET(CFG_DCACHE_BLOCKSZ, vdso_data, dcache_block_size);
OFFSET(CFG_ICACHE_LOGBLOCKSZ, vdso_data, icache_log_block_size);
@@ -419,7 +420,6 @@ int main(void)
DEFINE(CLOCK_REALTIME_COARSE, CLOCK_REALTIME_COARSE);
DEFINE(CLOCK_MONOTONIC_COARSE, CLOCK_MONOTONIC_COARSE);
DEFINE(NSEC_PER_SEC, NSEC_PER_SEC);
- DEFINE(CLOCK_REALTIME_RES, MONOTONIC_RES_NSEC);
#ifdef CONFIG_BUG
DEFINE(BUG_ENTRY_SIZE, sizeof(struct bug_entry));
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 325d60633dfa..4ea4e9d7a58e 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -963,6 +963,7 @@ void update_vsyscall(struct timekeeper *tk)
vdso_data->wtom_clock_nsec = tk->wall_to_monotonic.tv_nsec;
vdso_data->stamp_xtime = xt;
vdso_data->stamp_sec_fraction = frac_sec;
+ vdso_data->hrtimer_res = hrtimer_resolution;
smp_wmb();
++(vdso_data->tb_update_count);
}
diff --git a/arch/powerpc/kernel/vdso32/gettimeofday.S b/arch/powerpc/kernel/vdso32/gettimeofday.S
index afd516b572f8..2b5f9e83c610 100644
--- a/arch/powerpc/kernel/vdso32/gettimeofday.S
+++ b/arch/powerpc/kernel/vdso32/gettimeofday.S
@@ -160,12 +160,15 @@ V_FUNCTION_BEGIN(__kernel_clock_getres)
cror cr0*4+eq,cr0*4+eq,cr1*4+eq
bne cr0,99f
+ mflr r12
+ .cfi_register lr,r12
+ bl __get_datapage@local
+ lwz r5,CLOCK_REALTIME_RES(r3)
+ mtlr r12
li r3,0
cmpli cr0,r4,0
crclr cr0*4+so
beqlr
- lis r5,CLOCK_REALTIME_RES@h
- ori r5,r5,CLOCK_REALTIME_RES@l
stw r3,TSPC32_TV_SEC(r4)
stw r5,TSPC32_TV_NSEC(r4)
blr
diff --git a/arch/powerpc/kernel/vdso64/gettimeofday.S b/arch/powerpc/kernel/vdso64/gettimeofday.S
index 1f324c28705b..f07730f73d5e 100644
--- a/arch/powerpc/kernel/vdso64/gettimeofday.S
+++ b/arch/powerpc/kernel/vdso64/gettimeofday.S
@@ -190,12 +190,15 @@ V_FUNCTION_BEGIN(__kernel_clock_getres)
cror cr0*4+eq,cr0*4+eq,cr1*4+eq
bne cr0,99f
+ mflr r12
+ .cfi_register lr,r12
+ bl V_LOCAL_FUNC(__get_datapage)
+ lwz r5,CLOCK_REALTIME_RES(r3)
+ mtlr r12
li r3,0
cmpldi cr0,r4,0
crclr cr0*4+so
beqlr
- lis r5,CLOCK_REALTIME_RES@h
- ori r5,r5,CLOCK_REALTIME_RES@l
std r3,TSPC64_TV_SEC(r4)
std r5,TSPC64_TV_NSEC(r4)
blr
--
2.21.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v4 1/3] powerpc: Fix vDSO clock_getres()
2019-05-23 11:21 ` [PATCH v4 1/3] powerpc: " Vincenzo Frascino
@ 2019-05-29 13:14 ` Sasha Levin
0 siblings, 0 replies; 16+ messages in thread
From: Sasha Levin @ 2019-05-29 13:14 UTC (permalink / raw)
To: Sasha Levin, Vincenzo Frascino, linux-arch, linuxppc-dev
Cc: , stable, Paul Mackerras, vincenzo.frascino
Hi,
[This is an automated email]
This commit has been processed because it contains a "Fixes:" tag,
fixing commit: a7f290dad32ee [PATCH] powerpc: Merge vdso's and add vdso support to 32 bits kernel.
The bot has tested the following trees: v5.1.4, v5.0.18, v4.19.45, v4.14.121, v4.9.178, v4.4.180, v3.18.140.
v5.1.4: Build OK!
v5.0.18: Build OK!
v4.19.45: Build OK!
v4.14.121: Failed to apply! Possible dependencies:
5c929885f1bb4 ("powerpc/vdso64: Add support for CLOCK_{REALTIME/MONOTONIC}_COARSE")
b5b4453e7912f ("powerpc/vdso64: Fix CLOCK_MONOTONIC inconsistencies across Y2038")
v4.9.178: Failed to apply! Possible dependencies:
4546561551106 ("powerpc/asm: Use OFFSET macro in asm-offsets.c")
5c929885f1bb4 ("powerpc/vdso64: Add support for CLOCK_{REALTIME/MONOTONIC}_COARSE")
5d451a87e5ebb ("powerpc/64: Retrieve number of L1 cache sets from device-tree")
7c5b06cadf274 ("KVM: PPC: Book3S HV: Adapt TLB invalidations to work on POWER9")
83677f551e0a6 ("KVM: PPC: Book3S HV: Adjust host/guest context switch for POWER9")
902e06eb86cd6 ("powerpc/32: Change the stack protector canary value per task")
b5b4453e7912f ("powerpc/vdso64: Fix CLOCK_MONOTONIC inconsistencies across Y2038")
bd067f83b0840 ("powerpc/64: Fix naming of cache block vs. cache line")
e2827fe5c1566 ("powerpc/64: Clean up ppc64_caches using a struct per cache")
e9cf1e085647b ("KVM: PPC: Book3S HV: Add new POWER9 guest-accessible SPRs")
f4c51f841d2ac ("KVM: PPC: Book3S HV: Modify guest entry/exit paths to handle radix guests")
v4.4.180: Failed to apply! Possible dependencies:
153086644fd1f ("powerpc/ftrace: Add support for -mprofile-kernel ftrace ABI")
3eb5d5888dc68 ("powerpc: Add ppc_strict_facility_enable boot option")
4546561551106 ("powerpc/asm: Use OFFSET macro in asm-offsets.c")
579e633e764e6 ("powerpc: create flush_all_to_thread()")
5c929885f1bb4 ("powerpc/vdso64: Add support for CLOCK_{REALTIME/MONOTONIC}_COARSE")
70fe3d980f5f1 ("powerpc: Restore FPU/VEC/VSX if previously used")
85baa095497f3 ("powerpc/livepatch: Add live patching support on ppc64le")
902e06eb86cd6 ("powerpc/32: Change the stack protector canary value per task")
b5b4453e7912f ("powerpc/vdso64: Fix CLOCK_MONOTONIC inconsistencies across Y2038")
bf76f73c5f655 ("powerpc: enable UBSAN support")
c208505900b23 ("powerpc: create giveup_all()")
d1e1cf2e38def ("powerpc: clean up asm/switch_to.h")
dc4fbba11e466 ("powerpc: Create disable_kernel_{fp,altivec,vsx,spe}()")
f17c4e01e906c ("powerpc/module: Mark module stubs with a magic value")
v3.18.140: Failed to apply! Possible dependencies:
10239733ee861 ("powerpc: Remove bootmem allocator")
2449acc5348b9 ("powerpc/kernel: Enable seccomp filter")
4546561551106 ("powerpc/asm: Use OFFSET macro in asm-offsets.c")
49e4e15619cd7 ("tile: support CONTEXT_TRACKING and thus NOHZ_FULL")
5c929885f1bb4 ("powerpc/vdso64: Add support for CLOCK_{REALTIME/MONOTONIC}_COARSE")
73569d87e2cc5 ("MIPS: OCTEON: Enable little endian kernel.")
817820b0226a1 ("powerpc/iommu: Support "hybrid" iommu/direct DMA ops for coherent_mask < dma_mask")
83fe27ea53116 ("rcu: Make SRCU optional by using CONFIG_SRCU")
85baa095497f3 ("powerpc/livepatch: Add live patching support on ppc64le")
b01aec9b2c7d3 ("EDAC: Cleanup atomic_scrub mess")
b30e759072c18 ("powerpc/mm: Switch to generic RCU get_user_pages_fast")
b5b4453e7912f ("powerpc/vdso64: Fix CLOCK_MONOTONIC inconsistencies across Y2038")
bf76f73c5f655 ("powerpc: enable UBSAN support")
c54b2bf1b5e99 ("powerpc: Add ppc64 hard lockup detector support")
f30c59e921f12 ("mm: Update generic gup implementation to handle hugepage directory")
f47436734dc89 ("tile: Use the more common pr_warn instead of pr_warning")
How should we proceed with this patch?
--
Thanks,
Sasha
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v4 2/3] s390: Fix vDSO clock_getres()
2019-05-23 11:21 [PATCH v4 0/3] Fix vDSO clock_getres() Vincenzo Frascino
2019-05-23 11:21 ` [PATCH v4 1/3] powerpc: " Vincenzo Frascino
@ 2019-05-23 11:21 ` Vincenzo Frascino
2019-05-23 11:21 ` [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres Vincenzo Frascino
2 siblings, 0 replies; 16+ messages in thread
From: Vincenzo Frascino @ 2019-05-23 11:21 UTC (permalink / raw)
To: linux-arch, linuxppc-dev, linux-s390, linux-kselftest
Cc: Arnd Bergmann, Heiko Carstens, Paul Mackerras,
Martin Schwidefsky, Thomas Gleixner, vincenzo.frascino,
Shuah Khan
clock_getres in the vDSO library has to preserve the same behaviour
of posix_get_hrtimer_res().
In particular, posix_get_hrtimer_res() does:
sec = 0;
ns = hrtimer_resolution;
and hrtimer_resolution depends on the enablement of the high
resolution timers that can happen either at compile or at run time.
Fix the s390 vdso implementation of clock_getres keeping a copy of
hrtimer_resolution in vdso data and using that directly.
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Acked-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---
Note: This patch is independent from the others in this series, hence it
can be merged singularly by the s390 maintainers.
arch/s390/include/asm/vdso.h | 1 +
arch/s390/kernel/asm-offsets.c | 2 +-
arch/s390/kernel/time.c | 1 +
arch/s390/kernel/vdso32/clock_getres.S | 12 +++++++-----
arch/s390/kernel/vdso64/clock_getres.S | 10 +++++-----
5 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/arch/s390/include/asm/vdso.h b/arch/s390/include/asm/vdso.h
index 169d7604eb80..f3ba84fa9bd1 100644
--- a/arch/s390/include/asm/vdso.h
+++ b/arch/s390/include/asm/vdso.h
@@ -36,6 +36,7 @@ struct vdso_data {
__u32 tk_shift; /* Shift used for xtime_nsec 0x60 */
__u32 ts_dir; /* TOD steering direction 0x64 */
__u64 ts_end; /* TOD steering end 0x68 */
+ __u32 hrtimer_res; /* hrtimer resolution 0x70 */
};
struct vdso_per_cpu_data {
diff --git a/arch/s390/kernel/asm-offsets.c b/arch/s390/kernel/asm-offsets.c
index 41ac4ad21311..4a229a60b24a 100644
--- a/arch/s390/kernel/asm-offsets.c
+++ b/arch/s390/kernel/asm-offsets.c
@@ -76,6 +76,7 @@ int main(void)
OFFSET(__VDSO_TK_SHIFT, vdso_data, tk_shift);
OFFSET(__VDSO_TS_DIR, vdso_data, ts_dir);
OFFSET(__VDSO_TS_END, vdso_data, ts_end);
+ OFFSET(__VDSO_CLOCK_REALTIME_RES, vdso_data, hrtimer_res);
OFFSET(__VDSO_ECTG_BASE, vdso_per_cpu_data, ectg_timer_base);
OFFSET(__VDSO_ECTG_USER, vdso_per_cpu_data, ectg_user_time);
OFFSET(__VDSO_CPU_NR, vdso_per_cpu_data, cpu_nr);
@@ -87,7 +88,6 @@ int main(void)
DEFINE(__CLOCK_REALTIME_COARSE, CLOCK_REALTIME_COARSE);
DEFINE(__CLOCK_MONOTONIC_COARSE, CLOCK_MONOTONIC_COARSE);
DEFINE(__CLOCK_THREAD_CPUTIME_ID, CLOCK_THREAD_CPUTIME_ID);
- DEFINE(__CLOCK_REALTIME_RES, MONOTONIC_RES_NSEC);
DEFINE(__CLOCK_COARSE_RES, LOW_RES_NSEC);
BLANK();
/* idle data offsets */
diff --git a/arch/s390/kernel/time.c b/arch/s390/kernel/time.c
index e8766beee5ad..8ea9db599d38 100644
--- a/arch/s390/kernel/time.c
+++ b/arch/s390/kernel/time.c
@@ -310,6 +310,7 @@ void update_vsyscall(struct timekeeper *tk)
vdso_data->tk_mult = tk->tkr_mono.mult;
vdso_data->tk_shift = tk->tkr_mono.shift;
+ vdso_data->hrtimer_res = hrtimer_resolution;
smp_wmb();
++vdso_data->tb_update_count;
}
diff --git a/arch/s390/kernel/vdso32/clock_getres.S b/arch/s390/kernel/vdso32/clock_getres.S
index eaf9cf1417f6..fecd7684c645 100644
--- a/arch/s390/kernel/vdso32/clock_getres.S
+++ b/arch/s390/kernel/vdso32/clock_getres.S
@@ -18,20 +18,22 @@
__kernel_clock_getres:
CFI_STARTPROC
basr %r1,0
- la %r1,4f-.(%r1)
+10: al %r1,4f-10b(%r1)
+ l %r0,__VDSO_CLOCK_REALTIME_RES(%r1)
chi %r2,__CLOCK_REALTIME
je 0f
chi %r2,__CLOCK_MONOTONIC
je 0f
- la %r1,5f-4f(%r1)
+ basr %r1,0
+ la %r1,5f-.(%r1)
+ l %r0,0(%r1)
chi %r2,__CLOCK_REALTIME_COARSE
je 0f
chi %r2,__CLOCK_MONOTONIC_COARSE
jne 3f
0: ltr %r3,%r3
jz 2f /* res == NULL */
-1: l %r0,0(%r1)
- xc 0(4,%r3),0(%r3) /* set tp->tv_sec to zero */
+1: xc 0(4,%r3),0(%r3) /* set tp->tv_sec to zero */
st %r0,4(%r3) /* store tp->tv_usec */
2: lhi %r2,0
br %r14
@@ -39,6 +41,6 @@ __kernel_clock_getres:
svc 0
br %r14
CFI_ENDPROC
-4: .long __CLOCK_REALTIME_RES
+4: .long _vdso_data - 10b
5: .long __CLOCK_COARSE_RES
.size __kernel_clock_getres,.-__kernel_clock_getres
diff --git a/arch/s390/kernel/vdso64/clock_getres.S b/arch/s390/kernel/vdso64/clock_getres.S
index 081435398e0a..022b58c980db 100644
--- a/arch/s390/kernel/vdso64/clock_getres.S
+++ b/arch/s390/kernel/vdso64/clock_getres.S
@@ -17,12 +17,14 @@
.type __kernel_clock_getres,@function
__kernel_clock_getres:
CFI_STARTPROC
- larl %r1,4f
+ larl %r1,3f
+ lg %r0,0(%r1)
cghi %r2,__CLOCK_REALTIME_COARSE
je 0f
cghi %r2,__CLOCK_MONOTONIC_COARSE
je 0f
- larl %r1,3f
+ larl %r1,_vdso_data
+ l %r0,__VDSO_CLOCK_REALTIME_RES(%r1)
cghi %r2,__CLOCK_REALTIME
je 0f
cghi %r2,__CLOCK_MONOTONIC
@@ -36,7 +38,6 @@ __kernel_clock_getres:
jz 2f
0: ltgr %r3,%r3
jz 1f /* res == NULL */
- lg %r0,0(%r1)
xc 0(8,%r3),0(%r3) /* set tp->tv_sec to zero */
stg %r0,8(%r3) /* store tp->tv_usec */
1: lghi %r2,0
@@ -45,6 +46,5 @@ __kernel_clock_getres:
svc 0
br %r14
CFI_ENDPROC
-3: .quad __CLOCK_REALTIME_RES
-4: .quad __CLOCK_COARSE_RES
+3: .quad __CLOCK_COARSE_RES
.size __kernel_clock_getres,.-__kernel_clock_getres
--
2.21.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
2019-05-23 11:21 [PATCH v4 0/3] Fix vDSO clock_getres() Vincenzo Frascino
2019-05-23 11:21 ` [PATCH v4 1/3] powerpc: " Vincenzo Frascino
2019-05-23 11:21 ` [PATCH v4 2/3] s390: " Vincenzo Frascino
@ 2019-05-23 11:21 ` Vincenzo Frascino
2019-05-28 6:19 ` Michael Ellerman
2 siblings, 1 reply; 16+ messages in thread
From: Vincenzo Frascino @ 2019-05-23 11:21 UTC (permalink / raw)
To: linux-arch, linuxppc-dev, linux-s390, linux-kselftest
Cc: Arnd Bergmann, Heiko Carstens, Paul Mackerras,
Martin Schwidefsky, Thomas Gleixner, vincenzo.frascino,
Shuah Khan
The current version of the multiarch vDSO selftest verifies only
gettimeofday.
Extend the vDSO selftest to clock_getres, to verify that the
syscall and the vDSO library function return the same information.
The extension has been used to verify the hrtimer_resoltion fix.
Cc: Shuah Khan <shuah@kernel.org>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
Note: This patch is independent from the others in this series, hence it
can be merged singularly by the kselftest maintainers.
tools/testing/selftests/vDSO/Makefile | 2 +
.../selftests/vDSO/vdso_clock_getres.c | 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 000000000000..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 <elf.h>
+#include <err.h>
+#include <fcntl.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <time.h>
+#include <sys/auxv.h>
+#include <sys/mman.h>
+#include <sys/time.h>
+#include <unistd.h>
+#include <sys/syscall.h>
+
+#include "../kselftest.h"
+
+static long syscall_clock_getres(clockid_t _clkid, struct timespec *_ts)
+{
+ long ret;
+
+ ret = syscall(SYS_clock_getres, _clkid, _ts);
+
+ return ret;
+}
+
+const char *vdso_clock_name[12] = {
+ "CLOCK_REALTIME",
+ "CLOCK_MONOTONIC",
+ "CLOCK_PROCESS_CPUTIME_ID",
+ "CLOCK_THREAD_CPUTIME_ID",
+ "CLOCK_MONOTONIC_RAW",
+ "CLOCK_REALTIME_COARSE",
+ "CLOCK_MONOTONIC_COARSE",
+ "CLOCK_BOOTTIME",
+ "CLOCK_REALTIME_ALARM",
+ "CLOCK_BOOTTIME_ALARM",
+ "CLOCK_SGI_CYCLE",
+ "CLOCK_TAI",
+};
+
+/*
+ * This function calls clock_getres in vdso and by system call
+ * with different values for clock_id.
+ *
+ * Example of output:
+ *
+ * clock_id: CLOCK_REALTIME [PASS]
+ * clock_id: CLOCK_BOOTTIME [PASS]
+ * clock_id: CLOCK_TAI [PASS]
+ * clock_id: CLOCK_REALTIME_COARSE [PASS]
+ * clock_id: CLOCK_MONOTONIC [PASS]
+ * clock_id: CLOCK_MONOTONIC_RAW [PASS]
+ * clock_id: CLOCK_MONOTONIC_COARSE [PASS]
+ */
+static inline int vdso_test_clock(unsigned int clock_id)
+{
+ struct timespec x, y;
+
+ printf("clock_id: %s", vdso_clock_name[clock_id]);
+ clock_getres(clock_id, &x);
+ syscall_clock_getres(clock_id, &y);
+
+ if ((x.tv_sec != y.tv_sec) || (x.tv_sec != y.tv_sec)) {
+ printf(" [FAIL]\n");
+ return KSFT_FAIL;
+ }
+
+ printf(" [PASS]\n");
+ return 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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
2019-05-23 11:21 ` [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres Vincenzo Frascino
@ 2019-05-28 6:19 ` Michael Ellerman
2019-05-28 11:57 ` Vincenzo Frascino
0 siblings, 1 reply; 16+ messages in thread
From: Michael Ellerman @ 2019-05-28 6:19 UTC (permalink / raw)
To: Vincenzo Frascino, linux-arch, linuxppc-dev, linux-s390, linux-kselftest
Cc: Arnd Bergmann, Heiko Carstens, Paul Mackerras,
Martin Schwidefsky, Thomas Gleixner, vincenzo.frascino,
Shuah Khan
Vincenzo Frascino <vincenzo.frascino@arm.com> 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?
# 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 <shuah@kernel.org>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> ---
>
> Note: This patch is independent from the others in this series, hence it
> can be merged singularly by the kselftest maintainers.
>
> tools/testing/selftests/vDSO/Makefile | 2 +
> .../selftests/vDSO/vdso_clock_getres.c | 124 ++++++++++++++++++
> 2 files changed, 126 insertions(+)
> create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
2019-05-28 6:19 ` Michael Ellerman
@ 2019-05-28 11:57 ` Vincenzo Frascino
2019-05-28 17:01 ` Christophe Leroy
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Vincenzo Frascino @ 2019-05-28 11:57 UTC (permalink / raw)
To: Michael Ellerman, linux-arch, linuxppc-dev, linux-s390, linux-kselftest
Cc: Arnd Bergmann, Heiko Carstens, Paul Mackerras,
Martin Schwidefsky, Thomas Gleixner, Shuah Khan
Hi Michael,
thank you for your reply.
On 28/05/2019 07:19, Michael Ellerman wrote:
> Vincenzo Frascino <vincenzo.frascino@arm.com> 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 <shuah@kernel.org>
>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>> ---
>>
>> Note: This patch is independent from the others in this series, hence it
>> can be merged singularly by the kselftest maintainers.
>>
>> tools/testing/selftests/vDSO/Makefile | 2 +
>> .../selftests/vDSO/vdso_clock_getres.c | 124 ++++++++++++++++++
>> 2 files changed, 126 insertions(+)
>> create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
--
Regards,
Vincenzo
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
2019-05-28 11:57 ` Vincenzo Frascino
@ 2019-05-28 17:01 ` Christophe Leroy
2019-05-28 17:05 ` Vincenzo Frascino
2019-06-04 13:16 ` Christophe Leroy
2019-06-13 15:22 ` Vincenzo Frascino
2 siblings, 1 reply; 16+ messages in thread
From: Christophe Leroy @ 2019-05-28 17:01 UTC (permalink / raw)
To: Vincenzo Frascino
Cc: linux-arch, linux-s390, Arnd Bergmann, Heiko Carstens,
linuxppc-dev, Paul Mackerras, linux-kselftest,
Martin Schwidefsky, Thomas Gleixner, Shuah Khan
Vincenzo Frascino <vincenzo.frascino@arm.com> a écrit :
> Hi Michael,
>
> thank you for your reply.
>
> On 28/05/2019 07:19, Michael Ellerman wrote:
>> Vincenzo Frascino <vincenzo.frascino@arm.com> 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
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 <shuah@kernel.org>
>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>>> ---
>>>
>>> Note: This patch is independent from the others in this series, hence it
>>> can be merged singularly by the kselftest maintainers.
>>>
>>> tools/testing/selftests/vDSO/Makefile | 2 +
>>> .../selftests/vDSO/vdso_clock_getres.c | 124 ++++++++++++++++++
>>> 2 files changed, 126 insertions(+)
>>> create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
>
> --
> Regards,
> Vincenzo
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
2019-05-28 17:01 ` Christophe Leroy
@ 2019-05-28 17:05 ` Vincenzo Frascino
0 siblings, 0 replies; 16+ messages in thread
From: Vincenzo Frascino @ 2019-05-28 17:05 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-arch, linux-s390, Arnd Bergmann, Heiko Carstens,
linuxppc-dev, Paul Mackerras, linux-kselftest,
Martin Schwidefsky, Thomas Gleixner, Shuah Khan
Hi Christophe,
On 28/05/2019 18:01, Christophe Leroy wrote:
> Vincenzo Frascino <vincenzo.frascino@arm.com> a écrit :
>
>> Hi Michael,
>>
>> thank you for your reply.
>>
>> On 28/05/2019 07:19, Michael Ellerman wrote:
>>> Vincenzo Frascino <vincenzo.frascino@arm.com> 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 <shuah@kernel.org>
>>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>>>> ---
>>>>
>>>> Note: This patch is independent from the others in this series, hence it
>>>> can be merged singularly by the kselftest maintainers.
>>>>
>>>> tools/testing/selftests/vDSO/Makefile | 2 +
>>>> .../selftests/vDSO/vdso_clock_getres.c | 124 ++++++++++++++++++
>>>> 2 files changed, 126 insertions(+)
>>>> create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
>>
>> --
>> Regards,
>> Vincenzo
>
>
--
Regards,
Vincenzo
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
2019-05-28 11:57 ` Vincenzo Frascino
2019-05-28 17:01 ` Christophe Leroy
@ 2019-06-04 13:16 ` Christophe Leroy
2019-06-04 13:32 ` Vincenzo Frascino
2019-06-13 15:22 ` Vincenzo Frascino
2 siblings, 1 reply; 16+ messages in thread
From: Christophe Leroy @ 2019-06-04 13:16 UTC (permalink / raw)
To: Vincenzo Frascino, Michael Ellerman, linux-arch, linuxppc-dev,
linux-s390, linux-kselftest
Cc: Arnd Bergmann, Heiko Carstens, Paul Mackerras,
Martin Schwidefsky, Thomas Gleixner, Shuah Khan
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 <vincenzo.frascino@arm.com> 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 ?
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 <shuah@kernel.org>
>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>>> ---
>>>
>>> Note: This patch is independent from the others in this series, hence it
>>> can be merged singularly by the kselftest maintainers.
>>>
>>> tools/testing/selftests/vDSO/Makefile | 2 +
>>> .../selftests/vDSO/vdso_clock_getres.c | 124 ++++++++++++++++++
>>> 2 files changed, 126 insertions(+)
>>> create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
2019-06-04 13:16 ` Christophe Leroy
@ 2019-06-04 13:32 ` Vincenzo Frascino
2019-06-04 13:39 ` Christophe Leroy
0 siblings, 1 reply; 16+ messages in thread
From: Vincenzo Frascino @ 2019-06-04 13:32 UTC (permalink / raw)
To: Christophe Leroy, Michael Ellerman, linux-arch, linuxppc-dev,
linux-s390, linux-kselftest
Cc: Arnd Bergmann, Heiko Carstens, Paul Mackerras,
Martin Schwidefsky, Thomas Gleixner, Shuah Khan
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 <vincenzo.frascino@arm.com> 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
cror cr0*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 <shuah@kernel.org>
>>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>>>> ---
>>>>
>>>> Note: This patch is independent from the others in this series, hence it
>>>> can be merged singularly by the kselftest maintainers.
>>>>
>>>> tools/testing/selftests/vDSO/Makefile | 2 +
>>>> .../selftests/vDSO/vdso_clock_getres.c | 124 ++++++++++++++++++
>>>> 2 files changed, 126 insertions(+)
>>>> create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
>>
--
Regards,
Vincenzo
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
2019-06-04 13:32 ` Vincenzo Frascino
@ 2019-06-04 13:39 ` Christophe Leroy
2019-06-04 13:43 ` Vincenzo Frascino
0 siblings, 1 reply; 16+ messages in thread
From: Christophe Leroy @ 2019-06-04 13:39 UTC (permalink / raw)
To: Vincenzo Frascino, Michael Ellerman, linux-arch, linuxppc-dev,
linux-s390, linux-kselftest
Cc: Arnd Bergmann, Heiko Carstens, Paul Mackerras,
Martin Schwidefsky, Thomas Gleixner, Shuah Khan
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 <vincenzo.frascino@arm.com> 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
> cror cr0*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 ?
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 <shuah@kernel.org>
>>>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>>>>> ---
>>>>>
>>>>> Note: This patch is independent from the others in this series, hence it
>>>>> can be merged singularly by the kselftest maintainers.
>>>>>
>>>>> tools/testing/selftests/vDSO/Makefile | 2 +
>>>>> .../selftests/vDSO/vdso_clock_getres.c | 124 ++++++++++++++++++
>>>>> 2 files changed, 126 insertions(+)
>>>>> create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
>>>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
2019-06-04 13:39 ` Christophe Leroy
@ 2019-06-04 13:43 ` Vincenzo Frascino
2019-06-04 13:52 ` Christophe Leroy
0 siblings, 1 reply; 16+ messages in thread
From: Vincenzo Frascino @ 2019-06-04 13:43 UTC (permalink / raw)
To: Christophe Leroy, Michael Ellerman, linux-arch, linuxppc-dev,
linux-s390, linux-kselftest
Cc: Arnd Bergmann, Heiko Carstens, Paul Mackerras,
Martin Schwidefsky, Thomas Gleixner, Shuah Khan
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 <vincenzo.frascino@arm.com> 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
>> cror cr0*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 <shuah@kernel.org>
>>>>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>>>>>> ---
>>>>>>
>>>>>> Note: This patch is independent from the others in this series, hence it
>>>>>> can be merged singularly by the kselftest maintainers.
>>>>>>
>>>>>> tools/testing/selftests/vDSO/Makefile | 2 +
>>>>>> .../selftests/vDSO/vdso_clock_getres.c | 124 ++++++++++++++++++
>>>>>> 2 files changed, 126 insertions(+)
>>>>>> create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
>>>>
>>
--
Regards,
Vincenzo
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
2019-06-04 13:43 ` Vincenzo Frascino
@ 2019-06-04 13:52 ` Christophe Leroy
2019-06-04 14:21 ` Vincenzo Frascino
0 siblings, 1 reply; 16+ messages in thread
From: Christophe Leroy @ 2019-06-04 13:52 UTC (permalink / raw)
To: Vincenzo Frascino, Michael Ellerman, linux-arch, linuxppc-dev,
linux-s390, linux-kselftest
Cc: Arnd Bergmann, Heiko Carstens, Paul Mackerras,
Martin Schwidefsky, Thomas Gleixner, Shuah Khan
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 <vincenzo.frascino@arm.com> 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
>>> cror cr0*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.
No problem for myself.
By the way, next time (or next spin ?) I recommend you to handle your
patches independently and not as a series since they are all
independant. It would have avoided confusion and the need for you to
resend all 3 patches everytime you did a change in one of them.
Christophe
>
>> 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 <shuah@kernel.org>
>>>>>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>>>>>>> ---
>>>>>>>
>>>>>>> Note: This patch is independent from the others in this series, hence it
>>>>>>> can be merged singularly by the kselftest maintainers.
>>>>>>>
>>>>>>> tools/testing/selftests/vDSO/Makefile | 2 +
>>>>>>> .../selftests/vDSO/vdso_clock_getres.c | 124 ++++++++++++++++++
>>>>>>> 2 files changed, 126 insertions(+)
>>>>>>> create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
>>>>>
>>>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
2019-06-04 13:52 ` Christophe Leroy
@ 2019-06-04 14:21 ` Vincenzo Frascino
0 siblings, 0 replies; 16+ messages in thread
From: Vincenzo Frascino @ 2019-06-04 14:21 UTC (permalink / raw)
To: Christophe Leroy, Michael Ellerman, linux-arch, linuxppc-dev,
linux-s390, linux-kselftest
Cc: Arnd Bergmann, Heiko Carstens, Paul Mackerras,
Martin Schwidefsky, Thomas Gleixner, Shuah Khan
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 <vincenzo.frascino@arm.com> 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
>>>> cror cr0*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.
>
> No problem for myself.
>
> By the way, next time (or next spin ?) I recommend you to handle your
> patches independently and not as a series since they are all
> independant. It would have avoided confusion and the need for you to
> resend all 3 patches everytime you did a change in one of them.
>
Thanks for the advise, I will do next time.
> Christophe
>
>>
>>> 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 <shuah@kernel.org>
>>>>>>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>>>>>>>> ---
>>>>>>>>
>>>>>>>> Note: This patch is independent from the others in this series, hence it
>>>>>>>> can be merged singularly by the kselftest maintainers.
>>>>>>>>
>>>>>>>> tools/testing/selftests/vDSO/Makefile | 2 +
>>>>>>>> .../selftests/vDSO/vdso_clock_getres.c | 124 ++++++++++++++++++
>>>>>>>> 2 files changed, 126 insertions(+)
>>>>>>>> create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
>>>>>>
>>>>
>>
--
Regards,
Vincenzo
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
2019-05-28 11:57 ` Vincenzo Frascino
2019-05-28 17:01 ` Christophe Leroy
2019-06-04 13:16 ` Christophe Leroy
@ 2019-06-13 15:22 ` Vincenzo Frascino
2 siblings, 0 replies; 16+ messages in thread
From: Vincenzo Frascino @ 2019-06-13 15:22 UTC (permalink / raw)
To: Michael Ellerman, linux-arch, linuxppc-dev, linux-s390, linux-kselftest
Cc: Arnd Bergmann, Heiko Carstens, Paul Mackerras,
Martin Schwidefsky, Thomas Gleixner, Shuah Khan
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 <vincenzo.frascino@arm.com> 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 <shuah@kernel.org>
>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>>> ---
>>>
>>> Note: This patch is independent from the others in this series, hence it
>>> can be merged singularly by the kselftest maintainers.
>>>
>>> tools/testing/selftests/vDSO/Makefile | 2 +
>>> .../selftests/vDSO/vdso_clock_getres.c | 124 ++++++++++++++++++
>>> 2 files changed, 126 insertions(+)
>>> create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
>
^ permalink raw reply [flat|nested] 16+ messages in thread