Message ID | 20200212084525.7046-1-lukma@denx.de |
---|---|
State | New |
Headers | show |
Series | [v6] y2038: linux: Provide __gettimeofday64 implementation | expand |
On 12/02/2020 05:45, Lukasz Majewski wrote: > In the glibc the gettimeofday can use vDSO (on power and x86 the > USE_IFUNC_GETTIMEOFDAY is defined), gettimeofday syscall or 'default' > ___gettimeofday() from ./time/gettime.c (as a fallback). > > In this patch the last function (___gettimeofday) has been refactored and > moved to ./sysdeps/unix/sysv/linux/gettimeofday.c to be Linux specific. In fact the function implementation is replicated on Linux version. Once this patch is upstream I plan to make a generic implementation and refactor alpha, Linux, and generic implementation to avoid code duplication. > > The new __gettimeofday64 explicit 64 bit function for getting 64 bit time from > the kernel (by internally calling __clock_gettime64) has been introduced. > > Moreover, a 32 bit version - __gettimeofday has been refactored to internally > use __gettimeofday64. > > The __gettimeofday is now supposed to be used on systems still supporting 32 > bit time (__TIMESIZE != 64) - hence the necessary check for time_t potential > overflow and conversion of struct __timeval64 to 32 bit struct timespec. > > The alpha port is a bit problematic for this change - it supports 64 bit time > and can safely use gettimeofday implementation from ./time/gettimeofday.c as it > has ./sysdeps/unix/sysv/linux/alpha/gettimeofday.c, which includes > ./time/gettimeofday.c, so the Linux specific one can be avoided. > For that reason the code to set default gettimeofday symbol has not been moved > to ./sysdeps/unix/sysv/linux/gettimeofday.c as only alpha defines > VERSION_gettimeofday. You can remove this part about alpha from commit message, this patch does not change anything about alpha behaviour. > > The USE_IFUNC_GETTIMEOFDAY for powerpc and i686 has been undefined on > purpose. Due to that the support for gettimeofday vDSO on them has been > traded for Y2038 safeness (as this syscall is going to be obsolete). I think it would be useful to extend a bit why this optimization was removed from i686 and powerpc32. Maybe add: The iFUNC vDSO direct call optimization has been removed from both i686 and powerpc32 (USE_IFUNC_GETTIMEOFDAY is not defined for the architectures anymore). The Linux kernel does not provide a y2038 safe implementation of gettimeofday neither it plans to provide it in the future, clock_gettime64 should be use instead. It meant to keep support for this optimization it would require to handle another build permutation (!__ASSUME_TIME64_SYSCALLS && USE_IFUNC_GETTIMEOFDAY) with adds more complexity and has limited use (since the idea is to eventually have a y2038 safe glibc build). > > Build tests: > ./src/scripts/build-many-glibcs.py glibcs > > Run-time tests: > - Run specific tests on ARM/x86 32bit systems (qemu): > https://github.com/lmajewski/meta-y2038 and run tests: > https://github.com/lmajewski/y2038-tests/commits/master > > Above tests were performed with Y2038 redirection applied as well as without > to test proper usage of both __gettimeofday64 and __gettimeofday. LGTM with the suggested commit message changes. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > > --- > Changes for v6: > - Do not check if tv pointer is NULL as in generic implementation of > ./time/gettimeofday.c. Moreover, prototype for this function has > __nonnull GCC attribute for this parameter. > > Changes for v5: > - Replace '___gettimeofday64' with '__gettimeofday64' in the subject of the > patch > > Changes for v4: > - Rename ___gettimeofday{64} to __gettimeofday{64} as '___' prefix is not > needed for our implementation > - Correctly handle the case when tv is NULL (also in __gettimeofday). > - Do not define USE_IFUNC_GETTIMEOFDAY for 32 bit archs - namely powerpc and > i686. > > Changes for v3: > - New patch > --- > include/time.h | 4 ++ > sysdeps/unix/sysv/linux/gettimeofday.c | 40 ++++++++++++++++++- > .../unix/sysv/linux/powerpc/gettimeofday.c | 4 +- > sysdeps/unix/sysv/linux/x86/gettimeofday.c | 4 +- > 4 files changed, 49 insertions(+), 3 deletions(-) > > diff --git a/include/time.h b/include/time.h > index 73f66277ac..61806658e7 100644 > --- a/include/time.h > +++ b/include/time.h > @@ -227,10 +227,14 @@ libc_hidden_proto (__sched_rr_get_interval64); > > #if __TIMESIZE == 64 > # define __settimeofday64 __settimeofday > +# define __gettimeofday64 __gettimeofday > #else > extern int __settimeofday64 (const struct __timeval64 *tv, > const struct timezone *tz); > libc_hidden_proto (__settimeofday64) > +extern int __gettimeofday64 (struct __timeval64 *restrict tv, > + void *restrict tz); > +libc_hidden_proto (__gettimeofday64) > #endif > > /* Compute the `struct tm' representation of T, Ok. > diff --git a/sysdeps/unix/sysv/linux/gettimeofday.c b/sysdeps/unix/sysv/linux/gettimeofday.c > index d5cdb22495..cb57bc9cf2 100644 > --- a/sysdeps/unix/sysv/linux/gettimeofday.c > +++ b/sysdeps/unix/sysv/linux/gettimeofday.c > @@ -54,5 +54,43 @@ __gettimeofday (struct timeval *restrict tv, void *restrict tz) > # endif > weak_alias (__gettimeofday, gettimeofday) > #else /* USE_IFUNC_GETTIMEOFDAY */ > -# include <time/gettimeofday.c> > +/* Conversion of gettimeofday function to support 64 bit time on archs > + with __WORDSIZE == 32 and __TIMESIZE == 32/64 */ > +#include <errno.h> > + > +int > +__gettimeofday64 (struct __timeval64 *restrict tv, void *restrict tz) > +{ > + if (__glibc_unlikely (tz != 0)) > + memset (tz, 0, sizeof (struct timezone)); > + > + struct __timespec64 ts64; > + if (__clock_gettime64 (CLOCK_REALTIME, &ts64)) > + return -1; > + > + *tv = timespec64_to_timeval64 (ts64); > + return 0; > +} > + > +# if __TIMESIZE != 64 > +libc_hidden_def (__gettimeofday64) > + > +int > +__gettimeofday (struct timeval *restrict tv, void *restrict tz) > +{ > + struct __timeval64 tv64; > + if (__gettimeofday64 (&tv64, tz)) > + return -1; > + > + if (! in_time_t_range (tv64.tv_sec)) > + { > + __set_errno (EOVERFLOW); > + return -1; > + } > + > + *tv = valid_timeval64_to_timeval (tv64); > + return 0; > +} > +# endif > +weak_alias (__gettimeofday, gettimeofday) > #endif Ok. > diff --git a/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c b/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c > index 183fb0ac70..2d6978f333 100644 > --- a/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c > +++ b/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c > @@ -15,5 +15,7 @@ > License along with the GNU C Library; if not, see > <https://www.gnu.org/licenses/>. */ > > -#define USE_IFUNC_GETTIMEOFDAY > +#ifdef __powerpc64__ > +# define USE_IFUNC_GETTIMEOFDAY > +#endif > #include <sysdeps/unix/sysv/linux/gettimeofday.c> Ok. > diff --git a/sysdeps/unix/sysv/linux/x86/gettimeofday.c b/sysdeps/unix/sysv/linux/x86/gettimeofday.c > index 1b7aa880a2..0c1779dc83 100644 > --- a/sysdeps/unix/sysv/linux/x86/gettimeofday.c > +++ b/sysdeps/unix/sysv/linux/x86/gettimeofday.c > @@ -16,5 +16,7 @@ > License along with the GNU C Library; if not, see > <https://www.gnu.org/licenses/>. */ > > -#define USE_IFUNC_GETTIMEOFDAY > +#ifdef __x86_64__ > +# define USE_IFUNC_GETTIMEOFDAY > +#endif > #include <sysdeps/unix/sysv/linux/gettimeofday.c> > Ok.
Hi Adhemerval, Thank you for the review. > On 12/02/2020 05:45, Lukasz Majewski wrote: > > In the glibc the gettimeofday can use vDSO (on power and x86 the > > USE_IFUNC_GETTIMEOFDAY is defined), gettimeofday syscall or > > 'default' ___gettimeofday() from ./time/gettime.c (as a fallback). > > > > In this patch the last function (___gettimeofday) has been > > refactored and moved to ./sysdeps/unix/sysv/linux/gettimeofday.c to > > be Linux specific. > > In fact the function implementation is replicated on Linux version. > Once this patch is upstream I plan to make a generic implementation > and refactor alpha, Linux, and generic implementation to avoid code > duplication. Ok. I shall apply this patch till the end of the day. > > > > > The new __gettimeofday64 explicit 64 bit function for getting 64 > > bit time from the kernel (by internally calling __clock_gettime64) > > has been introduced. > > > > Moreover, a 32 bit version - __gettimeofday has been refactored to > > internally use __gettimeofday64. > > > > The __gettimeofday is now supposed to be used on systems still > > supporting 32 bit time (__TIMESIZE != 64) - hence the necessary > > check for time_t potential overflow and conversion of struct > > __timeval64 to 32 bit struct timespec. > > > > The alpha port is a bit problematic for this change - it supports > > 64 bit time and can safely use gettimeofday implementation from > > ./time/gettimeofday.c as it has > > ./sysdeps/unix/sysv/linux/alpha/gettimeofday.c, which includes > > ./time/gettimeofday.c, so the Linux specific one can be avoided. > > For that reason the code to set default gettimeofday symbol has not > > been moved to ./sysdeps/unix/sysv/linux/gettimeofday.c as only > > alpha defines VERSION_gettimeofday. > > You can remove this part about alpha from commit message, this > patch does not change anything about alpha behaviour. Ok. > > > > > The USE_IFUNC_GETTIMEOFDAY for powerpc and i686 has been undefined > > on purpose. Due to that the support for gettimeofday vDSO on them > > has been traded for Y2038 safeness (as this syscall is going to be > > obsolete). > > I think it would be useful to extend a bit why this optimization was > removed from i686 and powerpc32. Maybe add: > > The iFUNC vDSO direct call optimization has been removed from both > i686 and powerpc32 (USE_IFUNC_GETTIMEOFDAY is not defined for the > architectures anymore). The Linux kernel does not provide a y2038 > safe implementation of gettimeofday neither it plans to provide it > in the future, clock_gettime64 should be use instead. It meant to > keep support for this optimization it would require to handle > another build permutation (!__ASSUME_TIME64_SYSCALLS && > USE_IFUNC_GETTIMEOFDAY) with adds more complexity and has limited use > (since the idea is to eventually have a y2038 safe glibc build). > Ok. I will add this description. > > > > Build tests: > > ./src/scripts/build-many-glibcs.py glibcs > > > > Run-time tests: > > - Run specific tests on ARM/x86 32bit systems (qemu): > > https://github.com/lmajewski/meta-y2038 and run tests: > > https://github.com/lmajewski/y2038-tests/commits/master > > > > Above tests were performed with Y2038 redirection applied as well > > as without to test proper usage of both __gettimeofday64 and > > __gettimeofday. > > LGTM with the suggested commit message changes. > > Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> Thanks. Could you also review following patch set: https://patchwork.ozlabs.org/cover/1234918/ (It seems to be straightforward and has been posted to the mailing list some time ago). Thanks in advance. > > > > > --- > > Changes for v6: > > - Do not check if tv pointer is NULL as in generic implementation of > > ./time/gettimeofday.c. Moreover, prototype for this function has > > __nonnull GCC attribute for this parameter. > > > > Changes for v5: > > - Replace '___gettimeofday64' with '__gettimeofday64' in the > > subject of the patch > > > > Changes for v4: > > - Rename ___gettimeofday{64} to __gettimeofday{64} as '___' prefix > > is not needed for our implementation > > - Correctly handle the case when tv is NULL (also in > > __gettimeofday). > > - Do not define USE_IFUNC_GETTIMEOFDAY for 32 bit archs - namely > > powerpc and i686. > > > > Changes for v3: > > - New patch > > --- > > include/time.h | 4 ++ > > sysdeps/unix/sysv/linux/gettimeofday.c | 40 > > ++++++++++++++++++- .../unix/sysv/linux/powerpc/gettimeofday.c | > > 4 +- sysdeps/unix/sysv/linux/x86/gettimeofday.c | 4 +- > > 4 files changed, 49 insertions(+), 3 deletions(-) > > > > diff --git a/include/time.h b/include/time.h > > index 73f66277ac..61806658e7 100644 > > --- a/include/time.h > > +++ b/include/time.h > > @@ -227,10 +227,14 @@ libc_hidden_proto (__sched_rr_get_interval64); > > > > #if __TIMESIZE == 64 > > # define __settimeofday64 __settimeofday > > +# define __gettimeofday64 __gettimeofday > > #else > > extern int __settimeofday64 (const struct __timeval64 *tv, > > const struct timezone *tz); > > libc_hidden_proto (__settimeofday64) > > +extern int __gettimeofday64 (struct __timeval64 *restrict tv, > > + void *restrict tz); > > +libc_hidden_proto (__gettimeofday64) > > #endif > > > > /* Compute the `struct tm' representation of T, > > Ok. > > > diff --git a/sysdeps/unix/sysv/linux/gettimeofday.c > > b/sysdeps/unix/sysv/linux/gettimeofday.c index > > d5cdb22495..cb57bc9cf2 100644 --- > > a/sysdeps/unix/sysv/linux/gettimeofday.c +++ > > b/sysdeps/unix/sysv/linux/gettimeofday.c @@ -54,5 +54,43 @@ > > __gettimeofday (struct timeval *restrict tv, void *restrict tz) # > > endif weak_alias (__gettimeofday, gettimeofday) > > #else /* USE_IFUNC_GETTIMEOFDAY */ > > -# include <time/gettimeofday.c> > > +/* Conversion of gettimeofday function to support 64 bit time on > > archs > > + with __WORDSIZE == 32 and __TIMESIZE == 32/64 */ > > +#include <errno.h> > > + > > +int > > +__gettimeofday64 (struct __timeval64 *restrict tv, void *restrict > > tz) +{ > > + if (__glibc_unlikely (tz != 0)) > > + memset (tz, 0, sizeof (struct timezone)); > > + > > + struct __timespec64 ts64; > > + if (__clock_gettime64 (CLOCK_REALTIME, &ts64)) > > + return -1; > > + > > + *tv = timespec64_to_timeval64 (ts64); > > + return 0; > > +} > > + > > +# if __TIMESIZE != 64 > > +libc_hidden_def (__gettimeofday64) > > + > > +int > > +__gettimeofday (struct timeval *restrict tv, void *restrict tz) > > +{ > > + struct __timeval64 tv64; > > + if (__gettimeofday64 (&tv64, tz)) > > + return -1; > > + > > + if (! in_time_t_range (tv64.tv_sec)) > > + { > > + __set_errno (EOVERFLOW); > > + return -1; > > + } > > + > > + *tv = valid_timeval64_to_timeval (tv64); > > + return 0; > > +} > > +# endif > > +weak_alias (__gettimeofday, gettimeofday) > > #endif > > Ok. > > > diff --git a/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c > > b/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c index > > 183fb0ac70..2d6978f333 100644 --- > > a/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c +++ > > b/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c @@ -15,5 +15,7 @@ > > License along with the GNU C Library; if not, see > > <https://www.gnu.org/licenses/>. */ > > > > -#define USE_IFUNC_GETTIMEOFDAY > > +#ifdef __powerpc64__ > > +# define USE_IFUNC_GETTIMEOFDAY > > +#endif > > #include <sysdeps/unix/sysv/linux/gettimeofday.c> > > Ok. > > > diff --git a/sysdeps/unix/sysv/linux/x86/gettimeofday.c > > b/sysdeps/unix/sysv/linux/x86/gettimeofday.c index > > 1b7aa880a2..0c1779dc83 100644 --- > > a/sysdeps/unix/sysv/linux/x86/gettimeofday.c +++ > > b/sysdeps/unix/sysv/linux/x86/gettimeofday.c @@ -16,5 +16,7 @@ > > License along with the GNU C Library; if not, see > > <https://www.gnu.org/licenses/>. */ > > > > -#define USE_IFUNC_GETTIMEOFDAY > > +#ifdef __x86_64__ > > +# define USE_IFUNC_GETTIMEOFDAY > > +#endif > > #include <sysdeps/unix/sysv/linux/gettimeofday.c> > > > > 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
diff --git a/include/time.h b/include/time.h index 73f66277ac..61806658e7 100644 --- a/include/time.h +++ b/include/time.h @@ -227,10 +227,14 @@ libc_hidden_proto (__sched_rr_get_interval64); #if __TIMESIZE == 64 # define __settimeofday64 __settimeofday +# define __gettimeofday64 __gettimeofday #else extern int __settimeofday64 (const struct __timeval64 *tv, const struct timezone *tz); libc_hidden_proto (__settimeofday64) +extern int __gettimeofday64 (struct __timeval64 *restrict tv, + void *restrict tz); +libc_hidden_proto (__gettimeofday64) #endif /* Compute the `struct tm' representation of T, diff --git a/sysdeps/unix/sysv/linux/gettimeofday.c b/sysdeps/unix/sysv/linux/gettimeofday.c index d5cdb22495..cb57bc9cf2 100644 --- a/sysdeps/unix/sysv/linux/gettimeofday.c +++ b/sysdeps/unix/sysv/linux/gettimeofday.c @@ -54,5 +54,43 @@ __gettimeofday (struct timeval *restrict tv, void *restrict tz) # endif weak_alias (__gettimeofday, gettimeofday) #else /* USE_IFUNC_GETTIMEOFDAY */ -# include <time/gettimeofday.c> +/* Conversion of gettimeofday function to support 64 bit time on archs + with __WORDSIZE == 32 and __TIMESIZE == 32/64 */ +#include <errno.h> + +int +__gettimeofday64 (struct __timeval64 *restrict tv, void *restrict tz) +{ + if (__glibc_unlikely (tz != 0)) + memset (tz, 0, sizeof (struct timezone)); + + struct __timespec64 ts64; + if (__clock_gettime64 (CLOCK_REALTIME, &ts64)) + return -1; + + *tv = timespec64_to_timeval64 (ts64); + return 0; +} + +# if __TIMESIZE != 64 +libc_hidden_def (__gettimeofday64) + +int +__gettimeofday (struct timeval *restrict tv, void *restrict tz) +{ + struct __timeval64 tv64; + if (__gettimeofday64 (&tv64, tz)) + return -1; + + if (! in_time_t_range (tv64.tv_sec)) + { + __set_errno (EOVERFLOW); + return -1; + } + + *tv = valid_timeval64_to_timeval (tv64); + return 0; +} +# endif +weak_alias (__gettimeofday, gettimeofday) #endif diff --git a/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c b/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c index 183fb0ac70..2d6978f333 100644 --- a/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c +++ b/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c @@ -15,5 +15,7 @@ License along with the GNU C Library; if not, see <https://www.gnu.org/licenses/>. */ -#define USE_IFUNC_GETTIMEOFDAY +#ifdef __powerpc64__ +# define USE_IFUNC_GETTIMEOFDAY +#endif #include <sysdeps/unix/sysv/linux/gettimeofday.c> diff --git a/sysdeps/unix/sysv/linux/x86/gettimeofday.c b/sysdeps/unix/sysv/linux/x86/gettimeofday.c index 1b7aa880a2..0c1779dc83 100644 --- a/sysdeps/unix/sysv/linux/x86/gettimeofday.c +++ b/sysdeps/unix/sysv/linux/x86/gettimeofday.c @@ -16,5 +16,7 @@ License along with the GNU C Library; if not, see <https://www.gnu.org/licenses/>. */ -#define USE_IFUNC_GETTIMEOFDAY +#ifdef __x86_64__ +# define USE_IFUNC_GETTIMEOFDAY +#endif #include <sysdeps/unix/sysv/linux/gettimeofday.c>