From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5BBD4C282CE for ; Tue, 4 Jun 2019 14:27:12 +0000 (UTC) Received: from lists.ozlabs.org (lists.ozlabs.org [203.11.71.2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id DECD224437 for ; Tue, 4 Jun 2019 14:27:11 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DECD224437 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 45JDlL0DhTzDqYb for ; Wed, 5 Jun 2019 00:27:10 +1000 (AEST) Authentication-Results: lists.ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=arm.com (client-ip=217.140.101.70; helo=foss.arm.com; envelope-from=vincenzo.frascino@arm.com; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=arm.com Received: from foss.arm.com (foss.arm.com [217.140.101.70]) by lists.ozlabs.org (Postfix) with ESMTP id 45JDcS4nNDzDqHw for ; Wed, 5 Jun 2019 00:21:11 +1000 (AEST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 4DC53341; Tue, 4 Jun 2019 07:21:10 -0700 (PDT) Received: from [10.1.196.72] (e119884-lin.cambridge.arm.com [10.1.196.72]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 02D163F690; Tue, 4 Jun 2019 07:21:07 -0700 (PDT) Subject: Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres To: Christophe Leroy , Michael Ellerman , linux-arch@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-s390@vger.kernel.org, linux-kselftest@vger.kernel.org References: <20190523112116.19233-1-vincenzo.frascino@arm.com> <20190523112116.19233-4-vincenzo.frascino@arm.com> <87lfyrp0d2.fsf@concordia.ellerman.id.au> <5c99721a-ce6b-10a0-99f2-6c37c1da4542@c-s.fr>

<2389b7f2-288b-215e-994a-3b24be08e125@c-s.fr> From: Vincenzo Frascino Message-ID: <42cdb5cf-797a-2497-92a2-730250ad436a@arm.com> Date: Tue, 4 Jun 2019 15:21:06 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <2389b7f2-288b-215e-994a-3b24be08e125@c-s.fr> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Arnd Bergmann , Heiko Carstens , Paul Mackerras , Martin Schwidefsky , Thomas Gleixner , Shuah Khan Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" 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 >>>> 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 > >> >>> 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