From patchwork Thu Dec 5 10:48:45 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lukasz Majewski X-Patchwork-Id: 36528 Received: (qmail 24941 invoked by alias); 5 Dec 2019 10:49:01 -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 Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 24927 invoked by uid 89); 5 Dec 2019 10:49:00 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-20.6 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 spammy=2019-12 X-HELO: mail-out.m-online.net Date: Thu, 5 Dec 2019 11:48:45 +0100 From: Lukasz Majewski To: Alistair Francis Cc: Adhemerval Zanella , GNU C Library , Alistair Francis , arnd@arndb.de Subject: Re: [PATCH v3 2/2] sysdeps/clock_gettime: Use clock_gettime64 if avaliable Message-ID: <20191205114845.54c1cc6f@jawa> In-Reply-To: References: <20191123222708.17021-1-alistair.francis@wdc.com> <20191123222708.17021-2-alistair.francis@wdc.com> <621b67c9-028e-e913-8b9a-7978d2d05e04@linaro.org> <51162415-4919-0905-2aa3-4b5d33912f75@linaro.org> MIME-Version: 1.0 Hi Alistair, > On Wed, Dec 4, 2019 at 9:24 AM Adhemerval Zanella > wrote: > > > > > > > > On 03/12/2019 16:29, Adhemerval Zanella wrote: > > > > > > > > > On 23/11/2019 19:27, 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. > > > > > > Ok with the fix below regarding HAVE_CLOCK_GETTIME64_VSYSCALL. > > > > As [1] I think we can handle both the cases: > > > > 8. Define both __NR_clock_gettime64, provide a 64-bit vDSO, > > define __NR_clock_gettime, and provide a 32-bit vDSO. > > - i.e: some 32-bits ports built against kernel header equal > > or newer then v5.1 (i386, arm, and mips). > > > > 9. Define both __NR_clock_gettime64, provide a 64-bit vDSO, and > > define __NR_clock_gettime (unlikely, but possible).. > > > > With my vDSO refactor to use relro data structures. > > > > This patch is ok. > > > > Reviewed-by: Adhemerval Zanella > > Thanks :) > > I'll apply these two patches later today unless there are any other > comments. Thank you for preparing and applying this patch. Just informative note (for the record). For ARM 32 bit systems - after checking with [1] it has been apparent that those systems doesn't yet support __vdso_clock_gettime64 [2]. If anybody would need to build glibc with headers from Linux 5.1+ (with clock_gettime64 support), he/she shall apply attached patch. This is not a problem for now, as we still have some time until __ASSUME_TIME64_SYSCALLS is enabled by default in glibc. Moreover, I do guess that __vdso_clock_gettime64 for ARM 32 bits will be available by then :-). Links: [1] - https://github.com/lmajewski/meta-y2038 [2] - https://elixir.bootlin.com/linux/latest/ident/__vdso_clock_gettime64 > > Alistair > > > > > [1] https://sourceware.org/ml/libc-alpha/2019-12/msg00142.html > > > > > > > >> --- > > >> 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 > > >> > > >> v3: > > >> - Assume HAVE_CLOCK_GETTIME64_VSYSCALL means we have > > >> __NR_clock_gettime64 > > >> - Change code style to be more glibc-ish > > >> v2: > > >> - Add commit message > > >> - Change ret_64 to int > > >> > > >> include/time.h | 3 ++ > > >> sysdeps/unix/sysv/linux/clock_gettime.c | 44 > > >> ++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 1 > > >> deletion(-) > > >> > > >> diff --git a/include/time.h b/include/time.h > > >> index d7800eb30f..c19c73ae09 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 > > >> ca40983f6c..875c4fe905 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" > > >> @@ -30,10 +31,51 @@ > > >> > > >> /* 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); > > > > > > Ok, in this case either __vdso_clock_gettime/__NR_clock_gettime or > > > __vdso_clock_gettime64/__NR_clock_gettime64 will be called. > > > > > >> +#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 > > > This should be: > > > > > > #if defined __NR_clock_gettime64 > > > int ret64 = INLINE_VSYSCALL (clock_gettime64, ...) > > > [...] > > > > > > And before sysdep-vdso.h it should be. > > > > > > #if defined HAVE_CLOCK_GETTIME_VSYSCALL || defined > > > HAVE_CLOCK_GETTIME64_VSYSCALL # define HAVE_SYSCALL > > > #endif > > > > > > If !HAVE_SYSCALL the INLINE_VSYSCALL is defined as INLINE_SYCALL. > > > It means that if an architecture does provide __NR_clock_gettime64 > > > but not provide HAVE_CLOCK_GETTIME64_VSYSCALL, it will still use > > > the syscall. This code only uses the syscall if the vDSO is still > > > present. > > > > > > I hope I can get my vDSO refactor, which would simplify a bit this > > > code. > > > > > >> + 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 > > >> +} > > > > > > Ok, old fallback to time32 vdso/syscall. > > > > > >> + > > >> +#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. > > > Best regards, Lukasz Majewski --- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de From d48860f4f8a8d75e689ee7f91214698598fbbcb8 Mon Sep 17 00:00:00 2001 From: Lukasz Majewski Date: Thu, 5 Dec 2019 11:12:48 +0100 Subject: [PATCH] Y2038: clock_gettime: Use non vDSO syscall for clock_gettime64 on 32 bit ARM Up till Linux v5.4 [*], there is no 64 bit version of __vdso_clock_gettime64() available for 32 bit ARM. For that reason one shall use non VDSO syscall for this purpose. Link: [*] - https://elixir.bootlin.com/linux/latest/ident/__vdso_clock_gettime64 --- sysdeps/unix/sysv/linux/clock_gettime.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sysdeps/unix/sysv/linux/clock_gettime.c b/sysdeps/unix/sysv/linux/clock_gettime.c index 875c4fe905..a717d8d88a 100644 --- a/sysdeps/unix/sysv/linux/clock_gettime.c +++ b/sysdeps/unix/sysv/linux/clock_gettime.c @@ -38,7 +38,11 @@ __clock_gettime64 (clockid_t clock_id, struct __timespec64 *tp) # define __NR_clock_gettime64 __NR_clock_gettime # define __vdso_clock_gettime64 __vdso_clock_gettime # endif +# ifdef HAVE_CLOCK_GETTIME64_VSYSCALL return INLINE_VSYSCALL (clock_gettime64, 2, clock_id, tp); +# else + return INLINE_SYSCALL (clock_gettime64, 2, clock_id, tp); +# endif #else # if defined HAVE_CLOCK_GETTIME64_VSYSCALL int ret64 = INLINE_VSYSCALL (clock_gettime64, 2, clock_id, tp); -- 2.20.1