This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
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
Attachment:
pgp_3fRkD6Ek0.pgp
Description: OpenPGP digital signature
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |