Linux Kernel Selftest development
 help / color / mirror / Atom feed
* [PATCH v4 0/3] Fix vDSO clock_getres()
@ 2019-05-23 11:21 vincenzo.frascino
  2019-05-23 11:21 ` Vincenzo Frascino
                   ` (3 more replies)
  0 siblings, 4 replies; 29+ messages in thread
From: vincenzo.frascino @ 2019-05-23 11:21 UTC (permalink / raw)


clock_getres in the vDSO library has to preserve the same behaviour
of posix_get_hrtimer_res().

In particular, posix_get_hrtimer_res() does:
    sec = 0;
    ns = hrtimer_resolution;
and hrtimer_resolution depends on the enablement of the high
resolution timers that can happen either at compile or at run time.

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

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

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

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

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

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

Cc: Christophe Leroy <christophe.leroy at c-s.fr>
Cc: Benjamin Herrenschmidt <benh at kernel.crashing.org>
Cc: Paul Mackerras <paulus at samba.org>
Cc: Michael Ellerman <mpe at ellerman.id.au>
Cc: Martin Schwidefsky <schwidefsky at de.ibm.com>
Cc: Heiko Carstens <heiko.carstens at de.ibm.com>
Cc: Shuah Khan <shuah at kernel.org>
Cc: Thomas Gleixner <tglx at linutronix.de>
Cc: Arnd Bergmann <arnd at arndb.de>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino at 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        | 124 ++++++++++++++++++
 12 files changed, 155 insertions(+), 16 deletions(-)
 create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c

-- 
2.21.0

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

* [PATCH v4 0/3] 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-23 11:21 ` [PATCH v4 1/3] powerpc: " vincenzo.frascino
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 29+ messages in thread
From: Vincenzo Frascino @ 2019-05-23 11:21 UTC (permalink / raw)


clock_getres in the vDSO library has to preserve the same behaviour
of posix_get_hrtimer_res().

In particular, posix_get_hrtimer_res() does:
    sec = 0;
    ns = hrtimer_resolution;
and hrtimer_resolution depends on the enablement of the high
resolution timers that can happen either at compile or at run time.

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

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

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

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

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

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

Cc: Christophe Leroy <christophe.leroy at c-s.fr>
Cc: Benjamin Herrenschmidt <benh at kernel.crashing.org>
Cc: Paul Mackerras <paulus at samba.org>
Cc: Michael Ellerman <mpe at ellerman.id.au>
Cc: Martin Schwidefsky <schwidefsky at de.ibm.com>
Cc: Heiko Carstens <heiko.carstens at de.ibm.com>
Cc: Shuah Khan <shuah at kernel.org>
Cc: Thomas Gleixner <tglx at linutronix.de>
Cc: Arnd Bergmann <arnd at arndb.de>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino at 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        | 124 ++++++++++++++++++
 12 files changed, 155 insertions(+), 16 deletions(-)
 create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c

-- 
2.21.0

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

* [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-23 11:21 ` vincenzo.frascino
  2019-05-23 11:21   ` Vincenzo Frascino
  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
  3 siblings, 1 reply; 29+ messages in thread
From: vincenzo.frascino @ 2019-05-23 11:21 UTC (permalink / raw)


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 at vger.kernel.org
Cc: Benjamin Herrenschmidt <benh at kernel.crashing.org>
Cc: Paul Mackerras <paulus at samba.org>
Cc: Michael Ellerman <mpe at ellerman.id.au>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino at arm.com>
Reviewed-by: Christophe Leroy <christophe.leroy at 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 at 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 at h
-	ori	r5,r5,CLOCK_REALTIME_RES at 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 at h
-	ori	r5,r5,CLOCK_REALTIME_RES at l
 	std	r3,TSPC64_TV_SEC(r4)
 	std	r5,TSPC64_TV_NSEC(r4)
 	blr
-- 
2.21.0

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

* [PATCH v4 1/3] powerpc: Fix vDSO clock_getres()
  2019-05-23 11:21 ` [PATCH v4 1/3] powerpc: " vincenzo.frascino
@ 2019-05-23 11:21   ` Vincenzo Frascino
  0 siblings, 0 replies; 29+ messages in thread
From: Vincenzo Frascino @ 2019-05-23 11:21 UTC (permalink / raw)


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 at vger.kernel.org
Cc: Benjamin Herrenschmidt <benh at kernel.crashing.org>
Cc: Paul Mackerras <paulus at samba.org>
Cc: Michael Ellerman <mpe at ellerman.id.au>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino at arm.com>
Reviewed-by: Christophe Leroy <christophe.leroy at 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 at 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 at h
-	ori	r5,r5,CLOCK_REALTIME_RES at 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 at h
-	ori	r5,r5,CLOCK_REALTIME_RES at l
 	std	r3,TSPC64_TV_SEC(r4)
 	std	r5,TSPC64_TV_NSEC(r4)
 	blr
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 29+ 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 ` 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   ` Vincenzo Frascino
  2019-05-23 11:21 ` [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres vincenzo.frascino
  3 siblings, 1 reply; 29+ messages in thread
From: vincenzo.frascino @ 2019-05-23 11:21 UTC (permalink / raw)


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 at de.ibm.com>
Cc: Heiko Carstens <heiko.carstens at de.ibm.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino at arm.com>
Acked-by: Martin Schwidefsky <schwidefsky at 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, at 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] 29+ messages in thread

* [PATCH v4 2/3] s390: Fix vDSO clock_getres()
  2019-05-23 11:21 ` [PATCH v4 2/3] s390: " vincenzo.frascino
@ 2019-05-23 11:21   ` Vincenzo Frascino
  0 siblings, 0 replies; 29+ messages in thread
From: Vincenzo Frascino @ 2019-05-23 11:21 UTC (permalink / raw)


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 at de.ibm.com>
Cc: Heiko Carstens <heiko.carstens at de.ibm.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino at arm.com>
Acked-by: Martin Schwidefsky <schwidefsky at 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, at 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] 29+ 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
                   ` (2 preceding siblings ...)
  2019-05-23 11:21 ` [PATCH v4 2/3] s390: " vincenzo.frascino
@ 2019-05-23 11:21 ` vincenzo.frascino
  2019-05-23 11:21   ` Vincenzo Frascino
  2019-05-28  6:19   ` mpe
  3 siblings, 2 replies; 29+ messages in thread
From: vincenzo.frascino @ 2019-05-23 11:21 UTC (permalink / raw)


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 at kernel.org>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino at 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] 29+ messages in thread

* [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-23 11:21   ` Vincenzo Frascino
  2019-05-28  6:19   ` mpe
  1 sibling, 0 replies; 29+ messages in thread
From: Vincenzo Frascino @ 2019-05-23 11:21 UTC (permalink / raw)


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 at kernel.org>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino at 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] 29+ messages in thread

* [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-23 11:21   ` Vincenzo Frascino
@ 2019-05-28  6:19   ` mpe
  2019-05-28  6:19     ` Michael Ellerman
  2019-05-28 11:57     ` vincenzo.frascino
  1 sibling, 2 replies; 29+ messages in thread
From: mpe @ 2019-05-28  6:19 UTC (permalink / raw)


Vincenzo Frascino <vincenzo.frascino at 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 at kernel.org>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino at 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] 29+ messages in thread

* [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
  2019-05-28  6:19   ` mpe
@ 2019-05-28  6:19     ` Michael Ellerman
  2019-05-28 11:57     ` vincenzo.frascino
  1 sibling, 0 replies; 29+ messages in thread
From: Michael Ellerman @ 2019-05-28  6:19 UTC (permalink / raw)


Vincenzo Frascino <vincenzo.frascino at 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 at kernel.org>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino at 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] 29+ messages in thread

* [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
  2019-05-28  6:19   ` mpe
  2019-05-28  6:19     ` Michael Ellerman
@ 2019-05-28 11:57     ` vincenzo.frascino
  2019-05-28 11:57       ` Vincenzo Frascino
                         ` (3 more replies)
  1 sibling, 4 replies; 29+ messages in thread
From: vincenzo.frascino @ 2019-05-28 11:57 UTC (permalink / raw)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2387 bytes --]

Hi Michael,

thank you for your reply.

On 28/05/2019 07:19, Michael Ellerman wrote:
> Vincenzo Frascino <vincenzo.frascino at 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 at kernel.org>
>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino at 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] 29+ messages in thread

* [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
  2019-05-28 11:57     ` vincenzo.frascino
@ 2019-05-28 11:57       ` Vincenzo Frascino
  2019-05-28 17:01       ` christophe.leroy
                         ` (2 subsequent siblings)
  3 siblings, 0 replies; 29+ messages in thread
From: Vincenzo Frascino @ 2019-05-28 11:57 UTC (permalink / raw)


Hi Michael,

thank you for your reply.

On 28/05/2019 07:19, Michael Ellerman wrote:
> Vincenzo Frascino <vincenzo.frascino at 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 at kernel.org>
>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino at 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] 29+ messages in thread

* [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
  2019-05-28 11:57     ` vincenzo.frascino
  2019-05-28 11:57       ` Vincenzo Frascino
@ 2019-05-28 17:01       ` christophe.leroy
  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
  3 siblings, 2 replies; 29+ messages in thread
From: christophe.leroy @ 2019-05-28 17:01 UTC (permalink / raw)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2617 bytes --]

Vincenzo Frascino <vincenzo.frascino at arm.com> a écrit :

> Hi Michael,
>
> thank you for your reply.
>
> On 28/05/2019 07:19, Michael Ellerman wrote:
>> Vincenzo Frascino <vincenzo.frascino at 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 at kernel.org>
>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino at 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] 29+ messages in thread

* [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
  2019-05-28 17:01       ` christophe.leroy
@ 2019-05-28 17:01         ` Christophe Leroy
  2019-05-28 17:05         ` vincenzo.frascino
  1 sibling, 0 replies; 29+ messages in thread
From: Christophe Leroy @ 2019-05-28 17:01 UTC (permalink / raw)


Vincenzo Frascino <vincenzo.frascino at arm.com> a écrit :

> Hi Michael,
>
> thank you for your reply.
>
> On 28/05/2019 07:19, Michael Ellerman wrote:
>> Vincenzo Frascino <vincenzo.frascino at 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 at kernel.org>
>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino at 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] 29+ messages in thread

* [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
  2019-05-28 17:01       ` christophe.leroy
  2019-05-28 17:01         ` Christophe Leroy
@ 2019-05-28 17:05         ` vincenzo.frascino
  2019-05-28 17:05           ` Vincenzo Frascino
  1 sibling, 1 reply; 29+ messages in thread
From: vincenzo.frascino @ 2019-05-28 17:05 UTC (permalink / raw)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2860 bytes --]

Hi Christophe,

On 28/05/2019 18:01, Christophe Leroy wrote:
> Vincenzo Frascino <vincenzo.frascino at arm.com> a écrit :
> 
>> Hi Michael,
>>
>> thank you for your reply.
>>
>> On 28/05/2019 07:19, Michael Ellerman wrote:
>>> Vincenzo Frascino <vincenzo.frascino at 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 at kernel.org>
>>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino at 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] 29+ messages in thread

* [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
  2019-05-28 17:05         ` vincenzo.frascino
@ 2019-05-28 17:05           ` Vincenzo Frascino
  0 siblings, 0 replies; 29+ messages in thread
From: Vincenzo Frascino @ 2019-05-28 17:05 UTC (permalink / raw)


Hi Christophe,

On 28/05/2019 18:01, Christophe Leroy wrote:
> Vincenzo Frascino <vincenzo.frascino at arm.com> a écrit :
> 
>> Hi Michael,
>>
>> thank you for your reply.
>>
>> On 28/05/2019 07:19, Michael Ellerman wrote:
>>> Vincenzo Frascino <vincenzo.frascino at 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 at kernel.org>
>>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino at 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] 29+ messages in thread

* [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
  2019-05-28 11:57     ` vincenzo.frascino
  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:16         ` Christophe Leroy
  2019-06-04 13:32         ` vincenzo.frascino
  2019-06-13 15:22       ` Vincenzo Frascino
  3 siblings, 2 replies; 29+ messages in thread
From: christophe.leroy @ 2019-06-04 13:16 UTC (permalink / raw)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2701 bytes --]

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 at 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 at kernel.org>
>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino at 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] 29+ messages in thread

* [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
  2019-06-04 13:16       ` christophe.leroy
@ 2019-06-04 13:16         ` Christophe Leroy
  2019-06-04 13:32         ` vincenzo.frascino
  1 sibling, 0 replies; 29+ messages in thread
From: Christophe Leroy @ 2019-06-04 13:16 UTC (permalink / raw)


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 at 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 at kernel.org>
>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino at 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] 29+ messages in thread

* [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
  2019-06-04 13:16       ` christophe.leroy
  2019-06-04 13:16         ` Christophe Leroy
@ 2019-06-04 13:32         ` vincenzo.frascino
  2019-06-04 13:32           ` Vincenzo Frascino
  2019-06-04 13:39           ` christophe.leroy
  1 sibling, 2 replies; 29+ messages in thread
From: vincenzo.frascino @ 2019-06-04 13:32 UTC (permalink / raw)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 3740 bytes --]

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 at 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 at h
	ori	r5,r5,CLOCK_REALTIME_RES at 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 at kernel.org>
>>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino at 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] 29+ messages in thread

* [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
  2019-06-04 13:32         ` vincenzo.frascino
@ 2019-06-04 13:32           ` Vincenzo Frascino
  2019-06-04 13:39           ` christophe.leroy
  1 sibling, 0 replies; 29+ messages in thread
From: Vincenzo Frascino @ 2019-06-04 13:32 UTC (permalink / raw)


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 at 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 at h
	ori	r5,r5,CLOCK_REALTIME_RES at 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 at kernel.org>
>>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino at 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] 29+ messages in thread

* [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
  2019-06-04 13:32         ` vincenzo.frascino
  2019-06-04 13:32           ` Vincenzo Frascino
@ 2019-06-04 13:39           ` christophe.leroy
  2019-06-04 13:39             ` Christophe Leroy
  2019-06-04 13:43             ` vincenzo.frascino
  1 sibling, 2 replies; 29+ messages in thread
From: christophe.leroy @ 2019-06-04 13:39 UTC (permalink / raw)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 4169 bytes --]



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 at 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 at h
> 	ori	r5,r5,CLOCK_REALTIME_RES at 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 at kernel.org>
>>>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino at 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] 29+ messages in thread

* [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
  2019-06-04 13:39           ` christophe.leroy
@ 2019-06-04 13:39             ` Christophe Leroy
  2019-06-04 13:43             ` vincenzo.frascino
  1 sibling, 0 replies; 29+ messages in thread
From: Christophe Leroy @ 2019-06-04 13:39 UTC (permalink / raw)




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 at 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 at h
> 	ori	r5,r5,CLOCK_REALTIME_RES at 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 at kernel.org>
>>>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino at 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] 29+ messages in thread

* [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
  2019-06-04 13:39           ` christophe.leroy
  2019-06-04 13:39             ` Christophe Leroy
@ 2019-06-04 13:43             ` vincenzo.frascino
  2019-06-04 13:43               ` Vincenzo Frascino
  2019-06-04 13:52               ` christophe.leroy
  1 sibling, 2 replies; 29+ messages in thread
From: vincenzo.frascino @ 2019-06-04 13:43 UTC (permalink / raw)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 4606 bytes --]

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 at 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 at h
>> 	ori	r5,r5,CLOCK_REALTIME_RES at 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 at kernel.org>
>>>>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino at 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] 29+ messages in thread

* [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
  2019-06-04 13:43             ` vincenzo.frascino
@ 2019-06-04 13:43               ` Vincenzo Frascino
  2019-06-04 13:52               ` christophe.leroy
  1 sibling, 0 replies; 29+ messages in thread
From: Vincenzo Frascino @ 2019-06-04 13:43 UTC (permalink / raw)


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 at 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 at h
>> 	ori	r5,r5,CLOCK_REALTIME_RES at 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 at kernel.org>
>>>>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino at 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] 29+ messages in thread

* [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
  2019-06-04 13:43             ` vincenzo.frascino
  2019-06-04 13:43               ` Vincenzo Frascino
@ 2019-06-04 13:52               ` christophe.leroy
  2019-06-04 13:52                 ` Christophe Leroy
  2019-06-04 14:21                 ` vincenzo.frascino
  1 sibling, 2 replies; 29+ messages in thread
From: christophe.leroy @ 2019-06-04 13:52 UTC (permalink / raw)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 5112 bytes --]



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 at 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 at h
>>> 	ori	r5,r5,CLOCK_REALTIME_RES at 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 at kernel.org>
>>>>>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino at 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] 29+ messages in thread

* [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
  2019-06-04 13:52               ` christophe.leroy
@ 2019-06-04 13:52                 ` Christophe Leroy
  2019-06-04 14:21                 ` vincenzo.frascino
  1 sibling, 0 replies; 29+ messages in thread
From: Christophe Leroy @ 2019-06-04 13:52 UTC (permalink / raw)




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 at 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 at h
>>> 	ori	r5,r5,CLOCK_REALTIME_RES at 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 at kernel.org>
>>>>>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino at 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] 29+ messages in thread

* [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
  2019-06-04 13:52               ` christophe.leroy
  2019-06-04 13:52                 ` Christophe Leroy
@ 2019-06-04 14:21                 ` vincenzo.frascino
  2019-06-04 14:21                   ` Vincenzo Frascino
  1 sibling, 1 reply; 29+ messages in thread
From: vincenzo.frascino @ 2019-06-04 14:21 UTC (permalink / raw)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 5402 bytes --]


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 at 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 at h
>>>> 	ori	r5,r5,CLOCK_REALTIME_RES at 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 at kernel.org>
>>>>>>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino at 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] 29+ messages in thread

* [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
  2019-06-04 14:21                 ` vincenzo.frascino
@ 2019-06-04 14:21                   ` Vincenzo Frascino
  0 siblings, 0 replies; 29+ messages in thread
From: Vincenzo Frascino @ 2019-06-04 14:21 UTC (permalink / raw)



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 at 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 at h
>>>> 	ori	r5,r5,CLOCK_REALTIME_RES at 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 at kernel.org>
>>>>>>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino at 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] 29+ messages in thread

* Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
  2019-05-28 11:57     ` vincenzo.frascino
                         ` (2 preceding siblings ...)
  2019-06-04 13:16       ` christophe.leroy
@ 2019-06-13 15:22       ` Vincenzo Frascino
  3 siblings, 0 replies; 29+ 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: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
	Martin Schwidefsky, Heiko Carstens, Shuah Khan, Thomas Gleixner,
	Arnd Bergmann

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

end of thread, other threads:[~2019-06-13 15:22 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-23 11:21 [PATCH v4 0/3] Fix vDSO clock_getres() vincenzo.frascino
2019-05-23 11:21 ` 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 2/3] s390: " 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
2019-05-23 11:21   ` Vincenzo Frascino
2019-05-28  6:19   ` mpe
2019-05-28  6:19     ` Michael Ellerman
2019-05-28 11:57     ` vincenzo.frascino
2019-05-28 11:57       ` Vincenzo Frascino
2019-05-28 17:01       ` christophe.leroy
2019-05-28 17:01         ` Christophe Leroy
2019-05-28 17:05         ` vincenzo.frascino
2019-05-28 17:05           ` Vincenzo Frascino
2019-06-04 13:16       ` christophe.leroy
2019-06-04 13:16         ` Christophe Leroy
2019-06-04 13:32         ` vincenzo.frascino
2019-06-04 13:32           ` Vincenzo Frascino
2019-06-04 13:39           ` christophe.leroy
2019-06-04 13:39             ` Christophe Leroy
2019-06-04 13:43             ` vincenzo.frascino
2019-06-04 13:43               ` Vincenzo Frascino
2019-06-04 13:52               ` christophe.leroy
2019-06-04 13:52                 ` Christophe Leroy
2019-06-04 14:21                 ` vincenzo.frascino
2019-06-04 14:21                   ` Vincenzo Frascino
2019-06-13 15:22       ` Vincenzo Frascino

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox