Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres

From
Vincenzo Frascino <vincenzo.frascino@arm.com>
Date
2019-06-04 14:27:12
Also in
linux-kselftest, linux-s390, linuxppc-dev
Reply to
Christophe Leroy, 2019-06-04
On 04/06/2019 14:52, Christophe Leroy wrote:

Le 04/06/2019 à 15:43, Vincenzo Frascino a écrit :
quoted
On 04/06/2019 14:39, Christophe Leroy wrote:
quoted

Le 04/06/2019 à 15:32, Vincenzo Frascino a écrit :
quoted
Hi Christophe,

On 04/06/2019 14:16, Christophe Leroy wrote:
quoted
Hi Vincenzo

Le 28/05/2019 à 13:57, Vincenzo Frascino a écrit :
quoted
Hi Michael,

thank you for your reply.

On 28/05/2019 07:19, Michael Ellerman wrote:
quoted
Vincenzo Frascino [off-list ref] writes:
quoted
The current version of the multiarch vDSO selftest verifies only
gettimeofday.

Extend the vDSO selftest to clock_getres, to verify that the
syscall and the vDSO library function return the same information.

The extension has been used to verify the hrtimer_resoltion fix.
This is passing for me even without patch 1 applied, shouldn't it fail
without the fix? What am I missing?
This is correct, because during the refactoring process I missed an "n" :)

if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_sec·!=·y.tv_sec))

Should be:

if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_nsec·!=·y.tv_nsec))

My mistake, I am going to fix the test and re-post v5 of this set.

Without my patch if you pass "highres=off" to the kernel (as a command line
parameter) it leads to a broken implementation of clock_getres since the value
of CLOCK_REALTIME_RES does not change at runtime.

Expected result (with highres=off):

# uname -r
5.2.0-rc2
# ./vdso_clock_getres
clock_id: CLOCK_REALTIME [FAIL]
clock_id: CLOCK_BOOTTIME [PASS]
clock_id: CLOCK_TAI [PASS]
clock_id: CLOCK_REALTIME_COARSE [PASS]
clock_id: CLOCK_MONOTONIC [FAIL]
clock_id: CLOCK_MONOTONIC_RAW [PASS]
clock_id: CLOCK_MONOTONIC_COARSE [PASS]

The reason of this behavior is that the only clocks supported by getres on
powerpc are CLOCK_REALTIME and CLOCK_MONOTONIC, the rest on the clocks use
always syscalls.
vdso64 is supposed to implement CLOCK_{REALTIME/MONOTONIC}_COARSE, so I
guess it should fail for them too ?

Or is your test done on vdso32 ?
Based on what I can see in kernel/vdso64 in 5.2-rc3:

/*
   * Exact prototype of clock_getres()
   *
   * int __kernel_clock_getres(clockid_t clock_id, struct timespec *res);
   *
   */
V_FUNCTION_BEGIN(__kernel_clock_getres)
    .cfi_startproc
	/* Check for supported clock IDs */
	cmpwi	cr0,r3,CLOCK_REALTIME
	cmpwi	cr1,r3,CLOCK_MONOTONIC
	cror	cr0*4+eq,cr0*4+eq,cr1*4+eq
	bne	cr0,99f

	li	r3,0
	cmpldi	cr0,r4,0
	crclr	cr0*4+so
	beqlr
	lis	r5,CLOCK_REALTIME_RES@h
	ori	r5,r5,CLOCK_REALTIME_RES@l
	std	r3,TSPC64_TV_SEC(r4)
	std	r5,TSPC64_TV_NSEC(r4)
	blr

	/*
	 * syscall fallback
	 */
99:
	li	r0,__NR_clock_getres
	sc
	blr
    .cfi_endproc
V_FUNCTION_END(__kernel_clock_getres)

it does not seem so for what concerns vdso64. I did run again the test both on
ppc and ppc64 qemu instances and the result is the same to what I reported in
this thread.

Am I missing something?
I was thinking about
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5c929885f1bb
but apparently clock_getres() was left aside. Should we do something
about it ?
Sure, but I would like this series to be merged first (since the topic is
different). I am happy, after that, to push a separate one on top that addresses
the problem.

Please let me know if it works for you and Michael.
No problem for myself.

By the way, next time (or next spin ?) I recommend you to handle your 
patches independently and not as a series since they are all 
independant. It would have avoided confusion and the need for you to 
resend all 3 patches everytime you did a change in one of them.
Thanks for the advise, I will do next time.
Christophe
quoted
quoted
Christophe
quoted
quoted
Christophe
quoted
quoted
# 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
quoted
Cc: Shuah Khan <shuah@kernel.org>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---

Note: This patch is independent from the others in this series, hence it
can be merged singularly by the kselftest maintainers.

    tools/testing/selftests/vDSO/Makefile         |   2 +
    .../selftests/vDSO/vdso_clock_getres.c        | 124 ++++++++++++++++++
    2 files changed, 126 insertions(+)
    create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
-- 
Regards,
Vincenzo
Thread (16 messages) 16 messages, 4 authors, last reply 2019-06-13
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help