From: Vincenzo Frascino <vincenzo.frascino@arm.com>
To: Christophe Leroy <christophe.leroy@c-s.fr>,
Michael Ellerman <mpe@ellerman.id.au>,
linux-arch@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
linux-s390@vger.kernel.org, linux-kselftest@vger.kernel.org
Cc: Arnd Bergmann <arnd@arndb.de>,
Heiko Carstens <heiko.carstens@de.ibm.com>,
Paul Mackerras <paulus@samba.org>,
Martin Schwidefsky <schwidefsky@de.ibm.com>,
Thomas Gleixner <tglx@linutronix.de>,
Shuah Khan <shuah@kernel.org>
Subject: Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
Date: Tue, 4 Jun 2019 14:32:01 +0100 [thread overview]
Message-ID: <b710d906-edac-f8a7-792b-e6822399187c@arm.com> (raw)
In-Reply-To: <5c99721a-ce6b-10a0-99f2-6c37c1da4542@c-s.fr>
Hi Christophe,
On 04/06/2019 14:16, Christophe Leroy wrote:
> Hi Vincenzo
>
> Le 28/05/2019 à 13:57, Vincenzo Frascino a écrit :
>> Hi Michael,
>>
>> thank you for your reply.
>>
>> On 28/05/2019 07:19, Michael Ellerman wrote:
>>> Vincenzo Frascino <vincenzo.frascino@arm.com> writes:
>>>
>>>> The current version of the multiarch vDSO selftest verifies only
>>>> gettimeofday.
>>>>
>>>> Extend the vDSO selftest to clock_getres, to verify that the
>>>> syscall and the vDSO library function return the same information.
>>>>
>>>> The extension has been used to verify the hrtimer_resoltion fix.
>>>
>>> This is passing for me even without patch 1 applied, shouldn't it fail
>>> without the fix? What am I missing?
>>>
>>
>> This is correct, because during the refactoring process I missed an "n" :)
>>
>> if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_sec·!=·y.tv_sec))
>>
>> Should be:
>>
>> if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_nsec·!=·y.tv_nsec))
>>
>> My mistake, I am going to fix the test and re-post v5 of this set.
>>
>> Without my patch if you pass "highres=off" to the kernel (as a command line
>> parameter) it leads to a broken implementation of clock_getres since the value
>> of CLOCK_REALTIME_RES does not change at runtime.
>>
>> Expected result (with highres=off):
>>
>> # uname -r
>> 5.2.0-rc2
>> # ./vdso_clock_getres
>> clock_id: CLOCK_REALTIME [FAIL]
>> clock_id: CLOCK_BOOTTIME [PASS]
>> clock_id: CLOCK_TAI [PASS]
>> clock_id: CLOCK_REALTIME_COARSE [PASS]
>> clock_id: CLOCK_MONOTONIC [FAIL]
>> clock_id: CLOCK_MONOTONIC_RAW [PASS]
>> clock_id: CLOCK_MONOTONIC_COARSE [PASS]
>>
>> The reason of this behavior is that the only clocks supported by getres on
>> powerpc are CLOCK_REALTIME and CLOCK_MONOTONIC, the rest on the clocks use
>> always syscalls.
>
> vdso64 is supposed to implement CLOCK_{REALTIME/MONOTONIC}_COARSE, so I
> guess it should fail for them too ?
>
> Or is your test done on vdso32 ?
>
Based on what I can see in kernel/vdso64 in 5.2-rc3:
/*
* Exact prototype of clock_getres()
*
* int __kernel_clock_getres(clockid_t clock_id, struct timespec *res);
*
*/
V_FUNCTION_BEGIN(__kernel_clock_getres)
.cfi_startproc
/* Check for supported clock IDs */
cmpwi cr0,r3,CLOCK_REALTIME
cmpwi cr1,r3,CLOCK_MONOTONIC
cror cr0*4+eq,cr0*4+eq,cr1*4+eq
bne cr0,99f
li r3,0
cmpldi cr0,r4,0
crclr cr0*4+so
beqlr
lis r5,CLOCK_REALTIME_RES@h
ori r5,r5,CLOCK_REALTIME_RES@l
std r3,TSPC64_TV_SEC(r4)
std r5,TSPC64_TV_NSEC(r4)
blr
/*
* syscall fallback
*/
99:
li r0,__NR_clock_getres
sc
blr
.cfi_endproc
V_FUNCTION_END(__kernel_clock_getres)
it does not seem so for what concerns vdso64. I did run again the test both on
ppc and ppc64 qemu instances and the result is the same to what I reported in
this thread.
Am I missing something?
> Christophe
>
>>
>>> # uname -r
>>> 5.2.0-rc2-gcc-8.2.0
>>>
>>> # ./vdso_clock_getres
>>> clock_id: CLOCK_REALTIME [PASS]
>>> clock_id: CLOCK_BOOTTIME [PASS]
>>> clock_id: CLOCK_TAI [PASS]
>>> clock_id: CLOCK_REALTIME_COARSE [PASS]
>>> clock_id: CLOCK_MONOTONIC [PASS]
>>> clock_id: CLOCK_MONOTONIC_RAW [PASS]
>>> clock_id: CLOCK_MONOTONIC_COARSE [PASS]
>>>
>>> cheers
>>>
>>>> Cc: Shuah Khan <shuah@kernel.org>
>>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>>>> ---
>>>>
>>>> Note: This patch is independent from the others in this series, hence it
>>>> can be merged singularly by the kselftest maintainers.
>>>>
>>>> tools/testing/selftests/vDSO/Makefile | 2 +
>>>> .../selftests/vDSO/vdso_clock_getres.c | 124 ++++++++++++++++++
>>>> 2 files changed, 126 insertions(+)
>>>> create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
>>
--
Regards,
Vincenzo
next prev parent reply other threads:[~2019-06-04 13:34 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-23 11:21 [PATCH v4 0/3] Fix vDSO clock_getres() Vincenzo Frascino
2019-05-23 11:21 ` [PATCH v4 1/3] powerpc: " Vincenzo Frascino
2019-05-29 13:14 ` Sasha Levin
2019-05-23 11:21 ` [PATCH v4 2/3] s390: " Vincenzo Frascino
2019-05-23 11:21 ` [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres Vincenzo Frascino
2019-05-28 6:19 ` Michael Ellerman
2019-05-28 11:57 ` Vincenzo Frascino
2019-05-28 17:01 ` Christophe Leroy
2019-05-28 17:05 ` Vincenzo Frascino
2019-06-04 13:16 ` Christophe Leroy
2019-06-04 13:32 ` Vincenzo Frascino [this message]
2019-06-04 13:39 ` Christophe Leroy
2019-06-04 13:43 ` Vincenzo Frascino
2019-06-04 13:52 ` Christophe Leroy
2019-06-04 14:21 ` Vincenzo Frascino
2019-06-13 15:22 ` Vincenzo Frascino
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b710d906-edac-f8a7-792b-e6822399187c@arm.com \
--to=vincenzo.frascino@arm.com \
--cc=arnd@arndb.de \
--cc=christophe.leroy@c-s.fr \
--cc=heiko.carstens@de.ibm.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mpe@ellerman.id.au \
--cc=paulus@samba.org \
--cc=schwidefsky@de.ibm.com \
--cc=shuah@kernel.org \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).