| Message ID | d570d35acb1d9027cf7558ba47bf0fb3f5272860.1563321715.git.alistair.francis@wdc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC,v3,01/23] sysdeps/nanosleep: Use clock_nanosleep_time64 if avaliable | expand |
* Alistair Francis: > diff --git a/sysdeps/unix/sysv/linux/gettimeofday.c b/sysdeps/unix/sysv/linux/gettimeofday.c > index a74f03825a..151b1e606c 100644 > --- a/sysdeps/unix/sysv/linux/gettimeofday.c > +++ b/sysdeps/unix/sysv/linux/gettimeofday.c > @@ -32,7 +32,35 @@ > int > __gettimeofday (struct timeval *tv, struct timezone *tz) > { > +#ifdef __ASSUME_TIME64_SYSCALLS > + int ret; > + struct __timespec64 now; > + > + ret = INLINE_VSYSCALL (clock_gettime64, 2, CLOCK_REALTIME, > + &now); > + > + /* Convert from timespec to timeval */ > + tv->tv_sec = now.tv_sec; > + tv->tv_usec = now.tv_nsec / 1000; > + > + return ret; > +#else > +# ifdef __NR_clock_gettime64 > + long int ret; > + struct __timespec64 now; > + > + ret = INLINE_VSYSCALL (clock_gettime64, 2, CLOCK_REALTIME, > + &now); > + > + /* Convert from timespec to timeval */ > + tv->tv_sec = now.tv_sec; > + tv->tv_usec = now.tv_nsec / 1000; > + > + if (ret == 0 || errno != ENOSYS) > + return ret; > +# endif > return INLINE_VSYSCALL (gettimeofday, 2, tv, tz); > +#endif > } This loses vDSO acceleration if glibc is compiled with kernel headers which define __NR_clock_gettime64, but the run-time kernel does not have clock_gettime64 in the vDSO. And the kernel folks really want us to call clock_gettime when the user calls the 32-bit function, for tracability of legacy processes. Thanks, Florian
Hi Alistair, > Not all architectures support the obsolete gettimeofday so use the > newer clock_gettime64 syscall if it is avaliable. This fixes RV32 > build issues. > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > --- > ChangeLog | 1 + > sysdeps/unix/sysv/linux/gettimeofday.c | 28 > ++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) > > diff --git a/ChangeLog b/ChangeLog > index 477b9b49b3..9ca390a9c3 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1028,6 +1028,7 @@ > * nptl/thrd_sleep.c: Use clock_nanosleep_time64 instead of > nanosleep. > * sysdeps/unix/sysv/linux/nanosleep.c: Likewise. > * sysdeps/unix/sysv/linux/nanosleep_nocancel.c: Likewise. > + * sysdeps/unix/sysv/linux/gettimeofday.c: Use > clock_gettime64 syscall for gettimeofday. > 2019-06-20 Dmitry V. Levin <ldv@altlinux.org> > Florian Weimer <fweimer@redhat.com> > diff --git a/sysdeps/unix/sysv/linux/gettimeofday.c > b/sysdeps/unix/sysv/linux/gettimeofday.c index a74f03825a..151b1e606c > 100644 --- a/sysdeps/unix/sysv/linux/gettimeofday.c > +++ b/sysdeps/unix/sysv/linux/gettimeofday.c > @@ -32,7 +32,35 @@ > int > __gettimeofday (struct timeval *tv, struct timezone *tz) > { > +#ifdef __ASSUME_TIME64_SYSCALLS I'm not the glibc expert but according to [1], the __ASSUME_TIME64_SYSCALLS will be defined also for __WORDSIZE = 64 archs. This means that this code will be executed on x86_64 and return with an ENOTSUPP error as those archs shall not define clock_gettime64 and will just use the clock_settime. Please consider re-using pattern from clock_settime conversion [2]. > + int ret; > + struct __timespec64 now; > + > + ret = INLINE_VSYSCALL (clock_gettime64, 2, CLOCK_REALTIME, > + &now); > + > + /* Convert from timespec to timeval */ > + tv->tv_sec = now.tv_sec; > + tv->tv_usec = now.tv_nsec / 1000; > + > + return ret; > +#else > +# ifdef __NR_clock_gettime64 > + long int ret; > + struct __timespec64 now; > + > + ret = INLINE_VSYSCALL (clock_gettime64, 2, CLOCK_REALTIME, > + &now); > + > + /* Convert from timespec to timeval */ > + tv->tv_sec = now.tv_sec; > + tv->tv_usec = now.tv_nsec / 1000; > + > + if (ret == 0 || errno != ENOSYS) > + return ret; > +# endif > return INLINE_VSYSCALL (gettimeofday, 2, tv, tz); > +#endif > } > libc_hidden_def (__gettimeofday) > weak_alias (__gettimeofday, gettimeofday) Note: [1] - https://github.com/lmajewski/y2038_glibc/commit/1fdbc6002101a78a8a6a076bbb642b3082c2225d [2] - https://github.com/lmajewski/y2038_glibc/commit/69f842a8519ca13ed11fab0ff1bcc6fa1a524192 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
Hi, > Hi Alistair, > > > Not all architectures support the obsolete gettimeofday so use the > > newer clock_gettime64 syscall if it is avaliable. This fixes RV32 > > build issues. > > > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > > --- > > ChangeLog | 1 + > > sysdeps/unix/sysv/linux/gettimeofday.c | 28 > > ++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) > > > > diff --git a/ChangeLog b/ChangeLog > > index 477b9b49b3..9ca390a9c3 100644 > > --- a/ChangeLog > > +++ b/ChangeLog > > @@ -1028,6 +1028,7 @@ > > * nptl/thrd_sleep.c: Use clock_nanosleep_time64 instead of > > nanosleep. > > * sysdeps/unix/sysv/linux/nanosleep.c: Likewise. > > * sysdeps/unix/sysv/linux/nanosleep_nocancel.c: Likewise. > > + * sysdeps/unix/sysv/linux/gettimeofday.c: Use > > clock_gettime64 syscall for gettimeofday. > > 2019-06-20 Dmitry V. Levin <ldv@altlinux.org> > > Florian Weimer <fweimer@redhat.com> > > diff --git a/sysdeps/unix/sysv/linux/gettimeofday.c > > b/sysdeps/unix/sysv/linux/gettimeofday.c index > > a74f03825a..151b1e606c 100644 --- > > a/sysdeps/unix/sysv/linux/gettimeofday.c +++ > > b/sysdeps/unix/sysv/linux/gettimeofday.c @@ -32,7 +32,35 @@ > > int > > __gettimeofday (struct timeval *tv, struct timezone *tz) > > { > > +#ifdef __ASSUME_TIME64_SYSCALLS > > I'm not the glibc expert but according to [1], the > __ASSUME_TIME64_SYSCALLS will be defined also for __WORDSIZE = 64 > archs. > > This means that this code will be executed on x86_64 and return with > an ENOTSUPP error as those archs shall not define clock_gettime64 and ^^^^^^^ - sorry s/ENOTSUPP/ENOSYS/g > will just use the clock_settime. > > Please consider re-using pattern from clock_settime conversion [2]. > > > + int ret; > > + struct __timespec64 now; > > + > > + ret = INLINE_VSYSCALL (clock_gettime64, 2, CLOCK_REALTIME, > > + &now); > > + > > + /* Convert from timespec to timeval */ > > + tv->tv_sec = now.tv_sec; > > + tv->tv_usec = now.tv_nsec / 1000; > > + > > + return ret; > > +#else > > +# ifdef __NR_clock_gettime64 > > + long int ret; > > + struct __timespec64 now; > > + > > + ret = INLINE_VSYSCALL (clock_gettime64, 2, CLOCK_REALTIME, > > + &now); > > + > > + /* Convert from timespec to timeval */ > > + tv->tv_sec = now.tv_sec; > > + tv->tv_usec = now.tv_nsec / 1000; > > + > > + if (ret == 0 || errno != ENOSYS) > > + return ret; > > +# endif > > return INLINE_VSYSCALL (gettimeofday, 2, tv, tz); > > +#endif > > } > > libc_hidden_def (__gettimeofday) > > weak_alias (__gettimeofday, gettimeofday) > > Note: > > [1] - > https://github.com/lmajewski/y2038_glibc/commit/1fdbc6002101a78a8a6a076bbb642b3082c2225d > > [2] - > https://github.com/lmajewski/y2038_glibc/commit/69f842a8519ca13ed11fab0ff1bcc6fa1a524192 > > 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 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
On Wed, Jul 17, 2019 at 5:43 AM Lukasz Majewski <lukma@denx.de> wrote: > > Hi Alistair, > > > Not all architectures support the obsolete gettimeofday so use the > > newer clock_gettime64 syscall if it is avaliable. This fixes RV32 > > build issues. > > > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > > --- > > ChangeLog | 1 + > > sysdeps/unix/sysv/linux/gettimeofday.c | 28 > > ++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) > > > > diff --git a/ChangeLog b/ChangeLog > > index 477b9b49b3..9ca390a9c3 100644 > > --- a/ChangeLog > > +++ b/ChangeLog > > @@ -1028,6 +1028,7 @@ > > * nptl/thrd_sleep.c: Use clock_nanosleep_time64 instead of > > nanosleep. > > * sysdeps/unix/sysv/linux/nanosleep.c: Likewise. > > * sysdeps/unix/sysv/linux/nanosleep_nocancel.c: Likewise. > > + * sysdeps/unix/sysv/linux/gettimeofday.c: Use > > clock_gettime64 syscall for gettimeofday. > > 2019-06-20 Dmitry V. Levin <ldv@altlinux.org> > > Florian Weimer <fweimer@redhat.com> > > diff --git a/sysdeps/unix/sysv/linux/gettimeofday.c > > b/sysdeps/unix/sysv/linux/gettimeofday.c index a74f03825a..151b1e606c > > 100644 --- a/sysdeps/unix/sysv/linux/gettimeofday.c > > +++ b/sysdeps/unix/sysv/linux/gettimeofday.c > > @@ -32,7 +32,35 @@ > > int > > __gettimeofday (struct timeval *tv, struct timezone *tz) > > { > > +#ifdef __ASSUME_TIME64_SYSCALLS > > I'm not the glibc expert but according to [1], the > __ASSUME_TIME64_SYSCALLS will be defined also for __WORDSIZE = 64 archs. > > This means that this code will be executed on x86_64 and return with an > ENOTSUPP error as those archs shall not define clock_gettime64 and > will just use the clock_settime. > > Please consider re-using pattern from clock_settime conversion [2]. Yep, good point. I have updated this to better match what is being discussed on the list and your patches. Alistair > > > + int ret; > > + struct __timespec64 now; > > + > > + ret = INLINE_VSYSCALL (clock_gettime64, 2, CLOCK_REALTIME, > > + &now); > > + > > + /* Convert from timespec to timeval */ > > + tv->tv_sec = now.tv_sec; > > + tv->tv_usec = now.tv_nsec / 1000; > > + > > + return ret; > > +#else > > +# ifdef __NR_clock_gettime64 > > + long int ret; > > + struct __timespec64 now; > > + > > + ret = INLINE_VSYSCALL (clock_gettime64, 2, CLOCK_REALTIME, > > + &now); > > + > > + /* Convert from timespec to timeval */ > > + tv->tv_sec = now.tv_sec; > > + tv->tv_usec = now.tv_nsec / 1000; > > + > > + if (ret == 0 || errno != ENOSYS) > > + return ret; > > +# endif > > return INLINE_VSYSCALL (gettimeofday, 2, tv, tz); > > +#endif > > } > > libc_hidden_def (__gettimeofday) > > weak_alias (__gettimeofday, gettimeofday) > > Note: > > [1] - > https://github.com/lmajewski/y2038_glibc/commit/1fdbc6002101a78a8a6a076bbb642b3082c2225d > > [2] - > https://github.com/lmajewski/y2038_glibc/commit/69f842a8519ca13ed11fab0ff1bcc6fa1a524192 > > 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
On Wed, Jul 17, 2019 at 09:09:08AM +0200, Florian Weimer wrote: > * Alistair Francis: > > > diff --git a/sysdeps/unix/sysv/linux/gettimeofday.c b/sysdeps/unix/sysv/linux/gettimeofday.c > > index a74f03825a..151b1e606c 100644 > > --- a/sysdeps/unix/sysv/linux/gettimeofday.c > > +++ b/sysdeps/unix/sysv/linux/gettimeofday.c > > @@ -32,7 +32,35 @@ > > int > > __gettimeofday (struct timeval *tv, struct timezone *tz) > > { > > +#ifdef __ASSUME_TIME64_SYSCALLS > > + int ret; > > + struct __timespec64 now; > > + > > + ret = INLINE_VSYSCALL (clock_gettime64, 2, CLOCK_REALTIME, > > + &now); > > + > > + /* Convert from timespec to timeval */ > > + tv->tv_sec = now.tv_sec; > > + tv->tv_usec = now.tv_nsec / 1000; > > + > > + return ret; > > +#else > > +# ifdef __NR_clock_gettime64 > > + long int ret; > > + struct __timespec64 now; > > + > > + ret = INLINE_VSYSCALL (clock_gettime64, 2, CLOCK_REALTIME, > > + &now); > > + > > + /* Convert from timespec to timeval */ > > + tv->tv_sec = now.tv_sec; > > + tv->tv_usec = now.tv_nsec / 1000; > > + > > + if (ret == 0 || errno != ENOSYS) > > + return ret; > > +# endif > > return INLINE_VSYSCALL (gettimeofday, 2, tv, tz); > > +#endif > > } > > This loses vDSO acceleration if glibc is compiled with kernel headers > which define __NR_clock_gettime64, but the run-time kernel does not have > clock_gettime64 in the vDSO. > > And the kernel folks really want us to call clock_gettime when the user > calls the 32-bit function, for tracability of legacy processes. May or may not be relevant to glibc plans, but I want to chime in to mention that this (kernel tracability of legacy processes) is not going to work with musl, except possibly static-linked programs with old libc. Our legacy compat functions are just going to be thin, implementation-agnostic wrappers around the new functions, so even if a program is still calling the old clock_gettime symbol with 32-bit timespec, it's going to be executing the new one as its backend, and thereby calling the 64-bit syscall or vdso if available. In the case of new libc with old application on old kernel, there will actually be two conversions: 32->64 in the core (new) function after the 64-bit syscall fails with ENOSYS and we fallback to the old syscall, and 64->32 in the old-ABI-compat wrapper. This is a consequence of not duplicating functionality and ending up with two versions of everything to maintain, and also avoids incentivizing continued use of the old ABI for users with old kernels (as a way to bypass conversion costs). In any case, just grepping dynsym tables for references to legacy symbols seems like a more effective way of finding problems than grepping for syslog spam... Rich
On Fri, 19 Jul 2019, Rich Felker wrote: > > And the kernel folks really want us to call clock_gettime when the user > > calls the 32-bit function, for tracability of legacy processes. > > May or may not be relevant to glibc plans, but I want to chime in to > mention that this (kernel tracability of legacy processes) is not > going to work with musl, except possibly static-linked programs with > old libc. Our legacy compat functions are just going to be thin, > implementation-agnostic wrappers around the new functions, so even if > a program is still calling the old clock_gettime symbol with 32-bit > timespec, it's going to be executing the new one as its backend, and > thereby calling the 64-bit syscall or vdso if available. And as far as I'm concerned glibc should do likewise (I have a sustained objection to duplication of nontrivial function implementations at either the source and binary level for time_t variants when thin wrappers would do the job just as well, given the principle that the particular choice of syscalls for a given function is not a stable interface). (Where a function is defined entirely in syscalls.list, it might continue to use the old syscall, subject to working out the best way to set up function aliasing for that case.) > In any case, just grepping dynsym tables for references to legacy > symbols seems like a more effective way of finding problems than > grepping for syslog spam... Yes.
diff --git a/ChangeLog b/ChangeLog index 477b9b49b3..9ca390a9c3 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1028,6 +1028,7 @@ * nptl/thrd_sleep.c: Use clock_nanosleep_time64 instead of nanosleep. * sysdeps/unix/sysv/linux/nanosleep.c: Likewise. * sysdeps/unix/sysv/linux/nanosleep_nocancel.c: Likewise. + * sysdeps/unix/sysv/linux/gettimeofday.c: Use clock_gettime64 syscall for gettimeofday. 2019-06-20 Dmitry V. Levin <ldv@altlinux.org> Florian Weimer <fweimer@redhat.com> diff --git a/sysdeps/unix/sysv/linux/gettimeofday.c b/sysdeps/unix/sysv/linux/gettimeofday.c index a74f03825a..151b1e606c 100644 --- a/sysdeps/unix/sysv/linux/gettimeofday.c +++ b/sysdeps/unix/sysv/linux/gettimeofday.c @@ -32,7 +32,35 @@ int __gettimeofday (struct timeval *tv, struct timezone *tz) { +#ifdef __ASSUME_TIME64_SYSCALLS + int ret; + struct __timespec64 now; + + ret = INLINE_VSYSCALL (clock_gettime64, 2, CLOCK_REALTIME, + &now); + + /* Convert from timespec to timeval */ + tv->tv_sec = now.tv_sec; + tv->tv_usec = now.tv_nsec / 1000; + + return ret; +#else +# ifdef __NR_clock_gettime64 + long int ret; + struct __timespec64 now; + + ret = INLINE_VSYSCALL (clock_gettime64, 2, CLOCK_REALTIME, + &now); + + /* Convert from timespec to timeval */ + tv->tv_sec = now.tv_sec; + tv->tv_usec = now.tv_nsec / 1000; + + if (ret == 0 || errno != ENOSYS) + return ret; +# endif return INLINE_VSYSCALL (gettimeofday, 2, tv, tz); +#endif } libc_hidden_def (__gettimeofday) weak_alias (__gettimeofday, gettimeofday)
Not all architectures support the obsolete gettimeofday so use the newer clock_gettime64 syscall if it is avaliable. This fixes RV32 build issues. Signed-off-by: Alistair Francis <alistair.francis@wdc.com> --- ChangeLog | 1 + sysdeps/unix/sysv/linux/gettimeofday.c | 28 ++++++++++++++++++++++++++ 2 files changed, 29 insertions(+)