| Message ID | 20191110025658.3149-2-alistair.francis@wdc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/2] sysdeps: Add clock_gettime64 vDSO | expand |
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 @@ > <https://www.gnu.org/licenses/>. */ > > #include <sysdep.h> > +#include <kernel-features.h> > #include <errno.h> > #include <time.h> > #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 > + 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.
On Thu, Nov 21, 2019 at 9:47 AM Adhemerval Zanella <adhemerval.zanella@linaro.org> 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 @@ > > <https://www.gnu.org/licenses/>. */ > > > > #include <sysdep.h> > > +#include <kernel-features.h> > > #include <errno.h> > > #include <time.h> > > #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.
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 @@ <https://www.gnu.org/licenses/>. */ #include <sysdep.h> +#include <kernel-features.h> #include <errno.h> #include <time.h> #include "kernel-posix-cpu-timers.h" @@ -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); +#else +# if defined __NR_clock_gettime64 && defined HAVE_CLOCK_GETTIME64_VSYSCALL + 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 */ +} + +#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);