From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-2.9 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_GMAIL_RCVD, FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=no autolearn_force=no version=3.4.2 Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id A18931F4B5 for ; Thu, 21 Nov 2019 18:43:36 +0000 (UTC) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-type; q=dns; s=default; b=rsRj t3CPlYPsT7awyCLGs0eMo6YW71aYQhCxdGN2pWKIMfRLgOXYof5SvHGNSzEDGQgs 5ZItltbTjdi4Sj7QYGgSPjCCHeZ/c4Czj14pZIfMaP/Py4nEjkT0Mlk+7bAFCJf1 OxCnmICUoIByWKO4lc9uWGdhnHU57iuE2aIYVhs= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-type; s=default; bh=WIs+AGSRAx Btl1xP0/sdndJlb34=; b=yA9kqUQkAtlAj42WoAOWA1KhJe6C4+KFuX6ypkPKj3 P0KyKjvY3pBGxXzDrhwrFLxJoMQjRE0kQDohA2xEAVLo2XFyyWN1ZsGHKGQGUuPR 9wi1OSja5r8B78MSPqLdNe7AD02HhtLvtOCnvOTevz0IXZRwMKlciRrENKvaWX5L w= Received: (qmail 46623 invoked by alias); 21 Nov 2019 18:43:34 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 46614 invoked by uid 89); 21 Nov 2019 18:43:34 -0000 Authentication-Results: sourceware.org; auth=none X-HELO: mail-lj1-f194.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Ckqb+LYpfPadyZY2ph1n3dHqKuQSBn+C5RhJx1xMHqM=; b=Q47eh1yHfGOpt8O/gMT3pA7RXKJfrQTy5YH1RziqTgRx4VGMtQUuM/zWhoOQawz0DE uaOMOCFPsDXoKvyRqNPm+O7JHYacq2BTNr7wVR7Tq1FWGDvVNTga4F2t13smEgdSeitj 6IJSzYR6C4HP/7xRukV27OZ25rInd0c6Ph4V5ugQuj9oBEEjNXPxoKanUuNC4SfFuwXV NpO97vwZXLQMhbMsaiw1BtUVsgHcj39ZG9AZBq5GuJdyKuF0AA6BGAUxzyOxcuz6rLGz nodNuC3N/CQVD47JeWBdMJbadfv3liGHJ92fSY66tLfwqXLCUg5Fz/QjoACmpSP/cicp qWMA== MIME-Version: 1.0 References: <20191110025658.3149-1-alistair.francis@wdc.com> <20191110025658.3149-2-alistair.francis@wdc.com> <4fe5fa40-ab5a-6a73-3727-cf41c600a4e2@linaro.org> In-Reply-To: <4fe5fa40-ab5a-6a73-3727-cf41c600a4e2@linaro.org> From: Alistair Francis Date: Thu, 21 Nov 2019 10:37:39 -0800 Message-ID: Subject: Re: [PATCH v2 2/2] sysdeps/clock_gettime: Use clock_gettime64 if avaliable To: Adhemerval Zanella Cc: GNU C Library Content-Type: text/plain; charset="UTF-8" On Thu, Nov 21, 2019 at 9:47 AM Adhemerval Zanella wrote: > > > > On 09/11/2019 23:56, Alistair Francis wrote: > > With the clock_gettime64 call we prefer to use vDSO. There is no call > > to clock_gettime64 on glibc with older headers and kernel 5.1+ if it > > doesn't support vDSO. > > --- > > This was patch was runtime tested with RV32 and RV64 > > It was build tested using the ./scripts/build-many-glibcs.py script. > > > > I also ran: > > $ make ; make install ; make check > > tests on native ARM (32-bit) with the following three confiugrations: > > - 4.19 Kernel and 4.19 Headers > > - 5.2 Kernel and 4.19 Headers > > - 5.2 Kernel and 5.2 Headers > > and didn't see any regressions > > > > v2: > > - Add commit message > > - Change ret_64 to int > > > > include/time.h | 3 ++ > > sysdeps/unix/sysv/linux/clock_gettime.c | 50 ++++++++++++++++++++++++- > > 2 files changed, 52 insertions(+), 1 deletion(-) > > > > diff --git a/include/time.h b/include/time.h > > index d7800eb30f8..c19c73ae09f 100644 > > --- a/include/time.h > > +++ b/include/time.h > > @@ -211,11 +211,14 @@ extern double __difftime (time_t time1, time_t time0); > > > > #if __TIMESIZE == 64 > > # define __clock_nanosleep_time64 __clock_nanosleep > > +# define __clock_gettime64 __clock_gettime > > #else > > extern int __clock_nanosleep_time64 (clockid_t clock_id, > > int flags, const struct __timespec64 *req, > > struct __timespec64 *rem); > > libc_hidden_proto (__clock_nanosleep_time64) > > +extern int __clock_gettime64 (clockid_t clock_id, struct __timespec64 *tp); > > +libc_hidden_proto (__clock_gettime64) > > #endif > > > > /* Use in the clock_* functions. Size of the field representing the > > diff --git a/sysdeps/unix/sysv/linux/clock_gettime.c b/sysdeps/unix/sysv/linux/clock_gettime.c > > index ca40983f6ca..e7af9902f3e 100644 > > --- a/sysdeps/unix/sysv/linux/clock_gettime.c > > +++ b/sysdeps/unix/sysv/linux/clock_gettime.c > > @@ -17,6 +17,7 @@ > > . */ > > > > #include > > +#include > > #include > > #include > > #include "kernel-posix-cpu-timers.h" > > It requires to define HAVE_SYSCALL if HAVE_CLOCK_GETTIME64_VSYSCALL is also > defined, otherwise architectures that define __ASSUME_TIME64_SYSCALLS and > HAVE_CLOCK_GETTIME64_VSYSCALL will not use the vdso. > > > @@ -30,10 +31,57 @@ > > > > /* Get current value of CLOCK and store it in TP. */ > > int > > +__clock_gettime64 (clockid_t clock_id, struct __timespec64 *tp) > > +{ > > +#ifdef __ASSUME_TIME64_SYSCALLS > > +# ifndef __NR_clock_gettime64 > > +# define __NR_clock_gettime64 __NR_clock_gettime > > +# define __vdso_clock_gettime64 __vdso_clock_gettime > > +# endif > > + return INLINE_VSYSCALL (clock_gettime64, 2, clock_id, tp); > > Right, so it would call either __vdso_clock_gettime/__NR_clock_gettime or > __vdso_clock_gettime64/__NR_clock_gettime64. > > > +#else > > +# if defined __NR_clock_gettime64 && defined HAVE_CLOCK_GETTIME64_VSYSCALL > > I think we can assume that if the ABI provides the time64 vDSO symbol > it also provides the syscall (__NR_clock_gettime64). So we can just > check if HAVE_CLOCK_GETTIME64_VSYSCALL is defined. Something like: > > #ifdef __ASSUME_TIME64_SYSCALLS > # ifndef __NR_clock_gettime64 > # define __NR_clock_gettime64 __NR_clock_gettime > # define __vdso_clock_gettime64 __vdso_clock_gettime > # endif > return INLINE_VSYSCALL (clock_gettime64, 2, clock_id, tp); > #else > # if defined HAVE_CLOCK_GETTIME64_VSYSCALL > int ret64 = INLINE_VSYSCALL (clock_gettime64, 2, clock_id, tp); > if (ret64 == 0 || errno != ENOSYS) > return ret64; > # endif > struct timespec tp32; > int ret = INLINE_VSYSCALL (clock_gettime, 2, clock_id, &tp32); > if (ret == 0) > *tp = valid_timespec_to_timespec64 (tp32); > return ret; > #endif Thanks for the review. I have updated it to do what you wrote above. Alistair > > > > + int ret_64; > > + ret_64 = INLINE_VSYSCALL (clock_gettime64, 2, clock_id, tp); > > + > > + if (ret_64 == 0 || errno != ENOSYS) > > + return ret_64; > > +# endif /* __NR_clock_gettime64 && HAVE_CLOCK_GETTIME64_VSYSCALL */ > > + int ret; > > + struct timespec tp32; > > + > > + ret = INLINE_VSYSCALL (clock_gettime, 2, clock_id, &tp32); > > + > > + if (ret == 0) > > + *tp = valid_timespec_to_timespec64 (tp32); > > + > > + return ret; > > +#endif /* __ASSUME_TIME64_SYSCALLS */ > > Ok. > > > +} > > + > > +#if __TIMESIZE != 64 > > +int > > __clock_gettime (clockid_t clock_id, struct timespec *tp) > > { > > - return INLINE_VSYSCALL (clock_gettime, 2, clock_id, tp); > > + int ret; > > + struct __timespec64 tp64; > > + > > + ret = __clock_gettime64 (clock_id, &tp64); > > + > > + if (ret == 0) > > + { > > + if (! in_time_t_range (tp64.tv_sec)) > > + { > > + __set_errno (EOVERFLOW); > > + return -1; > > + } > > + > > + *tp = valid_timespec64_to_timespec (tp64); > > + } > > + > > + return ret; > > } > > +#endif > > libc_hidden_def (__clock_gettime) > > > > versioned_symbol (libc, __clock_gettime, clock_gettime, GLIBC_2_17); > > Ok.