Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
Hi Michael, I wanted to check with you if you had time to have a look at my new version (v5) of the patches with the fixed test, and if they are ready to be merged or if there is anything else I can do. Thanks and Regards, Vincenzo On 28/05/2019 12:57, Vincenzo Frascino wrote: > Hi Michael, > > thank you for your reply. > > On 28/05/2019 07:19, Michael Ellerman wrote: >> Vincenzo Frascino writes: >> >>> The current version of the multiarch vDSO selftest verifies only >>> gettimeofday. >>> >>> Extend the vDSO selftest to clock_getres, to verify that the >>> syscall and the vDSO library function return the same information. >>> >>> The extension has been used to verify the hrtimer_resoltion fix. >> >> This is passing for me even without patch 1 applied, shouldn't it fail >> without the fix? What am I missing? >> > > This is correct, because during the refactoring process I missed an "n" :) > > if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_sec·!=·y.tv_sec)) > > Should be: > > if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_nsec·!=·y.tv_nsec)) > > My mistake, I am going to fix the test and re-post v5 of this set. > > Without my patch if you pass "highres=off" to the kernel (as a command line > parameter) it leads to a broken implementation of clock_getres since the value > of CLOCK_REALTIME_RES does not change at runtime. > > Expected result (with highres=off): > > # uname -r > 5.2.0-rc2 > # ./vdso_clock_getres > clock_id: CLOCK_REALTIME [FAIL] > clock_id: CLOCK_BOOTTIME [PASS] > clock_id: CLOCK_TAI [PASS] > clock_id: CLOCK_REALTIME_COARSE [PASS] > clock_id: CLOCK_MONOTONIC [FAIL] > clock_id: CLOCK_MONOTONIC_RAW [PASS] > clock_id: CLOCK_MONOTONIC_COARSE [PASS] > > The reason of this behavior is that the only clocks supported by getres on > powerpc are CLOCK_REALTIME and CLOCK_MONOTONIC, the rest on the clocks use > always syscalls. > >> # uname -r >> 5.2.0-rc2-gcc-8.2.0 >> >> # ./vdso_clock_getres >> clock_id: CLOCK_REALTIME [PASS] >> clock_id: CLOCK_BOOTTIME [PASS] >> clock_id: CLOCK_TAI [PASS] >> clock_id: CLOCK_REALTIME_COARSE [PASS] >> clock_id: CLOCK_MONOTONIC [PASS] >> clock_id: CLOCK_MONOTONIC_RAW [PASS] >> clock_id: CLOCK_MONOTONIC_COARSE [PASS] >> >> cheers >> >>> Cc: Shuah Khan >>> Signed-off-by: Vincenzo Frascino >>> --- >>> >>> Note: This patch is independent from the others in this series, hence it >>> can be merged singularly by the kselftest maintainers. >>> >>> tools/testing/selftests/vDSO/Makefile | 2 + >>> .../selftests/vDSO/vdso_clock_getres.c| 124 ++ >>> 2 files changed, 126 insertions(+) >>> create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c >
Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
On 04/06/2019 14:52, Christophe Leroy wrote:
>
>
> Le 04/06/2019 à 15:43, Vincenzo Frascino a écrit :
>> On 04/06/2019 14:39, Christophe Leroy wrote:
>>>
>>>
>>> Le 04/06/2019 à 15:32, Vincenzo Frascino a écrit :
Hi Christophe,
On 04/06/2019 14:16, Christophe Leroy wrote:
> Hi Vincenzo
>
> Le 28/05/2019 à 13:57, Vincenzo Frascino a écrit :
>> Hi Michael,
>>
>> thank you for your reply.
>>
>> On 28/05/2019 07:19, Michael Ellerman wrote:
>>> Vincenzo Frascino writes:
>>>
The current version of the multiarch vDSO selftest verifies only
gettimeofday.
Extend the vDSO selftest to clock_getres, to verify that the
syscall and the vDSO library function return the same information.
The extension has been used to verify the hrtimer_resoltion fix.
>>>
>>> This is passing for me even without patch 1 applied, shouldn't it fail
>>> without the fix? What am I missing?
>>>
>>
>> This is correct, because during the refactoring process I missed an "n"
>> :)
>>
>> if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_sec·!=·y.tv_sec))
>>
>> Should be:
>>
>> if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_nsec·!=·y.tv_nsec))
>>
>> My mistake, I am going to fix the test and re-post v5 of this set.
>>
>> Without my patch if you pass "highres=off" to the kernel (as a command
>> line
>> parameter) it leads to a broken implementation of clock_getres since the
>> value
>> of CLOCK_REALTIME_RES does not change at runtime.
>>
>> Expected result (with highres=off):
>>
>> # uname -r
>> 5.2.0-rc2
>> # ./vdso_clock_getres
>> clock_id: CLOCK_REALTIME [FAIL]
>> clock_id: CLOCK_BOOTTIME [PASS]
>> clock_id: CLOCK_TAI [PASS]
>> clock_id: CLOCK_REALTIME_COARSE [PASS]
>> clock_id: CLOCK_MONOTONIC [FAIL]
>> clock_id: CLOCK_MONOTONIC_RAW [PASS]
>> clock_id: CLOCK_MONOTONIC_COARSE [PASS]
>>
>> The reason of this behavior is that the only clocks supported by getres
>> on
>> powerpc are CLOCK_REALTIME and CLOCK_MONOTONIC, the rest on the clocks
>> use
>> always syscalls.
>
> vdso64 is supposed to implement CLOCK_{REALTIME/MONOTONIC}_COARSE, so I
> guess it should fail for them too ?
>
> Or is your test done on vdso32 ?
>
Based on what I can see in kernel/vdso64 in 5.2-rc3:
/*
* Exact prototype of clock_getres()
*
* int __kernel_clock_getres(clockid_t clock_id, struct timespec *res);
*
*/
V_FUNCTION_BEGIN(__kernel_clock_getres)
.cfi_startproc
/* Check for supported clock IDs */
cmpwi cr0,r3,CLOCK_REALTIME
cmpwi cr1,r3,CLOCK_MONOTONIC
crorcr0*4+eq,cr0*4+eq,cr1*4+eq
bne cr0,99f
li r3,0
cmpldi cr0,r4,0
crclr cr0*4+so
beqlr
lis r5,CLOCK_REALTIME_RES@h
ori r5,r5,CLOCK_REALTIME_RES@l
std r3,TSPC64_TV_SEC(r4)
std r5,TSPC64_TV_NSEC(r4)
blr
/*
* syscall fallback
*/
99:
li r0,__NR_clock_getres
sc
blr
.cfi_endproc
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
Signed-off-by: Vincenzo Frascino
---
Note: This patch is independent
Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
Le 04/06/2019 à 15:43, Vincenzo Frascino a écrit :
On 04/06/2019 14:39, Christophe Leroy wrote:
Le 04/06/2019 à 15:32, Vincenzo Frascino a écrit :
Hi Christophe,
On 04/06/2019 14:16, Christophe Leroy wrote:
Hi Vincenzo
Le 28/05/2019 à 13:57, Vincenzo Frascino a écrit :
Hi Michael,
thank you for your reply.
On 28/05/2019 07:19, Michael Ellerman wrote:
Vincenzo Frascino writes:
The current version of the multiarch vDSO selftest verifies only
gettimeofday.
Extend the vDSO selftest to clock_getres, to verify that the
syscall and the vDSO library function return the same information.
The extension has been used to verify the hrtimer_resoltion fix.
This is passing for me even without patch 1 applied, shouldn't it fail
without the fix? What am I missing?
This is correct, because during the refactoring process I missed an "n" :)
if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_sec·!=·y.tv_sec))
Should be:
if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_nsec·!=·y.tv_nsec))
My mistake, I am going to fix the test and re-post v5 of this set.
Without my patch if you pass "highres=off" to the kernel (as a command line
parameter) it leads to a broken implementation of clock_getres since the value
of CLOCK_REALTIME_RES does not change at runtime.
Expected result (with highres=off):
# uname -r
5.2.0-rc2
# ./vdso_clock_getres
clock_id: CLOCK_REALTIME [FAIL]
clock_id: CLOCK_BOOTTIME [PASS]
clock_id: CLOCK_TAI [PASS]
clock_id: CLOCK_REALTIME_COARSE [PASS]
clock_id: CLOCK_MONOTONIC [FAIL]
clock_id: CLOCK_MONOTONIC_RAW [PASS]
clock_id: CLOCK_MONOTONIC_COARSE [PASS]
The reason of this behavior is that the only clocks supported by getres on
powerpc are CLOCK_REALTIME and CLOCK_MONOTONIC, the rest on the clocks use
always syscalls.
vdso64 is supposed to implement CLOCK_{REALTIME/MONOTONIC}_COARSE, so I
guess it should fail for them too ?
Or is your test done on vdso32 ?
Based on what I can see in kernel/vdso64 in 5.2-rc3:
/*
* Exact prototype of clock_getres()
*
* int __kernel_clock_getres(clockid_t clock_id, struct timespec *res);
*
*/
V_FUNCTION_BEGIN(__kernel_clock_getres)
.cfi_startproc
/* Check for supported clock IDs */
cmpwi cr0,r3,CLOCK_REALTIME
cmpwi cr1,r3,CLOCK_MONOTONIC
crorcr0*4+eq,cr0*4+eq,cr1*4+eq
bne cr0,99f
li r3,0
cmpldi cr0,r4,0
crclr cr0*4+so
beqlr
lis r5,CLOCK_REALTIME_RES@h
ori r5,r5,CLOCK_REALTIME_RES@l
std r3,TSPC64_TV_SEC(r4)
std r5,TSPC64_TV_NSEC(r4)
blr
/*
* syscall fallback
*/
99:
li r0,__NR_clock_getres
sc
blr
.cfi_endproc
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
Signed-off-by: Vincenzo Frascino
---
Note: This patch is independent from the others in this series, hence it
can be merged singularly by the kselftest maintainers.
tools/testing/selftests/vDSO/Makefile | 2 +
.../selftests/vDSO/vdso_clock_getres.c| 124 ++
2 files changed, 126 insertions(+)
create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
On 04/06/2019 14:39, Christophe Leroy wrote:
>
>
> Le 04/06/2019 à 15:32, Vincenzo Frascino a écrit :
>> Hi Christophe,
>>
>> On 04/06/2019 14:16, Christophe Leroy wrote:
>>> Hi Vincenzo
>>>
>>> Le 28/05/2019 à 13:57, Vincenzo Frascino a écrit :
Hi Michael,
thank you for your reply.
On 28/05/2019 07:19, Michael Ellerman wrote:
> Vincenzo Frascino writes:
>
>> The current version of the multiarch vDSO selftest verifies only
>> gettimeofday.
>>
>> Extend the vDSO selftest to clock_getres, to verify that the
>> syscall and the vDSO library function return the same information.
>>
>> The extension has been used to verify the hrtimer_resoltion fix.
>
> This is passing for me even without patch 1 applied, shouldn't it fail
> without the fix? What am I missing?
>
This is correct, because during the refactoring process I missed an "n" :)
if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_sec·!=·y.tv_sec))
Should be:
if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_nsec·!=·y.tv_nsec))
My mistake, I am going to fix the test and re-post v5 of this set.
Without my patch if you pass "highres=off" to the kernel (as a command line
parameter) it leads to a broken implementation of clock_getres since the
value
of CLOCK_REALTIME_RES does not change at runtime.
Expected result (with highres=off):
# uname -r
5.2.0-rc2
# ./vdso_clock_getres
clock_id: CLOCK_REALTIME [FAIL]
clock_id: CLOCK_BOOTTIME [PASS]
clock_id: CLOCK_TAI [PASS]
clock_id: CLOCK_REALTIME_COARSE [PASS]
clock_id: CLOCK_MONOTONIC [FAIL]
clock_id: CLOCK_MONOTONIC_RAW [PASS]
clock_id: CLOCK_MONOTONIC_COARSE [PASS]
The reason of this behavior is that the only clocks supported by getres on
powerpc are CLOCK_REALTIME and CLOCK_MONOTONIC, the rest on the clocks use
always syscalls.
>>>
>>> vdso64 is supposed to implement CLOCK_{REALTIME/MONOTONIC}_COARSE, so I
>>> guess it should fail for them too ?
>>>
>>> Or is your test done on vdso32 ?
>>>
>>
>> Based on what I can see in kernel/vdso64 in 5.2-rc3:
>>
>> /*
>> * Exact prototype of clock_getres()
>> *
>> * int __kernel_clock_getres(clockid_t clock_id, struct timespec *res);
>> *
>> */
>> V_FUNCTION_BEGIN(__kernel_clock_getres)
>>.cfi_startproc
>> /* Check for supported clock IDs */
>> cmpwi cr0,r3,CLOCK_REALTIME
>> cmpwi cr1,r3,CLOCK_MONOTONIC
>> crorcr0*4+eq,cr0*4+eq,cr1*4+eq
>> bne cr0,99f
>>
>> li r3,0
>> cmpldi cr0,r4,0
>> crclr cr0*4+so
>> beqlr
>> lis r5,CLOCK_REALTIME_RES@h
>> ori r5,r5,CLOCK_REALTIME_RES@l
>> std r3,TSPC64_TV_SEC(r4)
>> std r5,TSPC64_TV_NSEC(r4)
>> blr
>>
>> /*
>> * syscall fallback
>> */
>> 99:
>> li r0,__NR_clock_getres
>> sc
>> blr
>>.cfi_endproc
>> V_FUNCTION_END(__kernel_clock_getres)
>>
>> it does not seem so for what concerns vdso64. I did run again the test both
>> on
>> ppc and ppc64 qemu instances and the result is the same to what I reported in
>> this thread.
>>
>> Am I missing something?
>
> I was thinking about
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5c929885f1bb
>
> but apparently clock_getres() was left aside. Should we do something
> about it ?
>
Sure, but I would like this series to be merged first (since the topic is
different). I am happy, after that, to push a separate one on top that addresses
the problem.
Please let me know if it works for you and Michael.
> Christophe
>
>>
>>> Christophe
>>>
> # uname -r
> 5.2.0-rc2-gcc-8.2.0
>
> # ./vdso_clock_getres
> clock_id: CLOCK_REALTIME [PASS]
> clock_id: CLOCK_BOOTTIME [PASS]
> clock_id: CLOCK_TAI [PASS]
> clock_id: CLOCK_REALTIME_COARSE [PASS]
> clock_id: CLOCK_MONOTONIC [PASS]
> clock_id: CLOCK_MONOTONIC_RAW [PASS]
> clock_id: CLOCK_MONOTONIC_COARSE [PASS]
>
> cheers
>
>> Cc: Shuah Khan
>> Signed-off-by: Vincenzo Frascino
>> ---
>>
>> Note: This patch is independent from the others in this series, hence it
>> can be merged singularly by the kselftest maintainers.
>>
>>tools/testing/selftests/vDSO/Makefile | 2 +
>>.../selftests/vDSO/vdso_clock_getres.c| 124 ++
>>2 files changed, 126 insertions(+)
>>create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
>>
--
Regards,
Vincenzo
Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
Le 04/06/2019 à 15:32, Vincenzo Frascino a écrit :
Hi Christophe,
On 04/06/2019 14:16, Christophe Leroy wrote:
Hi Vincenzo
Le 28/05/2019 à 13:57, Vincenzo Frascino a écrit :
Hi Michael,
thank you for your reply.
On 28/05/2019 07:19, Michael Ellerman wrote:
Vincenzo Frascino writes:
The current version of the multiarch vDSO selftest verifies only
gettimeofday.
Extend the vDSO selftest to clock_getres, to verify that the
syscall and the vDSO library function return the same information.
The extension has been used to verify the hrtimer_resoltion fix.
This is passing for me even without patch 1 applied, shouldn't it fail
without the fix? What am I missing?
This is correct, because during the refactoring process I missed an "n" :)
if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_sec·!=·y.tv_sec))
Should be:
if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_nsec·!=·y.tv_nsec))
My mistake, I am going to fix the test and re-post v5 of this set.
Without my patch if you pass "highres=off" to the kernel (as a command line
parameter) it leads to a broken implementation of clock_getres since the value
of CLOCK_REALTIME_RES does not change at runtime.
Expected result (with highres=off):
# uname -r
5.2.0-rc2
# ./vdso_clock_getres
clock_id: CLOCK_REALTIME [FAIL]
clock_id: CLOCK_BOOTTIME [PASS]
clock_id: CLOCK_TAI [PASS]
clock_id: CLOCK_REALTIME_COARSE [PASS]
clock_id: CLOCK_MONOTONIC [FAIL]
clock_id: CLOCK_MONOTONIC_RAW [PASS]
clock_id: CLOCK_MONOTONIC_COARSE [PASS]
The reason of this behavior is that the only clocks supported by getres on
powerpc are CLOCK_REALTIME and CLOCK_MONOTONIC, the rest on the clocks use
always syscalls.
vdso64 is supposed to implement CLOCK_{REALTIME/MONOTONIC}_COARSE, so I
guess it should fail for them too ?
Or is your test done on vdso32 ?
Based on what I can see in kernel/vdso64 in 5.2-rc3:
/*
* Exact prototype of clock_getres()
*
* int __kernel_clock_getres(clockid_t clock_id, struct timespec *res);
*
*/
V_FUNCTION_BEGIN(__kernel_clock_getres)
.cfi_startproc
/* Check for supported clock IDs */
cmpwi cr0,r3,CLOCK_REALTIME
cmpwi cr1,r3,CLOCK_MONOTONIC
crorcr0*4+eq,cr0*4+eq,cr1*4+eq
bne cr0,99f
li r3,0
cmpldi cr0,r4,0
crclr cr0*4+so
beqlr
lis r5,CLOCK_REALTIME_RES@h
ori r5,r5,CLOCK_REALTIME_RES@l
std r3,TSPC64_TV_SEC(r4)
std r5,TSPC64_TV_NSEC(r4)
blr
/*
* syscall fallback
*/
99:
li r0,__NR_clock_getres
sc
blr
.cfi_endproc
V_FUNCTION_END(__kernel_clock_getres)
it does not seem so for what concerns vdso64. I did run again the test both on
ppc and ppc64 qemu instances and the result is the same to what I reported in
this thread.
Am I missing something?
I was thinking about
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5c929885f1bb
but apparently clock_getres() was left aside. Should we do something
about it ?
Christophe
Christophe
# uname -r
5.2.0-rc2-gcc-8.2.0
# ./vdso_clock_getres
clock_id: CLOCK_REALTIME [PASS]
clock_id: CLOCK_BOOTTIME [PASS]
clock_id: CLOCK_TAI [PASS]
clock_id: CLOCK_REALTIME_COARSE [PASS]
clock_id: CLOCK_MONOTONIC [PASS]
clock_id: CLOCK_MONOTONIC_RAW [PASS]
clock_id: CLOCK_MONOTONIC_COARSE [PASS]
cheers
Cc: Shuah Khan
Signed-off-by: Vincenzo Frascino
---
Note: This patch is independent from the others in this series, hence it
can be merged singularly by the kselftest maintainers.
tools/testing/selftests/vDSO/Makefile | 2 +
.../selftests/vDSO/vdso_clock_getres.c| 124 ++
2 files changed, 126 insertions(+)
create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
Hi Christophe,
On 04/06/2019 14:16, Christophe Leroy wrote:
> Hi Vincenzo
>
> Le 28/05/2019 à 13:57, Vincenzo Frascino a écrit :
>> Hi Michael,
>>
>> thank you for your reply.
>>
>> On 28/05/2019 07:19, Michael Ellerman wrote:
>>> Vincenzo Frascino writes:
>>>
The current version of the multiarch vDSO selftest verifies only
gettimeofday.
Extend the vDSO selftest to clock_getres, to verify that the
syscall and the vDSO library function return the same information.
The extension has been used to verify the hrtimer_resoltion fix.
>>>
>>> This is passing for me even without patch 1 applied, shouldn't it fail
>>> without the fix? What am I missing?
>>>
>>
>> This is correct, because during the refactoring process I missed an "n" :)
>>
>> if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_sec·!=·y.tv_sec))
>>
>> Should be:
>>
>> if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_nsec·!=·y.tv_nsec))
>>
>> My mistake, I am going to fix the test and re-post v5 of this set.
>>
>> Without my patch if you pass "highres=off" to the kernel (as a command line
>> parameter) it leads to a broken implementation of clock_getres since the
>> value
>> of CLOCK_REALTIME_RES does not change at runtime.
>>
>> Expected result (with highres=off):
>>
>> # uname -r
>> 5.2.0-rc2
>> # ./vdso_clock_getres
>> clock_id: CLOCK_REALTIME [FAIL]
>> clock_id: CLOCK_BOOTTIME [PASS]
>> clock_id: CLOCK_TAI [PASS]
>> clock_id: CLOCK_REALTIME_COARSE [PASS]
>> clock_id: CLOCK_MONOTONIC [FAIL]
>> clock_id: CLOCK_MONOTONIC_RAW [PASS]
>> clock_id: CLOCK_MONOTONIC_COARSE [PASS]
>>
>> The reason of this behavior is that the only clocks supported by getres on
>> powerpc are CLOCK_REALTIME and CLOCK_MONOTONIC, the rest on the clocks use
>> always syscalls.
>
> vdso64 is supposed to implement CLOCK_{REALTIME/MONOTONIC}_COARSE, so I
> guess it should fail for them too ?
>
> Or is your test done on vdso32 ?
>
Based on what I can see in kernel/vdso64 in 5.2-rc3:
/*
* Exact prototype of clock_getres()
*
* int __kernel_clock_getres(clockid_t clock_id, struct timespec *res);
*
*/
V_FUNCTION_BEGIN(__kernel_clock_getres)
.cfi_startproc
/* Check for supported clock IDs */
cmpwi cr0,r3,CLOCK_REALTIME
cmpwi cr1,r3,CLOCK_MONOTONIC
crorcr0*4+eq,cr0*4+eq,cr1*4+eq
bne cr0,99f
li r3,0
cmpldi cr0,r4,0
crclr cr0*4+so
beqlr
lis r5,CLOCK_REALTIME_RES@h
ori r5,r5,CLOCK_REALTIME_RES@l
std r3,TSPC64_TV_SEC(r4)
std r5,TSPC64_TV_NSEC(r4)
blr
/*
* syscall fallback
*/
99:
li r0,__NR_clock_getres
sc
blr
.cfi_endproc
V_FUNCTION_END(__kernel_clock_getres)
it does not seem so for what concerns vdso64. I did run again the test both on
ppc and ppc64 qemu instances and the result is the same to what I reported in
this thread.
Am I missing something?
> Christophe
>
>>
>>> # uname -r
>>> 5.2.0-rc2-gcc-8.2.0
>>>
>>> # ./vdso_clock_getres
>>> clock_id: CLOCK_REALTIME [PASS]
>>> clock_id: CLOCK_BOOTTIME [PASS]
>>> clock_id: CLOCK_TAI [PASS]
>>> clock_id: CLOCK_REALTIME_COARSE [PASS]
>>> clock_id: CLOCK_MONOTONIC [PASS]
>>> clock_id: CLOCK_MONOTONIC_RAW [PASS]
>>> clock_id: CLOCK_MONOTONIC_COARSE [PASS]
>>>
>>> cheers
>>>
Cc: Shuah Khan
Signed-off-by: Vincenzo Frascino
---
Note: This patch is independent from the others in this series, hence it
can be merged singularly by the kselftest maintainers.
tools/testing/selftests/vDSO/Makefile | 2 +
.../selftests/vDSO/vdso_clock_getres.c| 124 ++
2 files changed, 126 insertions(+)
create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
>>
--
Regards,
Vincenzo
Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
Hi Vincenzo
Le 28/05/2019 à 13:57, Vincenzo Frascino a écrit :
Hi Michael,
thank you for your reply.
On 28/05/2019 07:19, Michael Ellerman wrote:
Vincenzo Frascino writes:
The current version of the multiarch vDSO selftest verifies only
gettimeofday.
Extend the vDSO selftest to clock_getres, to verify that the
syscall and the vDSO library function return the same information.
The extension has been used to verify the hrtimer_resoltion fix.
This is passing for me even without patch 1 applied, shouldn't it fail
without the fix? What am I missing?
This is correct, because during the refactoring process I missed an "n" :)
if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_sec·!=·y.tv_sec))
Should be:
if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_nsec·!=·y.tv_nsec))
My mistake, I am going to fix the test and re-post v5 of this set.
Without my patch if you pass "highres=off" to the kernel (as a command line
parameter) it leads to a broken implementation of clock_getres since the value
of CLOCK_REALTIME_RES does not change at runtime.
Expected result (with highres=off):
# uname -r
5.2.0-rc2
# ./vdso_clock_getres
clock_id: CLOCK_REALTIME [FAIL]
clock_id: CLOCK_BOOTTIME [PASS]
clock_id: CLOCK_TAI [PASS]
clock_id: CLOCK_REALTIME_COARSE [PASS]
clock_id: CLOCK_MONOTONIC [FAIL]
clock_id: CLOCK_MONOTONIC_RAW [PASS]
clock_id: CLOCK_MONOTONIC_COARSE [PASS]
The reason of this behavior is that the only clocks supported by getres on
powerpc are CLOCK_REALTIME and CLOCK_MONOTONIC, the rest on the clocks use
always syscalls.
vdso64 is supposed to implement CLOCK_{REALTIME/MONOTONIC}_COARSE, so I
guess it should fail for them too ?
Or is your test done on vdso32 ?
Christophe
# uname -r
5.2.0-rc2-gcc-8.2.0
# ./vdso_clock_getres
clock_id: CLOCK_REALTIME [PASS]
clock_id: CLOCK_BOOTTIME [PASS]
clock_id: CLOCK_TAI [PASS]
clock_id: CLOCK_REALTIME_COARSE [PASS]
clock_id: CLOCK_MONOTONIC [PASS]
clock_id: CLOCK_MONOTONIC_RAW [PASS]
clock_id: CLOCK_MONOTONIC_COARSE [PASS]
cheers
Cc: Shuah Khan
Signed-off-by: Vincenzo Frascino
---
Note: This patch is independent from the others in this series, hence it
can be merged singularly by the kselftest maintainers.
tools/testing/selftests/vDSO/Makefile | 2 +
.../selftests/vDSO/vdso_clock_getres.c| 124 ++
2 files changed, 126 insertions(+)
create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
Hi Christophe, On 28/05/2019 18:01, Christophe Leroy wrote: > Vincenzo Frascino a écrit : > >> Hi Michael, >> >> thank you for your reply. >> >> On 28/05/2019 07:19, Michael Ellerman wrote: >>> Vincenzo Frascino writes: >>> The current version of the multiarch vDSO selftest verifies only gettimeofday. Extend the vDSO selftest to clock_getres, to verify that the syscall and the vDSO library function return the same information. The extension has been used to verify the hrtimer_resoltion fix. >>> >>> This is passing for me even without patch 1 applied, shouldn't it fail >>> without the fix? What am I missing? >>> >> >> This is correct, because during the refactoring process I missed an "n" :) >> >> if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_sec·!=·y.tv_sec)) >> >> Should be: >> >> if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_nsec·!=·y.tv_nsec)) > > Maybe you'd better use timercmp() from sys/time.h > timercmp() takes "struct timeval" not "struct timespec". > Christophe > >> >> My mistake, I am going to fix the test and re-post v5 of this set. >> >> Without my patch if you pass "highres=off" to the kernel (as a command line >> parameter) it leads to a broken implementation of clock_getres since >> the value >> of CLOCK_REALTIME_RES does not change at runtime. >> >> Expected result (with highres=off): >> >> # uname -r >> 5.2.0-rc2 >> # ./vdso_clock_getres >> clock_id: CLOCK_REALTIME [FAIL] >> clock_id: CLOCK_BOOTTIME [PASS] >> clock_id: CLOCK_TAI [PASS] >> clock_id: CLOCK_REALTIME_COARSE [PASS] >> clock_id: CLOCK_MONOTONIC [FAIL] >> clock_id: CLOCK_MONOTONIC_RAW [PASS] >> clock_id: CLOCK_MONOTONIC_COARSE [PASS] >> >> The reason of this behavior is that the only clocks supported by getres on >> powerpc are CLOCK_REALTIME and CLOCK_MONOTONIC, the rest on the clocks use >> always syscalls. >> >>> # uname -r >>> 5.2.0-rc2-gcc-8.2.0 >>> >>> # ./vdso_clock_getres >>> clock_id: CLOCK_REALTIME [PASS] >>> clock_id: CLOCK_BOOTTIME [PASS] >>> clock_id: CLOCK_TAI [PASS] >>> clock_id: CLOCK_REALTIME_COARSE [PASS] >>> clock_id: CLOCK_MONOTONIC [PASS] >>> clock_id: CLOCK_MONOTONIC_RAW [PASS] >>> clock_id: CLOCK_MONOTONIC_COARSE [PASS] >>> >>> cheers >>> Cc: Shuah Khan Signed-off-by: Vincenzo Frascino --- Note: This patch is independent from the others in this series, hence it can be merged singularly by the kselftest maintainers. tools/testing/selftests/vDSO/Makefile | 2 + .../selftests/vDSO/vdso_clock_getres.c| 124 ++ 2 files changed, 126 insertions(+) create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c >> >> -- >> Regards, >> Vincenzo > > -- Regards, Vincenzo
Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
Vincenzo Frascino a écrit : Hi Michael, thank you for your reply. On 28/05/2019 07:19, Michael Ellerman wrote: Vincenzo Frascino writes: The current version of the multiarch vDSO selftest verifies only gettimeofday. Extend the vDSO selftest to clock_getres, to verify that the syscall and the vDSO library function return the same information. The extension has been used to verify the hrtimer_resoltion fix. This is passing for me even without patch 1 applied, shouldn't it fail without the fix? What am I missing? This is correct, because during the refactoring process I missed an "n" :) if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_sec·!=·y.tv_sec)) Should be: if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_nsec·!=·y.tv_nsec)) Maybe you'd better use timercmp() from sys/time.h Christophe My mistake, I am going to fix the test and re-post v5 of this set. Without my patch if you pass "highres=off" to the kernel (as a command line parameter) it leads to a broken implementation of clock_getres since the value of CLOCK_REALTIME_RES does not change at runtime. Expected result (with highres=off): # uname -r 5.2.0-rc2 # ./vdso_clock_getres clock_id: CLOCK_REALTIME [FAIL] clock_id: CLOCK_BOOTTIME [PASS] clock_id: CLOCK_TAI [PASS] clock_id: CLOCK_REALTIME_COARSE [PASS] clock_id: CLOCK_MONOTONIC [FAIL] clock_id: CLOCK_MONOTONIC_RAW [PASS] clock_id: CLOCK_MONOTONIC_COARSE [PASS] The reason of this behavior is that the only clocks supported by getres on powerpc are CLOCK_REALTIME and CLOCK_MONOTONIC, the rest on the clocks use always syscalls. # uname -r 5.2.0-rc2-gcc-8.2.0 # ./vdso_clock_getres clock_id: CLOCK_REALTIME [PASS] clock_id: CLOCK_BOOTTIME [PASS] clock_id: CLOCK_TAI [PASS] clock_id: CLOCK_REALTIME_COARSE [PASS] clock_id: CLOCK_MONOTONIC [PASS] clock_id: CLOCK_MONOTONIC_RAW [PASS] clock_id: CLOCK_MONOTONIC_COARSE [PASS] cheers Cc: Shuah Khan Signed-off-by: Vincenzo Frascino --- Note: This patch is independent from the others in this series, hence it can be merged singularly by the kselftest maintainers. tools/testing/selftests/vDSO/Makefile | 2 + .../selftests/vDSO/vdso_clock_getres.c| 124 ++ 2 files changed, 126 insertions(+) create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c -- Regards, Vincenzo
Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
Hi Michael, thank you for your reply. On 28/05/2019 07:19, Michael Ellerman wrote: > Vincenzo Frascino writes: > >> The current version of the multiarch vDSO selftest verifies only >> gettimeofday. >> >> Extend the vDSO selftest to clock_getres, to verify that the >> syscall and the vDSO library function return the same information. >> >> The extension has been used to verify the hrtimer_resoltion fix. > > This is passing for me even without patch 1 applied, shouldn't it fail > without the fix? What am I missing? > This is correct, because during the refactoring process I missed an "n" :) if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_sec·!=·y.tv_sec)) Should be: if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_nsec·!=·y.tv_nsec)) My mistake, I am going to fix the test and re-post v5 of this set. Without my patch if you pass "highres=off" to the kernel (as a command line parameter) it leads to a broken implementation of clock_getres since the value of CLOCK_REALTIME_RES does not change at runtime. Expected result (with highres=off): # uname -r 5.2.0-rc2 # ./vdso_clock_getres clock_id: CLOCK_REALTIME [FAIL] clock_id: CLOCK_BOOTTIME [PASS] clock_id: CLOCK_TAI [PASS] clock_id: CLOCK_REALTIME_COARSE [PASS] clock_id: CLOCK_MONOTONIC [FAIL] clock_id: CLOCK_MONOTONIC_RAW [PASS] clock_id: CLOCK_MONOTONIC_COARSE [PASS] The reason of this behavior is that the only clocks supported by getres on powerpc are CLOCK_REALTIME and CLOCK_MONOTONIC, the rest on the clocks use always syscalls. > # uname -r > 5.2.0-rc2-gcc-8.2.0 > > # ./vdso_clock_getres > clock_id: CLOCK_REALTIME [PASS] > clock_id: CLOCK_BOOTTIME [PASS] > clock_id: CLOCK_TAI [PASS] > clock_id: CLOCK_REALTIME_COARSE [PASS] > clock_id: CLOCK_MONOTONIC [PASS] > clock_id: CLOCK_MONOTONIC_RAW [PASS] > clock_id: CLOCK_MONOTONIC_COARSE [PASS] > > cheers > >> Cc: Shuah Khan >> Signed-off-by: Vincenzo Frascino >> --- >> >> Note: This patch is independent from the others in this series, hence it >> can be merged singularly by the kselftest maintainers. >> >> tools/testing/selftests/vDSO/Makefile | 2 + >> .../selftests/vDSO/vdso_clock_getres.c| 124 ++ >> 2 files changed, 126 insertions(+) >> create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c -- Regards, Vincenzo
Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
Vincenzo Frascino writes: > The current version of the multiarch vDSO selftest verifies only > gettimeofday. > > Extend the vDSO selftest to clock_getres, to verify that the > syscall and the vDSO library function return the same information. > > The extension has been used to verify the hrtimer_resoltion fix. This is passing for me even without patch 1 applied, shouldn't it fail without the fix? What am I missing? # uname -r 5.2.0-rc2-gcc-8.2.0 # ./vdso_clock_getres clock_id: CLOCK_REALTIME [PASS] clock_id: CLOCK_BOOTTIME [PASS] clock_id: CLOCK_TAI [PASS] clock_id: CLOCK_REALTIME_COARSE [PASS] clock_id: CLOCK_MONOTONIC [PASS] clock_id: CLOCK_MONOTONIC_RAW [PASS] clock_id: CLOCK_MONOTONIC_COARSE [PASS] cheers > Cc: Shuah Khan > Signed-off-by: Vincenzo Frascino > --- > > Note: This patch is independent from the others in this series, hence it > can be merged singularly by the kselftest maintainers. > > tools/testing/selftests/vDSO/Makefile | 2 + > .../selftests/vDSO/vdso_clock_getres.c| 124 ++ > 2 files changed, 126 insertions(+) > create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
[PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
The current version of the multiarch vDSO selftest verifies only
gettimeofday.
Extend the vDSO selftest to clock_getres, to verify that the
syscall and the vDSO library function return the same information.
The extension has been used to verify the hrtimer_resoltion fix.
Cc: Shuah Khan
Signed-off-by: Vincenzo Frascino
---
Note: This patch is independent from the others in this series, hence it
can be merged singularly by the kselftest maintainers.
tools/testing/selftests/vDSO/Makefile | 2 +
.../selftests/vDSO/vdso_clock_getres.c| 124 ++
2 files changed, 126 insertions(+)
create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
diff --git a/tools/testing/selftests/vDSO/Makefile
b/tools/testing/selftests/vDSO/Makefile
index 9e03d61f52fd..d5c5bfdf1ac1 100644
--- a/tools/testing/selftests/vDSO/Makefile
+++ b/tools/testing/selftests/vDSO/Makefile
@@ -5,6 +5,7 @@ uname_M := $(shell uname -m 2>/dev/null || echo not)
ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/)
TEST_GEN_PROGS := $(OUTPUT)/vdso_test
+TEST_GEN_PROGS += $(OUTPUT)/vdso_clock_getres
ifeq ($(ARCH),x86)
TEST_GEN_PROGS += $(OUTPUT)/vdso_standalone_test_x86
endif
@@ -18,6 +19,7 @@ endif
all: $(TEST_GEN_PROGS)
$(OUTPUT)/vdso_test: parse_vdso.c vdso_test.c
+$(OUTPUT)/vdso_clock_getres: vdso_clock_getres.c
$(OUTPUT)/vdso_standalone_test_x86: vdso_standalone_test_x86.c parse_vdso.c
$(CC) $(CFLAGS) $(CFLAGS_vdso_standalone_test_x86) \
vdso_standalone_test_x86.c parse_vdso.c \
diff --git a/tools/testing/selftests/vDSO/vdso_clock_getres.c
b/tools/testing/selftests/vDSO/vdso_clock_getres.c
new file mode 100644
index ..b62d8d4f7c38
--- /dev/null
+++ b/tools/testing/selftests/vDSO/vdso_clock_getres.c
@@ -0,0 +1,124 @@
+// SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note
+/*
+ * vdso_clock_getres.c: Sample code to test clock_getres.
+ * Copyright (c) 2019 Arm Ltd.
+ *
+ * Compile with:
+ * gcc -std=gnu99 vdso_clock_getres.c
+ *
+ * Tested on ARM, ARM64, MIPS32, x86 (32-bit and 64-bit),
+ * Power (32-bit and 64-bit), S390x (32-bit and 64-bit).
+ * Might work on other architectures.
+ */
+
+#define _GNU_SOURCE
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "../kselftest.h"
+
+static long syscall_clock_getres(clockid_t _clkid, struct timespec *_ts)
+{
+ long ret;
+
+ ret = syscall(SYS_clock_getres, _clkid, _ts);
+
+ return ret;
+}
+
+const char *vdso_clock_name[12] = {
+ "CLOCK_REALTIME",
+ "CLOCK_MONOTONIC",
+ "CLOCK_PROCESS_CPUTIME_ID",
+ "CLOCK_THREAD_CPUTIME_ID",
+ "CLOCK_MONOTONIC_RAW",
+ "CLOCK_REALTIME_COARSE",
+ "CLOCK_MONOTONIC_COARSE",
+ "CLOCK_BOOTTIME",
+ "CLOCK_REALTIME_ALARM",
+ "CLOCK_BOOTTIME_ALARM",
+ "CLOCK_SGI_CYCLE",
+ "CLOCK_TAI",
+};
+
+/*
+ * This function calls clock_getres in vdso and by system call
+ * with different values for clock_id.
+ *
+ * Example of output:
+ *
+ * clock_id: CLOCK_REALTIME [PASS]
+ * clock_id: CLOCK_BOOTTIME [PASS]
+ * clock_id: CLOCK_TAI [PASS]
+ * clock_id: CLOCK_REALTIME_COARSE [PASS]
+ * clock_id: CLOCK_MONOTONIC [PASS]
+ * clock_id: CLOCK_MONOTONIC_RAW [PASS]
+ * clock_id: CLOCK_MONOTONIC_COARSE [PASS]
+ */
+static inline int vdso_test_clock(unsigned int clock_id)
+{
+ struct timespec x, y;
+
+ printf("clock_id: %s", vdso_clock_name[clock_id]);
+ clock_getres(clock_id, );
+ syscall_clock_getres(clock_id, );
+
+ 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
