linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] Fix vDSO clock_getres()
@ 2019-05-22 11:07 Vincenzo Frascino
  2019-05-22 11:07 ` [PATCH v3 1/3] powerpc: " Vincenzo Frascino
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Vincenzo Frascino @ 2019-05-22 11:07 UTC (permalink / raw)
  To: linux-arch, linuxppc-dev, linux-s390, linux-kselftest
  Cc: Arnd Bergmann, Heiko Carstens, Paul Mackerras,
	Martin Schwidefsky, Thomas Gleixner, 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.

A possible fix is to change the vdso implementation of clock_getres,
keeping a copy of hrtimer_resolution in vdso data and using that
directly [1].

This patchset implements the proposed fix for arm64, powerpc, s390,
nds32 and adds a test to verify that the syscall and the vdso library
implementation of clock_getres return the same values.

Even if these patches are unified by the same topic, there is no
dependency between them, hence they can be merged singularly by each
arch maintainer.

Note: arm64 and nds32 respective fixes have been merged in 5.2-rc1,
hence they have been removed from this series.

[1] https://marc.info/?l=linux-arm-kernel&m=155110381930196&w=2

Changes:
--------
v3:
  - Rebased on 5.2-rc1.
  - Addressed review comments.
v2:
  - Rebased on 5.1-rc5.
  - Addressed review comments.

Cc: Christophe Leroy <christophe.leroy@c-s.fr>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>

Vincenzo Frascino (3):
  powerpc: Fix vDSO clock_getres()
  s390: Fix vDSO clock_getres()
  kselftest: Extend vDSO selftest to clock_getres

 arch/powerpc/include/asm/vdso_datapage.h      |   2 +
 arch/powerpc/kernel/asm-offsets.c             |   2 +-
 arch/powerpc/kernel/time.c                    |   1 +
 arch/powerpc/kernel/vdso32/gettimeofday.S     |   7 +-
 arch/powerpc/kernel/vdso64/gettimeofday.S     |   7 +-
 arch/s390/include/asm/vdso.h                  |   1 +
 arch/s390/kernel/asm-offsets.c                |   2 +-
 arch/s390/kernel/time.c                       |   1 +
 arch/s390/kernel/vdso32/clock_getres.S        |  12 +-
 arch/s390/kernel/vdso64/clock_getres.S        |  10 +-
 tools/testing/selftests/vDSO/Makefile         |   2 +
 .../selftests/vDSO/vdso_clock_getres.c        | 137 ++++++++++++++++++
 12 files changed, 168 insertions(+), 16 deletions(-)
 create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c

-- 
2.21.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 1/3] powerpc: Fix vDSO clock_getres()
  2019-05-22 11:07 [PATCH v3 0/3] Fix vDSO clock_getres() Vincenzo Frascino
@ 2019-05-22 11:07 ` Vincenzo Frascino
  2019-05-29 13:14   ` Sasha Levin
  2019-05-22 11:07 ` [PATCH v3 2/3] s390: " Vincenzo Frascino
  2019-05-22 11:07 ` [PATCH v3 3/3] kselftest: Extend vDSO selftest to clock_getres Vincenzo Frascino
  2 siblings, 1 reply; 8+ messages in thread
From: Vincenzo Frascino @ 2019-05-22 11:07 UTC (permalink / raw)
  To: linux-arch, linuxppc-dev, linux-s390, linux-kselftest
  Cc: Arnd Bergmann, Heiko Carstens, stable, Paul Mackerras,
	Martin Schwidefsky, Thomas Gleixner, 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] 8+ messages in thread

* [PATCH v3 2/3] s390: Fix vDSO clock_getres()
  2019-05-22 11:07 [PATCH v3 0/3] Fix vDSO clock_getres() Vincenzo Frascino
  2019-05-22 11:07 ` [PATCH v3 1/3] powerpc: " Vincenzo Frascino
@ 2019-05-22 11:07 ` Vincenzo Frascino
  2019-05-22 11:07 ` [PATCH v3 3/3] kselftest: Extend vDSO selftest to clock_getres Vincenzo Frascino
  2 siblings, 0 replies; 8+ messages in thread
From: Vincenzo Frascino @ 2019-05-22 11:07 UTC (permalink / raw)
  To: linux-arch, linuxppc-dev, linux-s390, linux-kselftest
  Cc: Arnd Bergmann, Heiko Carstens, Paul Mackerras,
	Martin Schwidefsky, Thomas Gleixner, 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] 8+ messages in thread

* [PATCH v3 3/3] kselftest: Extend vDSO selftest to clock_getres
  2019-05-22 11:07 [PATCH v3 0/3] Fix vDSO clock_getres() Vincenzo Frascino
  2019-05-22 11:07 ` [PATCH v3 1/3] powerpc: " Vincenzo Frascino
  2019-05-22 11:07 ` [PATCH v3 2/3] s390: " Vincenzo Frascino
@ 2019-05-22 11:07 ` Vincenzo Frascino
  2019-05-22 11:50   ` Christophe Leroy
  2 siblings, 1 reply; 8+ messages in thread
From: Vincenzo Frascino @ 2019-05-22 11:07 UTC (permalink / raw)
  To: linux-arch, linuxppc-dev, linux-s390, linux-kselftest
  Cc: Arnd Bergmann, Heiko Carstens, Paul Mackerras,
	Martin Schwidefsky, Thomas Gleixner, 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        | 137 ++++++++++++++++++
 2 files changed, 139 insertions(+)
 create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c

diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile
index 9e03d61f52fd..d5c5bfdf1ac1 100644
--- a/tools/testing/selftests/vDSO/Makefile
+++ b/tools/testing/selftests/vDSO/Makefile
@@ -5,6 +5,7 @@ uname_M := $(shell uname -m 2>/dev/null || echo not)
 ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/)
 
 TEST_GEN_PROGS := $(OUTPUT)/vdso_test
+TEST_GEN_PROGS += $(OUTPUT)/vdso_clock_getres
 ifeq ($(ARCH),x86)
 TEST_GEN_PROGS += $(OUTPUT)/vdso_standalone_test_x86
 endif
@@ -18,6 +19,7 @@ endif
 
 all: $(TEST_GEN_PROGS)
 $(OUTPUT)/vdso_test: parse_vdso.c vdso_test.c
+$(OUTPUT)/vdso_clock_getres: vdso_clock_getres.c
 $(OUTPUT)/vdso_standalone_test_x86: vdso_standalone_test_x86.c parse_vdso.c
 	$(CC) $(CFLAGS) $(CFLAGS_vdso_standalone_test_x86) \
 		vdso_standalone_test_x86.c parse_vdso.c \
diff --git a/tools/testing/selftests/vDSO/vdso_clock_getres.c b/tools/testing/selftests/vDSO/vdso_clock_getres.c
new file mode 100644
index 000000000000..341a9bc34ffc
--- /dev/null
+++ b/tools/testing/selftests/vDSO/vdso_clock_getres.c
@@ -0,0 +1,137 @@
+// SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note
+/*
+ * vdso_clock_getres.c: Sample code to test clock_getres.
+ * Copyright (c) 2019 Arm Ltd.
+ *
+ * Compile with:
+ * gcc -std=gnu99 vdso_clock_getres.c
+ *
+ * Tested on ARM, ARM64, MIPS32, x86 (32-bit and 64-bit),
+ * Power (32-bit and 64-bit), S390x (32-bit and 64-bit).
+ * Might work on other architectures.
+ */
+
+#define _GNU_SOURCE
+#include <elf.h>
+#include <err.h>
+#include <fcntl.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <time.h>
+#include <sys/auxv.h>
+#include <sys/mman.h>
+#include <sys/time.h>
+#include <unistd.h>
+#include <sys/syscall.h>
+
+#include "../kselftest.h"
+
+static long syscall_clock_getres(clockid_t _clkid, struct timespec *_ts)
+{
+	long ret;
+
+	ret = syscall(SYS_clock_getres, _clkid, _ts);
+
+	return ret;
+}
+
+const char *vdso_clock_name[12] = {
+	"CLOCK_REALTIME",
+	"CLOCK_MONOTONIC",
+	"CLOCK_PROCESS_CPUTIME_ID",
+	"CLOCK_THREAD_CPUTIME_ID",
+	"CLOCK_MONOTONIC_RAW",
+	"CLOCK_REALTIME_COARSE",
+	"CLOCK_MONOTONIC_COARSE",
+	"CLOCK_BOOTTIME",
+	"CLOCK_REALTIME_ALARM",
+	"CLOCK_BOOTTIME_ALARM",
+	"CLOCK_SGI_CYCLE",
+	"CLOCK_TAI",
+};
+
+/*
+ * This function calls clock_getres in vdso and by system call
+ * with different values for clock_id.
+ *
+ * Example of output:
+ *
+ * clock_id: CLOCK_REALTIME [PASS]
+ * clock_id: CLOCK_BOOTTIME [PASS]
+ * clock_id: CLOCK_TAI [PASS]
+ * clock_id: CLOCK_REALTIME_COARSE [PASS]
+ * clock_id: CLOCK_MONOTONIC [PASS]
+ * clock_id: CLOCK_MONOTONIC_RAW [PASS]
+ * clock_id: CLOCK_MONOTONIC_COARSE [PASS]
+ */
+static inline int vdso_test_clock(unsigned int clock_id)
+{
+	struct timespec x, y;
+
+	printf("clock_id: %s", vdso_clock_name[clock_id]);
+	clock_getres(clock_id, &x);
+	syscall_clock_getres(clock_id, &y);
+
+	if ((x.tv_sec != y.tv_sec) || (x.tv_sec != y.tv_sec)) {
+		printf(" [FAIL]\n");
+		return KSFT_FAIL;
+	}
+
+	printf(" [PASS]\n");
+	return 0;
+}
+
+int main(int argc, char **argv)
+{
+	int ret;
+
+#if _POSIX_TIMERS > 0
+
+#ifdef CLOCK_REALTIME
+	ret = vdso_test_clock(CLOCK_REALTIME);
+	if (ret)
+		goto out;
+#endif
+
+#ifdef CLOCK_BOOTTIME
+	ret = vdso_test_clock(CLOCK_BOOTTIME);
+	if (ret)
+		goto out;
+#endif
+
+#ifdef CLOCK_TAI
+	ret = vdso_test_clock(CLOCK_TAI);
+	if (ret)
+		goto out;
+#endif
+
+#ifdef CLOCK_REALTIME_COARSE
+	ret = vdso_test_clock(CLOCK_REALTIME_COARSE);
+	if (ret)
+		goto out;
+#endif
+
+#ifdef CLOCK_MONOTONIC
+	ret = vdso_test_clock(CLOCK_MONOTONIC);
+	if (ret)
+		goto out;
+#endif
+
+#ifdef CLOCK_MONOTONIC_RAW
+	ret = vdso_test_clock(CLOCK_MONOTONIC_RAW);
+	if (ret)
+		goto out;
+#endif
+
+#ifdef CLOCK_MONOTONIC_COARSE
+	ret = vdso_test_clock(CLOCK_MONOTONIC_COARSE);
+	if (ret)
+		goto out;
+#endif
+
+#endif
+
+out:
+	return ret;
+}
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 3/3] kselftest: Extend vDSO selftest to clock_getres
  2019-05-22 11:07 ` [PATCH v3 3/3] kselftest: Extend vDSO selftest to clock_getres Vincenzo Frascino
@ 2019-05-22 11:50   ` Christophe Leroy
  2019-05-22 14:55     ` Vincenzo Frascino
  2019-05-22 15:03     ` Vincenzo Frascino
  0 siblings, 2 replies; 8+ messages in thread
From: Christophe Leroy @ 2019-05-22 11:50 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, Shuah Khan



Le 22/05/2019 à 13:07, Vincenzo Frascino a écrit :
> The current version of the multiarch vDSO selftest verifies only
> gettimeofday.
> 
> Extend the vDSO selftest to clock_getres, to verify that the
> syscall and the vDSO library function return the same information.
> 
> The extension has been used to verify the hrtimer_resoltion fix.
> 
> Cc: Shuah Khan <shuah@kernel.org>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> ---
> 
> Note: This patch is independent from the others in this series, hence it
> can be merged singularly by the kselftest maintainers.
> 
>   tools/testing/selftests/vDSO/Makefile         |   2 +
>   .../selftests/vDSO/vdso_clock_getres.c        | 137 ++++++++++++++++++
>   2 files changed, 139 insertions(+)
>   create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
> 
> diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile
> index 9e03d61f52fd..d5c5bfdf1ac1 100644
> --- a/tools/testing/selftests/vDSO/Makefile
> +++ b/tools/testing/selftests/vDSO/Makefile
> @@ -5,6 +5,7 @@ uname_M := $(shell uname -m 2>/dev/null || echo not)
>   ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/)
>   
>   TEST_GEN_PROGS := $(OUTPUT)/vdso_test
> +TEST_GEN_PROGS += $(OUTPUT)/vdso_clock_getres
>   ifeq ($(ARCH),x86)
>   TEST_GEN_PROGS += $(OUTPUT)/vdso_standalone_test_x86
>   endif
> @@ -18,6 +19,7 @@ endif
>   
>   all: $(TEST_GEN_PROGS)
>   $(OUTPUT)/vdso_test: parse_vdso.c vdso_test.c
> +$(OUTPUT)/vdso_clock_getres: vdso_clock_getres.c
>   $(OUTPUT)/vdso_standalone_test_x86: vdso_standalone_test_x86.c parse_vdso.c
>   	$(CC) $(CFLAGS) $(CFLAGS_vdso_standalone_test_x86) \
>   		vdso_standalone_test_x86.c parse_vdso.c \
> diff --git a/tools/testing/selftests/vDSO/vdso_clock_getres.c b/tools/testing/selftests/vDSO/vdso_clock_getres.c
> new file mode 100644
> index 000000000000..341a9bc34ffc
> --- /dev/null
> +++ b/tools/testing/selftests/vDSO/vdso_clock_getres.c
> @@ -0,0 +1,137 @@
> +// SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note
> +/*
> + * vdso_clock_getres.c: Sample code to test clock_getres.
> + * Copyright (c) 2019 Arm Ltd.
> + *
> + * Compile with:
> + * gcc -std=gnu99 vdso_clock_getres.c
> + *
> + * Tested on ARM, ARM64, MIPS32, x86 (32-bit and 64-bit),
> + * Power (32-bit and 64-bit), S390x (32-bit and 64-bit).
> + * Might work on other architectures.
> + */
> +
> +#define _GNU_SOURCE
> +#include <elf.h>
> +#include <err.h>
> +#include <fcntl.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <time.h>
> +#include <sys/auxv.h>
> +#include <sys/mman.h>
> +#include <sys/time.h>
> +#include <unistd.h>
> +#include <sys/syscall.h>
> +
> +#include "../kselftest.h"
> +
> +static long syscall_clock_getres(clockid_t _clkid, struct timespec *_ts)
> +{
> +	long ret;
> +
> +	ret = syscall(SYS_clock_getres, _clkid, _ts);
> +
> +	return ret;
> +}
> +
> +const char *vdso_clock_name[12] = {
> +	"CLOCK_REALTIME",
> +	"CLOCK_MONOTONIC",
> +	"CLOCK_PROCESS_CPUTIME_ID",
> +	"CLOCK_THREAD_CPUTIME_ID",
> +	"CLOCK_MONOTONIC_RAW",
> +	"CLOCK_REALTIME_COARSE",
> +	"CLOCK_MONOTONIC_COARSE",
> +	"CLOCK_BOOTTIME",
> +	"CLOCK_REALTIME_ALARM",
> +	"CLOCK_BOOTTIME_ALARM",
> +	"CLOCK_SGI_CYCLE",
> +	"CLOCK_TAI",
> +};
> +
> +/*
> + * This function calls clock_getres in vdso and by system call
> + * with different values for clock_id.
> + *
> + * Example of output:
> + *
> + * clock_id: CLOCK_REALTIME [PASS]
> + * clock_id: CLOCK_BOOTTIME [PASS]
> + * clock_id: CLOCK_TAI [PASS]
> + * clock_id: CLOCK_REALTIME_COARSE [PASS]
> + * clock_id: CLOCK_MONOTONIC [PASS]
> + * clock_id: CLOCK_MONOTONIC_RAW [PASS]
> + * clock_id: CLOCK_MONOTONIC_COARSE [PASS]
> + */
> +static inline int vdso_test_clock(unsigned int clock_id)
> +{
> +	struct timespec x, y;
> +
> +	printf("clock_id: %s", vdso_clock_name[clock_id]);
> +	clock_getres(clock_id, &x);
> +	syscall_clock_getres(clock_id, &y);
> +
> +	if ((x.tv_sec != y.tv_sec) || (x.tv_sec != y.tv_sec)) {
> +		printf(" [FAIL]\n");
> +		return KSFT_FAIL;
> +	}
> +
> +	printf(" [PASS]\n");
> +	return 0;
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	int ret;
> +
> +#if _POSIX_TIMERS > 0
> +
> +#ifdef CLOCK_REALTIME

Why do you need that #ifdef and all the ones below ?

CLOCK_REALTIME (and others) is defined in include/uapi/linux/time.h, so 
it should be there when you build the test, shouldn't it ?

> +	ret = vdso_test_clock(CLOCK_REALTIME);
> +	if (ret)
> +		goto out;

Why that goto ? Nothing is done at out, so a 'return ret' would be 
better I think.

And do we really want to stop at first failure ? Wouldn't it be better 
to run all the tests regardless ?

Christophe

> +#endif
> +
> +#ifdef CLOCK_BOOTTIME
> +	ret = vdso_test_clock(CLOCK_BOOTTIME);
> +	if (ret)
> +		goto out;
> +#endif
> +
> +#ifdef CLOCK_TAI
> +	ret = vdso_test_clock(CLOCK_TAI);
> +	if (ret)
> +		goto out;
> +#endif
> +
> +#ifdef CLOCK_REALTIME_COARSE
> +	ret = vdso_test_clock(CLOCK_REALTIME_COARSE);
> +	if (ret)
> +		goto out;
> +#endif
> +
> +#ifdef CLOCK_MONOTONIC
> +	ret = vdso_test_clock(CLOCK_MONOTONIC);
> +	if (ret)
> +		goto out;
> +#endif
> +
> +#ifdef CLOCK_MONOTONIC_RAW
> +	ret = vdso_test_clock(CLOCK_MONOTONIC_RAW);
> +	if (ret)
> +		goto out;
> +#endif
> +
> +#ifdef CLOCK_MONOTONIC_COARSE
> +	ret = vdso_test_clock(CLOCK_MONOTONIC_COARSE);
> +	if (ret)
> +		goto out;
> +#endif
> +
> +#endif
> +
> +out:
> +	return ret;
> +}
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 3/3] kselftest: Extend vDSO selftest to clock_getres
  2019-05-22 11:50   ` Christophe Leroy
@ 2019-05-22 14:55     ` Vincenzo Frascino
  2019-05-22 15:03     ` Vincenzo Frascino
  1 sibling, 0 replies; 8+ messages in thread
From: Vincenzo Frascino @ 2019-05-22 14:55 UTC (permalink / raw)
  To: Christophe Leroy, linux-arch, linuxppc-dev, linux-s390, linux-kselftest
  Cc: Arnd Bergmann, Heiko Carstens, Paul Mackerras,
	Martin Schwidefsky, Thomas Gleixner, Shuah Khan

Hi Christophe,

thank you for your review.

On 22/05/2019 12:50, Christophe Leroy wrote:
> 
> 
> Le 22/05/2019 à 13:07, Vincenzo Frascino a écrit :
>> The current version of the multiarch vDSO selftest verifies only
>> gettimeofday.
>>
>> Extend the vDSO selftest to clock_getres, to verify that the
>> syscall and the vDSO library function return the same information.
>>
>> The extension has been used to verify the hrtimer_resoltion fix.
>>
>> Cc: Shuah Khan <shuah@kernel.org>
>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>> ---
>>
>> Note: This patch is independent from the others in this series, hence it
>> can be merged singularly by the kselftest maintainers.
>>
>>   tools/testing/selftests/vDSO/Makefile         |   2 +
>>   .../selftests/vDSO/vdso_clock_getres.c        | 137 ++++++++++++++++++
>>   2 files changed, 139 insertions(+)
>>   create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
>>
>> diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile
>> index 9e03d61f52fd..d5c5bfdf1ac1 100644
>> --- a/tools/testing/selftests/vDSO/Makefile
>> +++ b/tools/testing/selftests/vDSO/Makefile
>> @@ -5,6 +5,7 @@ uname_M := $(shell uname -m 2>/dev/null || echo not)
>>   ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/)
>>   
>>   TEST_GEN_PROGS := $(OUTPUT)/vdso_test
>> +TEST_GEN_PROGS += $(OUTPUT)/vdso_clock_getres
>>   ifeq ($(ARCH),x86)
>>   TEST_GEN_PROGS += $(OUTPUT)/vdso_standalone_test_x86
>>   endif
>> @@ -18,6 +19,7 @@ endif
>>   
>>   all: $(TEST_GEN_PROGS)
>>   $(OUTPUT)/vdso_test: parse_vdso.c vdso_test.c
>> +$(OUTPUT)/vdso_clock_getres: vdso_clock_getres.c
>>   $(OUTPUT)/vdso_standalone_test_x86: vdso_standalone_test_x86.c parse_vdso.c
>>   	$(CC) $(CFLAGS) $(CFLAGS_vdso_standalone_test_x86) \
>>   		vdso_standalone_test_x86.c parse_vdso.c \
>> diff --git a/tools/testing/selftests/vDSO/vdso_clock_getres.c b/tools/testing/selftests/vDSO/vdso_clock_getres.c
>> new file mode 100644
>> index 000000000000..341a9bc34ffc
>> --- /dev/null
>> +++ b/tools/testing/selftests/vDSO/vdso_clock_getres.c
>> @@ -0,0 +1,137 @@
>> +// SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note
>> +/*
>> + * vdso_clock_getres.c: Sample code to test clock_getres.
>> + * Copyright (c) 2019 Arm Ltd.
>> + *
>> + * Compile with:
>> + * gcc -std=gnu99 vdso_clock_getres.c
>> + *
>> + * Tested on ARM, ARM64, MIPS32, x86 (32-bit and 64-bit),
>> + * Power (32-bit and 64-bit), S390x (32-bit and 64-bit).
>> + * Might work on other architectures.
>> + */
>> +
>> +#define _GNU_SOURCE
>> +#include <elf.h>
>> +#include <err.h>
>> +#include <fcntl.h>
>> +#include <stdint.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <time.h>
>> +#include <sys/auxv.h>
>> +#include <sys/mman.h>
>> +#include <sys/time.h>
>> +#include <unistd.h>
>> +#include <sys/syscall.h>
>> +
>> +#include "../kselftest.h"
>> +
>> +static long syscall_clock_getres(clockid_t _clkid, struct timespec *_ts)
>> +{
>> +	long ret;
>> +
>> +	ret = syscall(SYS_clock_getres, _clkid, _ts);
>> +
>> +	return ret;
>> +}
>> +
>> +const char *vdso_clock_name[12] = {
>> +	"CLOCK_REALTIME",
>> +	"CLOCK_MONOTONIC",
>> +	"CLOCK_PROCESS_CPUTIME_ID",
>> +	"CLOCK_THREAD_CPUTIME_ID",
>> +	"CLOCK_MONOTONIC_RAW",
>> +	"CLOCK_REALTIME_COARSE",
>> +	"CLOCK_MONOTONIC_COARSE",
>> +	"CLOCK_BOOTTIME",
>> +	"CLOCK_REALTIME_ALARM",
>> +	"CLOCK_BOOTTIME_ALARM",
>> +	"CLOCK_SGI_CYCLE",
>> +	"CLOCK_TAI",
>> +};
>> +
>> +/*
>> + * This function calls clock_getres in vdso and by system call
>> + * with different values for clock_id.
>> + *
>> + * Example of output:
>> + *
>> + * clock_id: CLOCK_REALTIME [PASS]
>> + * clock_id: CLOCK_BOOTTIME [PASS]
>> + * clock_id: CLOCK_TAI [PASS]
>> + * clock_id: CLOCK_REALTIME_COARSE [PASS]
>> + * clock_id: CLOCK_MONOTONIC [PASS]
>> + * clock_id: CLOCK_MONOTONIC_RAW [PASS]
>> + * clock_id: CLOCK_MONOTONIC_COARSE [PASS]
>> + */
>> +static inline int vdso_test_clock(unsigned int clock_id)
>> +{
>> +	struct timespec x, y;
>> +
>> +	printf("clock_id: %s", vdso_clock_name[clock_id]);
>> +	clock_getres(clock_id, &x);
>> +	syscall_clock_getres(clock_id, &y);
>> +
>> +	if ((x.tv_sec != y.tv_sec) || (x.tv_sec != y.tv_sec)) {
>> +		printf(" [FAIL]\n");
>> +		return KSFT_FAIL;
>> +	}
>> +
>> +	printf(" [PASS]\n");
>> +	return 0;
>> +}
>> +
>> +int main(int argc, char **argv)
>> +{
>> +	int ret;
>> +
>> +#if _POSIX_TIMERS > 0
>> +
>> +#ifdef CLOCK_REALTIME
> 
> Why do you need that #ifdef and all the ones below ?
> 
> CLOCK_REALTIME (and others) is defined in include/uapi/linux/time.h, so 
> it should be there when you build the test, shouldn't it ?
> 

In implementing this test I followed what the man page for clock_gettime(2)
defines in terms of availability of the timers. Since I do not know how old are
the userspace headers, I think it is a good idea checking that the clocks are
defined before trying to use them.

>> +	ret = vdso_test_clock(CLOCK_REALTIME);
>> +	if (ret)
>> +		goto out;
> 
> Why that goto ? Nothing is done at out, so a 'return ret' would be 
> better I think.
>

Agree, thanks for pointing this out. Will fix in v4.


> And do we really want to stop at first failure ? Wouldn't it be better 
> to run all the tests regardless ?
> 

The test is supposed to fail if one of the sub-tests fails, hence once the first
fails doesn't seem convenient running the others, because we already know the
result.

> Christophe
> 
>> +#endif
>> +
>> +#ifdef CLOCK_BOOTTIME
>> +	ret = vdso_test_clock(CLOCK_BOOTTIME);
>> +	if (ret)
>> +		goto out;
>> +#endif
>> +
>> +#ifdef CLOCK_TAI
>> +	ret = vdso_test_clock(CLOCK_TAI);
>> +	if (ret)
>> +		goto out;
>> +#endif
>> +
>> +#ifdef CLOCK_REALTIME_COARSE
>> +	ret = vdso_test_clock(CLOCK_REALTIME_COARSE);
>> +	if (ret)
>> +		goto out;
>> +#endif
>> +
>> +#ifdef CLOCK_MONOTONIC
>> +	ret = vdso_test_clock(CLOCK_MONOTONIC);
>> +	if (ret)
>> +		goto out;
>> +#endif
>> +
>> +#ifdef CLOCK_MONOTONIC_RAW
>> +	ret = vdso_test_clock(CLOCK_MONOTONIC_RAW);
>> +	if (ret)
>> +		goto out;
>> +#endif
>> +
>> +#ifdef CLOCK_MONOTONIC_COARSE
>> +	ret = vdso_test_clock(CLOCK_MONOTONIC_COARSE);
>> +	if (ret)
>> +		goto out;
>> +#endif
>> +
>> +#endif
>> +
>> +out:
>> +	return ret;
>> +}
>>

-- 
Regards,
Vincenzo

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 3/3] kselftest: Extend vDSO selftest to clock_getres
  2019-05-22 11:50   ` Christophe Leroy
  2019-05-22 14:55     ` Vincenzo Frascino
@ 2019-05-22 15:03     ` Vincenzo Frascino
  1 sibling, 0 replies; 8+ messages in thread
From: Vincenzo Frascino @ 2019-05-22 15:03 UTC (permalink / raw)
  To: Christophe Leroy, linux-arch, linuxppc-dev, linux-s390, linux-kselftest
  Cc: Arnd Bergmann, Heiko Carstens, Paul Mackerras,
	Martin Schwidefsky, Thomas Gleixner, Shuah Khan

Hi Christophe,

thank you for your review.

On 22/05/2019 12:50, Christophe Leroy wrote:
> 
> 
> Le 22/05/2019 à 13:07, Vincenzo Frascino a écrit :
>> The current version of the multiarch vDSO selftest verifies only
>> gettimeofday.
>>
>> Extend the vDSO selftest to clock_getres, to verify that the
>> syscall and the vDSO library function return the same information.
>>
>> The extension has been used to verify the hrtimer_resoltion fix.
>>
>> Cc: Shuah Khan <shuah@kernel.org>
>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>> ---
>>
>> Note: This patch is independent from the others in this series, hence it
>> can be merged singularly by the kselftest maintainers.
>>
>>   tools/testing/selftests/vDSO/Makefile         |   2 +
>>   .../selftests/vDSO/vdso_clock_getres.c        | 137 ++++++++++++++++++
>>   2 files changed, 139 insertions(+)
>>   create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
>>
>> diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile
>> index 9e03d61f52fd..d5c5bfdf1ac1 100644
>> --- a/tools/testing/selftests/vDSO/Makefile
>> +++ b/tools/testing/selftests/vDSO/Makefile
>> @@ -5,6 +5,7 @@ uname_M := $(shell uname -m 2>/dev/null || echo not)
>>   ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/)
>>   
>>   TEST_GEN_PROGS := $(OUTPUT)/vdso_test
>> +TEST_GEN_PROGS += $(OUTPUT)/vdso_clock_getres
>>   ifeq ($(ARCH),x86)
>>   TEST_GEN_PROGS += $(OUTPUT)/vdso_standalone_test_x86
>>   endif
>> @@ -18,6 +19,7 @@ endif
>>   
>>   all: $(TEST_GEN_PROGS)
>>   $(OUTPUT)/vdso_test: parse_vdso.c vdso_test.c
>> +$(OUTPUT)/vdso_clock_getres: vdso_clock_getres.c
>>   $(OUTPUT)/vdso_standalone_test_x86: vdso_standalone_test_x86.c parse_vdso.c
>>   	$(CC) $(CFLAGS) $(CFLAGS_vdso_standalone_test_x86) \
>>   		vdso_standalone_test_x86.c parse_vdso.c \
>> diff --git a/tools/testing/selftests/vDSO/vdso_clock_getres.c b/tools/testing/selftests/vDSO/vdso_clock_getres.c
>> new file mode 100644
>> index 000000000000..341a9bc34ffc
>> --- /dev/null
>> +++ b/tools/testing/selftests/vDSO/vdso_clock_getres.c
>> @@ -0,0 +1,137 @@
>> +// SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note
>> +/*
>> + * vdso_clock_getres.c: Sample code to test clock_getres.
>> + * Copyright (c) 2019 Arm Ltd.
>> + *
>> + * Compile with:
>> + * gcc -std=gnu99 vdso_clock_getres.c
>> + *
>> + * Tested on ARM, ARM64, MIPS32, x86 (32-bit and 64-bit),
>> + * Power (32-bit and 64-bit), S390x (32-bit and 64-bit).
>> + * Might work on other architectures.
>> + */
>> +
>> +#define _GNU_SOURCE
>> +#include <elf.h>
>> +#include <err.h>
>> +#include <fcntl.h>
>> +#include <stdint.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <time.h>
>> +#include <sys/auxv.h>
>> +#include <sys/mman.h>
>> +#include <sys/time.h>
>> +#include <unistd.h>
>> +#include <sys/syscall.h>
>> +
>> +#include "../kselftest.h"
>> +
>> +static long syscall_clock_getres(clockid_t _clkid, struct timespec *_ts)
>> +{
>> +	long ret;
>> +
>> +	ret = syscall(SYS_clock_getres, _clkid, _ts);
>> +
>> +	return ret;
>> +}
>> +
>> +const char *vdso_clock_name[12] = {
>> +	"CLOCK_REALTIME",
>> +	"CLOCK_MONOTONIC",
>> +	"CLOCK_PROCESS_CPUTIME_ID",
>> +	"CLOCK_THREAD_CPUTIME_ID",
>> +	"CLOCK_MONOTONIC_RAW",
>> +	"CLOCK_REALTIME_COARSE",
>> +	"CLOCK_MONOTONIC_COARSE",
>> +	"CLOCK_BOOTTIME",
>> +	"CLOCK_REALTIME_ALARM",
>> +	"CLOCK_BOOTTIME_ALARM",
>> +	"CLOCK_SGI_CYCLE",
>> +	"CLOCK_TAI",
>> +};
>> +
>> +/*
>> + * This function calls clock_getres in vdso and by system call
>> + * with different values for clock_id.
>> + *
>> + * Example of output:
>> + *
>> + * clock_id: CLOCK_REALTIME [PASS]
>> + * clock_id: CLOCK_BOOTTIME [PASS]
>> + * clock_id: CLOCK_TAI [PASS]
>> + * clock_id: CLOCK_REALTIME_COARSE [PASS]
>> + * clock_id: CLOCK_MONOTONIC [PASS]
>> + * clock_id: CLOCK_MONOTONIC_RAW [PASS]
>> + * clock_id: CLOCK_MONOTONIC_COARSE [PASS]
>> + */
>> +static inline int vdso_test_clock(unsigned int clock_id)
>> +{
>> +	struct timespec x, y;
>> +
>> +	printf("clock_id: %s", vdso_clock_name[clock_id]);
>> +	clock_getres(clock_id, &x);
>> +	syscall_clock_getres(clock_id, &y);
>> +
>> +	if ((x.tv_sec != y.tv_sec) || (x.tv_sec != y.tv_sec)) {
>> +		printf(" [FAIL]\n");
>> +		return KSFT_FAIL;
>> +	}
>> +
>> +	printf(" [PASS]\n");
>> +	return 0;
>> +}
>> +
>> +int main(int argc, char **argv)
>> +{
>> +	int ret;
>> +
>> +#if _POSIX_TIMERS > 0
>> +
>> +#ifdef CLOCK_REALTIME
> 
> Why do you need that #ifdef and all the ones below ?
> 
> CLOCK_REALTIME (and others) is defined in include/uapi/linux/time.h, so 
> it should be there when you build the test, shouldn't it ?
> 

In implementing this test I tried to follow what the man page for
clock_gettime(2) defines in terms of availability of the timers. Since I do not
know how old are the userspace headers, I think it is a good idea checking that
the clocks are defined before trying to use them.

>> +	ret = vdso_test_clock(CLOCK_REALTIME);
>> +	if (ret)
>> +		goto out;
> 
> Why that goto ? Nothing is done at out, so a 'return ret' would be 
> better I think.
> 

Agree, thanks for pointing this out. Will fix in v4.

> And do we really want to stop at first failure ? Wouldn't it be better 
> to run all the tests regardless ?
> 

The test is supposed to fail if one of the sub-tests fails, hence once the first
fails doesn't seem convenient to run the others, because we already know the
result.

> Christophe
> 
>> +#endif
>> +
>> +#ifdef CLOCK_BOOTTIME
>> +	ret = vdso_test_clock(CLOCK_BOOTTIME);
>> +	if (ret)
>> +		goto out;
>> +#endif
>> +
>> +#ifdef CLOCK_TAI
>> +	ret = vdso_test_clock(CLOCK_TAI);
>> +	if (ret)
>> +		goto out;
>> +#endif
>> +
>> +#ifdef CLOCK_REALTIME_COARSE
>> +	ret = vdso_test_clock(CLOCK_REALTIME_COARSE);
>> +	if (ret)
>> +		goto out;
>> +#endif
>> +
>> +#ifdef CLOCK_MONOTONIC
>> +	ret = vdso_test_clock(CLOCK_MONOTONIC);
>> +	if (ret)
>> +		goto out;
>> +#endif
>> +
>> +#ifdef CLOCK_MONOTONIC_RAW
>> +	ret = vdso_test_clock(CLOCK_MONOTONIC_RAW);
>> +	if (ret)
>> +		goto out;
>> +#endif
>> +
>> +#ifdef CLOCK_MONOTONIC_COARSE
>> +	ret = vdso_test_clock(CLOCK_MONOTONIC_COARSE);
>> +	if (ret)
>> +		goto out;
>> +#endif
>> +
>> +#endif
>> +
>> +out:
>> +	return ret;
>> +}
>>

-- 
Regards,
Vincenzo

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/3] powerpc: Fix vDSO clock_getres()
  2019-05-22 11:07 ` [PATCH v3 1/3] powerpc: " Vincenzo Frascino
@ 2019-05-29 13:14   ` Sasha Levin
  0 siblings, 0 replies; 8+ 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

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] 8+ messages in thread

end of thread, other threads:[~2019-05-29 13:20 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-22 11:07 [PATCH v3 0/3] Fix vDSO clock_getres() Vincenzo Frascino
2019-05-22 11:07 ` [PATCH v3 1/3] powerpc: " Vincenzo Frascino
2019-05-29 13:14   ` Sasha Levin
2019-05-22 11:07 ` [PATCH v3 2/3] s390: " Vincenzo Frascino
2019-05-22 11:07 ` [PATCH v3 3/3] kselftest: Extend vDSO selftest to clock_getres Vincenzo Frascino
2019-05-22 11:50   ` Christophe Leroy
2019-05-22 14:55     ` Vincenzo Frascino
2019-05-22 15:03     ` Vincenzo Frascino

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).