Message ID | 20200106121742.1628-1-lukma@denx.de |
---|---|
State | New |
Headers | show |
Series | [v4,1/2] y2038: linux: Provide __timerfd_gettime64 implementation | expand |
On 06/01/2020 09:17, Lukasz Majewski wrote: > This patch replaces auto generated wrapper (as described in > sysdeps/unix/sysv/linux/syscalls.list) for timerfd_gettime with one which > adds extra support for reading 64 bit time values on machines with > __TIMESIZE != 64. > There is no functional change for architectures already supporting 64 bit > time ABI. > > This patch is conceptually identical to timer_gettime conversion already > done in sysdeps/unix/sysv/linux/timer_gettime.c. > Please refer to corresponding commit message for detailed description of > introduced functions and the testing procedure. > > --- > Changes for v4: > - Update date from 2019 to 2020 > > Changes for v3: > - Add missing libc_hidden_def() > > Changes for v2: > - Remove "Contributed by" from the file header > - Remove early check for (fd < 0) in __timerfd_gettime64 as the fd > correctness check is already done in Linux kernel > - Add single descriptive comment line to provide concise explanation > of the code LGTM when 2.32 opens, thanks. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > --- > include/time.h | 3 + > sysdeps/unix/sysv/linux/Makefile | 3 +- > sysdeps/unix/sysv/linux/syscalls.list | 1 - > sysdeps/unix/sysv/linux/timerfd_gettime.c | 68 +++++++++++++++++++++++ > 4 files changed, 73 insertions(+), 2 deletions(-) > create mode 100644 sysdeps/unix/sysv/linux/timerfd_gettime.c > > diff --git a/include/time.h b/include/time.h > index e5e8246eac..eb5082b4d7 100644 > --- a/include/time.h > +++ b/include/time.h > @@ -181,9 +181,12 @@ libc_hidden_proto (__futimens64); > > #if __TIMESIZE == 64 > # define __timer_gettime64 __timer_gettime > +# define __timerfd_gettime64 __timerfd_gettime > #else > extern int __timer_gettime64 (timer_t timerid, struct __itimerspec64 *value); > +extern int __timerfd_gettime64 (int fd, struct __itimerspec64 *value); > libc_hidden_proto (__timer_gettime64); > +libc_hidden_proto (__timerfd_gettime64); > #endif > > #if __TIMESIZE == 64 Ok. > diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile > index f12b7b1a2d..74923740b9 100644 > --- a/sysdeps/unix/sysv/linux/Makefile > +++ b/sysdeps/unix/sysv/linux/Makefile > @@ -60,7 +60,8 @@ sysdep_routines += adjtimex clone umount umount2 readahead \ > setfsuid setfsgid epoll_pwait signalfd \ > eventfd eventfd_read eventfd_write prlimit \ > personality epoll_wait tee vmsplice splice \ > - open_by_handle_at mlock2 pkey_mprotect pkey_set pkey_get > + open_by_handle_at mlock2 pkey_mprotect pkey_set pkey_get \ > + timerfd_gettime > > CFLAGS-gethostid.c = -fexceptions > CFLAGS-tee.c = -fexceptions -fasynchronous-unwind-tables Ok. > diff --git a/sysdeps/unix/sysv/linux/syscalls.list b/sysdeps/unix/sysv/linux/syscalls.list > index 5f1352ad43..adb9055ce2 100644 > --- a/sysdeps/unix/sysv/linux/syscalls.list > +++ b/sysdeps/unix/sysv/linux/syscalls.list > @@ -94,7 +94,6 @@ mq_setattr - mq_getsetattr i:ipp mq_setattr > > timerfd_create EXTRA timerfd_create i:ii timerfd_create > timerfd_settime EXTRA timerfd_settime i:iipp timerfd_settime > -timerfd_gettime EXTRA timerfd_gettime i:ip timerfd_gettime > > fanotify_init EXTRA fanotify_init i:ii fanotify_init > Ok. > diff --git a/sysdeps/unix/sysv/linux/timerfd_gettime.c b/sysdeps/unix/sysv/linux/timerfd_gettime.c > new file mode 100644 > index 0000000000..7d09eeb11a > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/timerfd_gettime.c > @@ -0,0 +1,68 @@ > +/* timerfd_gettime -- get the timer setting referred to by file descriptor. > + Copyright (C) 2020 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public License as > + published by the Free Software Foundation; either version 2.1 of the > + License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; see the file COPYING.LIB. If > + not, see <https://www.gnu.org/licenses/>. */ > + > +#include <errno.h> > +#include <stdlib.h> > +#include <time.h> > +#include <sysdep.h> > +#include <kernel-features.h> > + > +int > +__timerfd_gettime64 (int fd, struct __itimerspec64 *value) > +{ > +#ifdef __ASSUME_TIME64_SYSCALLS > +# ifndef __NR_timerfd_gettime64 > +# define __NR_timerfd_gettime64 __NR_timerfd_gettime > +# endif > + return INLINE_SYSCALL_CALL (timerfd_gettime64, fd, value); > +#else > +# ifdef __NR_timerfd_gettime64 > + int ret = INLINE_SYSCALL_CALL (timerfd_gettime64, fd, value); > + if (ret == 0 || errno != ENOSYS) > + return ret; > +# endif Ok. As a side note, now that arch-syscall patch is upstream should we assume that for !__ASSUME_TIME64_SYSCALLS the __NR_timerfd_gettime64 should be defined (meaning that Linux supports time64 for all 32-bit architectures)? > + struct itimerspec its32; > + int retval = INLINE_SYSCALL_CALL (timerfd_gettime, fd, &its32); > + if (retval == 0) > + { > + value->it_interval = valid_timespec_to_timespec64 (its32.it_interval); > + value->it_value = valid_timespec_to_timespec64 (its32.it_value); > + } > + > + return retval; > +#endif > +} Ok. > + > +#if __TIMESIZE != 64 > +libc_hidden_def (__timerfd_gettime64) Ok. As a side note, we should fix it on clock_{get,set}time to add the missing libc_hidden_def. > + > +int > +__timerfd_gettime (int fd, struct itimerspec *value) > +{ > + struct __itimerspec64 its64; > + int retval = __timerfd_gettime64 (fd, &its64); > + if (retval == 0) > + { > + value->it_interval = valid_timespec64_to_timespec (its64.it_interval); > + value->it_value = valid_timespec64_to_timespec (its64.it_value); > + } > + > + return retval; > +} > +#endif > +strong_alias (__timerfd_gettime, timerfd_gettime) > Ok.
Hi Adhemerval, > On 06/01/2020 09:17, Lukasz Majewski wrote: > > This patch replaces auto generated wrapper (as described in > > sysdeps/unix/sysv/linux/syscalls.list) for timerfd_gettime with one > > which adds extra support for reading 64 bit time values on machines > > with __TIMESIZE != 64. > > There is no functional change for architectures already supporting > > 64 bit time ABI. > > > > This patch is conceptually identical to timer_gettime conversion > > already done in sysdeps/unix/sysv/linux/timer_gettime.c. > > Please refer to corresponding commit message for detailed > > description of introduced functions and the testing procedure. > > > > --- > > Changes for v4: > > - Update date from 2019 to 2020 > > > > Changes for v3: > > - Add missing libc_hidden_def() > > > > Changes for v2: > > - Remove "Contributed by" from the file header > > - Remove early check for (fd < 0) in __timerfd_gettime64 as the fd > > correctness check is already done in Linux kernel > > - Add single descriptive comment line to provide concise explanation > > of the code > > LGTM when 2.32 opens, thanks. Ok. Now we do have the "freeze" (fixing period) for 2.31. > > Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > > > --- > > include/time.h | 3 + > > sysdeps/unix/sysv/linux/Makefile | 3 +- > > sysdeps/unix/sysv/linux/syscalls.list | 1 - > > sysdeps/unix/sysv/linux/timerfd_gettime.c | 68 > > +++++++++++++++++++++++ 4 files changed, 73 insertions(+), 2 > > deletions(-) create mode 100644 > > sysdeps/unix/sysv/linux/timerfd_gettime.c > > > > diff --git a/include/time.h b/include/time.h > > index e5e8246eac..eb5082b4d7 100644 > > --- a/include/time.h > > +++ b/include/time.h > > @@ -181,9 +181,12 @@ libc_hidden_proto (__futimens64); > > > > #if __TIMESIZE == 64 > > # define __timer_gettime64 __timer_gettime > > +# define __timerfd_gettime64 __timerfd_gettime > > #else > > extern int __timer_gettime64 (timer_t timerid, struct > > __itimerspec64 *value); +extern int __timerfd_gettime64 (int fd, > > struct __itimerspec64 *value); libc_hidden_proto > > (__timer_gettime64); +libc_hidden_proto (__timerfd_gettime64); > > #endif > > > > #if __TIMESIZE == 64 > > Ok. > > > diff --git a/sysdeps/unix/sysv/linux/Makefile > > b/sysdeps/unix/sysv/linux/Makefile index f12b7b1a2d..74923740b9 > > 100644 --- a/sysdeps/unix/sysv/linux/Makefile > > +++ b/sysdeps/unix/sysv/linux/Makefile > > @@ -60,7 +60,8 @@ sysdep_routines += adjtimex clone umount umount2 > > readahead \ setfsuid setfsgid epoll_pwait signalfd \ > > eventfd eventfd_read eventfd_write prlimit \ > > personality epoll_wait tee vmsplice splice \ > > - open_by_handle_at mlock2 pkey_mprotect pkey_set > > pkey_get > > + open_by_handle_at mlock2 pkey_mprotect pkey_set > > pkey_get \ > > + timerfd_gettime > > > > CFLAGS-gethostid.c = -fexceptions > > CFLAGS-tee.c = -fexceptions -fasynchronous-unwind-tables > > Ok. > > > diff --git a/sysdeps/unix/sysv/linux/syscalls.list > > b/sysdeps/unix/sysv/linux/syscalls.list index > > 5f1352ad43..adb9055ce2 100644 --- > > a/sysdeps/unix/sysv/linux/syscalls.list +++ > > b/sysdeps/unix/sysv/linux/syscalls.list @@ -94,7 +94,6 @@ > > mq_setattr - mq_getsetattr i:ipp > > mq_setattr timerfd_create EXTRA timerfd_create > > i:ii timerfd_create timerfd_settime EXTRA > > timerfd_settime i:iipp timerfd_settime > > -timerfd_gettime EXTRA timerfd_gettime > > i:ip timerfd_gettime fanotify_init EXTRA > > fanotify_init i:ii fanotify_init > > Ok. > > > diff --git a/sysdeps/unix/sysv/linux/timerfd_gettime.c > > b/sysdeps/unix/sysv/linux/timerfd_gettime.c new file mode 100644 > > index 0000000000..7d09eeb11a > > --- /dev/null > > +++ b/sysdeps/unix/sysv/linux/timerfd_gettime.c > > @@ -0,0 +1,68 @@ > > +/* timerfd_gettime -- get the timer setting referred to by file > > descriptor. > > + Copyright (C) 2020 Free Software Foundation, Inc. > > + This file is part of the GNU C Library. > > + > > + The GNU C Library is free software; you can redistribute it > > and/or > > + modify it under the terms of the GNU Lesser General Public > > License as > > + published by the Free Software Foundation; either version 2.1 > > of the > > + License, or (at your option) any later version. > > + > > + The GNU C Library is distributed in the hope that it will be > > useful, > > + but WITHOUT ANY WARRANTY; without even the implied warranty of > > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > GNU > > + Lesser General Public License for more details. > > + > > + You should have received a copy of the GNU Lesser General Public > > + License along with the GNU C Library; see the file COPYING.LIB. > > If > > + not, see <https://www.gnu.org/licenses/>. */ > > + > > +#include <errno.h> > > +#include <stdlib.h> > > +#include <time.h> > > +#include <sysdep.h> > > +#include <kernel-features.h> > > + > > +int > > +__timerfd_gettime64 (int fd, struct __itimerspec64 *value) > > +{ > > +#ifdef __ASSUME_TIME64_SYSCALLS > > +# ifndef __NR_timerfd_gettime64 > > +# define __NR_timerfd_gettime64 __NR_timerfd_gettime > > +# endif > > + return INLINE_SYSCALL_CALL (timerfd_gettime64, fd, value); > > +#else > > +# ifdef __NR_timerfd_gettime64 > > + int ret = INLINE_SYSCALL_CALL (timerfd_gettime64, fd, value); > > + if (ret == 0 || errno != ENOSYS) > > + return ret; > > +# endif > > Ok. > > As a side note, now that arch-syscall patch is upstream should we > assume that for !__ASSUME_TIME64_SYSCALLS the __NR_timerfd_gettime64 > should be defined (meaning that Linux supports time64 for all 32-bit > architectures)? Only Linux version >= 5.1 supports 64 bit time on archs with __WORDSIZE = 32. I do guess (but I may be wrong here) that the arch-syscall is supposed to reflect the exact syscalls provided by kernel headers used for building (to help with validation of Y2038 patches). > > > + struct itimerspec its32; > > + int retval = INLINE_SYSCALL_CALL (timerfd_gettime, fd, &its32); > > + if (retval == 0) > > + { > > + value->it_interval = valid_timespec_to_timespec64 > > (its32.it_interval); > > + value->it_value = valid_timespec_to_timespec64 > > (its32.it_value); > > + } > > + > > + return retval; > > +#endif > > +} > > > Ok. > > > + > > +#if __TIMESIZE != 64 > > +libc_hidden_def (__timerfd_gettime64) > > Ok. > > As a side note, we should fix it on clock_{get,set}time to add the > missing libc_hidden_def. The clock_gettime already has libc_hidden_def. The difference is that we use some compatibility code (after moving clock_gettime from librt to libc) instead of strong_alias (as it mimics the behavior from auto generated syscall wrapper). > > > + > > +int > > +__timerfd_gettime (int fd, struct itimerspec *value) > > +{ > > + struct __itimerspec64 its64; > > + int retval = __timerfd_gettime64 (fd, &its64); > > + if (retval == 0) > > + { > > + value->it_interval = valid_timespec64_to_timespec > > (its64.it_interval); > > + value->it_value = valid_timespec64_to_timespec > > (its64.it_value); > > + } > > + > > + return retval; > > +} > > +#endif > > +strong_alias (__timerfd_gettime, timerfd_gettime) > > > > Ok. Thanks for the review. 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 07/01/2020 06:27, Lukasz Majewski wrote: >> As a side note, now that arch-syscall patch is upstream should we >> assume that for !__ASSUME_TIME64_SYSCALLS the __NR_timerfd_gettime64 >> should be defined (meaning that Linux supports time64 for all 32-bit >> architectures)? > > Only Linux version >= 5.1 supports 64 bit time on archs with __WORDSIZE > = 32. I do guess (but I may be wrong here) that the arch-syscall is > supposed to reflect the exact syscalls provided by kernel headers used > for building (to help with validation of Y2038 patches). The arch-syscall is now autogenerated from the latest kernel release defined in build-many-glibcs.py. So the question is whether Linux support and enforces time64 support on all and future 32-bit architectures or if there is still some missing ones (as it has happen on some syscall additions, where some architecture lag behind some releases). > >> >>> + struct itimerspec its32; >>> + int retval = INLINE_SYSCALL_CALL (timerfd_gettime, fd, &its32); >>> + if (retval == 0) >>> + { >>> + value->it_interval = valid_timespec_to_timespec64 >>> (its32.it_interval); >>> + value->it_value = valid_timespec_to_timespec64 >>> (its32.it_value); >>> + } >>> + >>> + return retval; >>> +#endif >>> +} >> >> >> Ok. >> >>> + >>> +#if __TIMESIZE != 64 >>> +libc_hidden_def (__timerfd_gettime64) >> >> Ok. >> >> As a side note, we should fix it on clock_{get,set}time to add the >> missing libc_hidden_def. > > The clock_gettime already has libc_hidden_def. The difference is that we > use some compatibility code (after moving clock_gettime from librt to > libc) instead of strong_alias (as it mimics the behavior from auto > generated syscall wrapper). I meant for the new time64 symbols. Currently it is not an issue because the internal time64 symbol is not exported and static linker uses the internal __GI_ name for the symbol. For instance, objdump -t on clock_gettime.os on a 32-bit architecture (powerpc in this case) shows: 00000144 g F .text 00000088 __clock_gettime 00000144 g F .text 00000088 __clock_gettime_2 00000000 g F .text 00000144 .hidden __GI___clock_gettime64 00000144 g F .text 00000088 .hidden __GI___clock_gettime 00000144 g F .text 00000088 clock_gettime@@GLIBC_2.17 00000144 g F .text 00000088 clock_gettime@GLIBC_2.2 Where with a libc_hidden_def (__clock_gettime64) it shows: 00000144 g F .text 00000088 __clock_gettime 00000144 g F .text 00000088 __clock_gettime_2 00000000 g F .text 00000144 .hidden __GI___clock_gettime64 *00000000 g F .text 00000144 __clock_gettime64 00000144 g F .text 00000088 .hidden __GI___clock_gettime 00000144 g F .text 00000088 clock_gettime@@GLIBC_2.17 00000144 g F .text 00000088 clock_gettime@GLIBC_2.2 The requirement of libc_hidden_def will de defined in the end if glibc exports or not __clock_gettime64 on some header redirection or if clock_gettime64 would be suffice (with a {weak,strong}_alias). However I do think we should fix it to avoid such confusion why there is a hidden_proto and not a hidden_def.
* Lukasz Majewski: > Only Linux version >= 5.1 supports 64 bit time on archs with __WORDSIZE > = 32. I do guess (but I may be wrong here) that the arch-syscall is > supposed to reflect the exact syscalls provided by kernel headers used > for building (to help with validation of Y2038 patches). The tables are supposed to be complete in the sense that they include all system calls which (a) have a system call number assigned in some recent kernel version, and (b) are actually used during the glibc build in some way. The intent is that you can build glibc against older kernel headers (say Linux 4.18) and still get the exact same system call profile as you would get when building against Linux 5.3 (or Linux 4.18 with random system call backports). As Adhemerval pointed out, (a) can be problematic in the sense that system call numbers could be added late on some architectures. This only matters if their existence actually affects the glibc build, as per (b). But if it does, we would have to update the tables. Thanks, Florian
Hi Adhemerval, > On 07/01/2020 06:27, Lukasz Majewski wrote: > > >> As a side note, now that arch-syscall patch is upstream should we > >> assume that for !__ASSUME_TIME64_SYSCALLS the > >> __NR_timerfd_gettime64 should be defined (meaning that Linux > >> supports time64 for all 32-bit architectures)? > > > > Only Linux version >= 5.1 supports 64 bit time on archs with > > __WORDSIZE = 32. I do guess (but I may be wrong here) that the > > arch-syscall is supposed to reflect the exact syscalls provided by > > kernel headers used for building (to help with validation of Y2038 > > patches). > > The arch-syscall is now autogenerated from the latest kernel release > defined in build-many-glibcs.py. So the question is whether Linux > support and enforces time64 support on all and future 32-bit > architectures or if there is still some missing ones (as it has > happen on some syscall additions, where some architecture lag > behind some releases). This question would be best answered by Arnd (CC'ed) IMHO. From what I know all 32 bit architectures gained syscalls covered by __ASSUME_TIME64_SYSCALLS from Linux 5.1+. The arch-syscall seems to me like a mean to test for example the time related syscalls which use different versions (32bit time vs 64 bit) on different archs. Notable example - clock_gettime(). Am I right? > > > > > >> > >>> + struct itimerspec its32; > >>> + int retval = INLINE_SYSCALL_CALL (timerfd_gettime, fd, &its32); > >>> + if (retval == 0) > >>> + { > >>> + value->it_interval = valid_timespec_to_timespec64 > >>> (its32.it_interval); > >>> + value->it_value = valid_timespec_to_timespec64 > >>> (its32.it_value); > >>> + } > >>> + > >>> + return retval; > >>> +#endif > >>> +} > >> > >> > >> Ok. > >> > >>> + > >>> +#if __TIMESIZE != 64 > >>> +libc_hidden_def (__timerfd_gettime64) > >> > >> Ok. > >> > >> As a side note, we should fix it on clock_{get,set}time to add the > >> missing libc_hidden_def. > > > > The clock_gettime already has libc_hidden_def. The difference is > > that we use some compatibility code (after moving clock_gettime > > from librt to libc) instead of strong_alias (as it mimics the > > behavior from auto generated syscall wrapper). > > I meant for the new time64 symbols. Currently it is not an issue > because the internal time64 symbol is not exported and static linker > uses the internal __GI_ name for the symbol. For instance, objdump > -t on clock_gettime.os on a 32-bit architecture (powerpc in this > case) shows: > > 00000144 g F .text 00000088 __clock_gettime > 00000144 g F .text 00000088 __clock_gettime_2 > 00000000 g F .text 00000144 .hidden __GI___clock_gettime64 > 00000144 g F .text 00000088 .hidden __GI___clock_gettime > 00000144 g F .text 00000088 clock_gettime@@GLIBC_2.17 > 00000144 g F .text 00000088 clock_gettime@GLIBC_2.2 > > Where with a libc_hidden_def (__clock_gettime64) it shows: > > 00000144 g F .text 00000088 __clock_gettime > 00000144 g F .text 00000088 __clock_gettime_2 > 00000000 g F .text 00000144 .hidden __GI___clock_gettime64 > *00000000 g F .text 00000144 __clock_gettime64 > 00000144 g F .text 00000088 .hidden __GI___clock_gettime > 00000144 g F .text 00000088 clock_gettime@@GLIBC_2.17 > 00000144 g F .text 00000088 clock_gettime@GLIBC_2.2 > > The requirement of libc_hidden_def will de defined in the end if glibc > exports or not __clock_gettime64 on some header redirection or if The __clock_gettime64 is going to be exported (as clock_gettime redirection) on 32 bit archs which are going to be Y2038 safe (with 64 bit time_t). > clock_gettime64 would be suffice (with a {weak,strong}_alias). > The internal in-glibc usage (calling) of clock_gettime() shall be replaced by either __clock_gettime64 or clock_gettime64. I would prefer the former as it reflects that it is internal function (with __ prefix). > However I do think we should fix it to avoid such confusion why there > is a hidden_proto and not a hidden_def. +1. 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 Tue, Jan 7, 2020 at 3:25 PM Lukasz Majewski <lukma@denx.de> wrote: > > On 07/01/2020 06:27, Lukasz Majewski wrote: > > > > >> As a side note, now that arch-syscall patch is upstream should we > > >> assume that for !__ASSUME_TIME64_SYSCALLS the > > >> __NR_timerfd_gettime64 should be defined (meaning that Linux > > >> supports time64 for all 32-bit architectures)? > > > > > > Only Linux version >= 5.1 supports 64 bit time on archs with > > > __WORDSIZE = 32. I do guess (but I may be wrong here) that the > > > arch-syscall is supposed to reflect the exact syscalls provided by > > > kernel headers used for building (to help with validation of Y2038 > > > patches). > > > > The arch-syscall is now autogenerated from the latest kernel release > > defined in build-many-glibcs.py. So the question is whether Linux > > support and enforces time64 support on all and future 32-bit > > architectures or if there is still some missing ones (as it has > > happen on some syscall additions, where some architecture lag > > behind some releases). > > This question would be best answered by Arnd (CC'ed) IMHO. From what I > know all 32 bit architectures gained syscalls covered by > __ASSUME_TIME64_SYSCALLS from Linux 5.1+. Yes, we intentionally converted all architectures at the same time to have a reliable baseline, i.e. once a future glibc requires linux-5.1 as the minimum kernel all the backwards-compatibility support for old kernels can be dropped. New 32-bit architectures (if any) will only support the time64 syscalls and not time time32 ones. For some ioctl interfaces, you also need to use the latest kernel headers, e.g. sound/asound.h from kernels before 5.6 has some bugs with time64. For the ioctl implementation I hope to wrap up the final bits in linux-5.6 as well, earlier kernels may return -EINVAL on some of the ioctls that pass a time_t. Arnd
On 07/01/2020 11:25, Lukasz Majewski wrote: > Hi Adhemerval, > >> On 07/01/2020 06:27, Lukasz Majewski wrote: >> >>>> As a side note, now that arch-syscall patch is upstream should we >>>> assume that for !__ASSUME_TIME64_SYSCALLS the >>>> __NR_timerfd_gettime64 should be defined (meaning that Linux >>>> supports time64 for all 32-bit architectures)? >>> >>> Only Linux version >= 5.1 supports 64 bit time on archs with >>> __WORDSIZE = 32. I do guess (but I may be wrong here) that the >>> arch-syscall is supposed to reflect the exact syscalls provided by >>> kernel headers used for building (to help with validation of Y2038 >>> patches). >> >> The arch-syscall is now autogenerated from the latest kernel release >> defined in build-many-glibcs.py. So the question is whether Linux >> support and enforces time64 support on all and future 32-bit >> architectures or if there is still some missing ones (as it has >> happen on some syscall additions, where some architecture lag >> behind some releases). > > This question would be best answered by Arnd (CC'ed) IMHO. From what I > know all 32 bit architectures gained syscalls covered by > __ASSUME_TIME64_SYSCALLS from Linux 5.1+. > > The arch-syscall seems to me like a mean to test for example the time > related syscalls which use different versions (32bit time vs 64 bit) on > different archs. Notable example - clock_gettime(). Am I right? The arch-syscall is a way to decouple the build from the kernel header used on build, which might simplify the logic to use some kernel features. On the clock_gettime, for instance, as Arnd has indicated we can assume that __NR_clock_gettime64 will be always presented for !__ASSUME_TIME64_SYSCALLS. It would be interesting if kernel also could enforce that new generic syscalls would be wire-up, or at least the syscall number reserved; once a new generic syscall is introduced. It would simplify the __ASSUME_* macro, not requiring the arch-specific overrides on some architectures. > > The __clock_gettime64 is going to be exported (as clock_gettime > redirection) on 32 bit archs which are going to be Y2038 safe (with 64 > bit time_t). > >> clock_gettime64 would be suffice (with a {weak,strong}_alias). >> > > The internal in-glibc usage (calling) of clock_gettime() shall be > replaced by either __clock_gettime64 or clock_gettime64. I would prefer > the former as it reflects that it is internal function (with __ prefix). It required to be the former because we also need to take in consideration linking namespace pollution. > >> However I do think we should fix it to avoid such confusion why there >> is a hidden_proto and not a hidden_def. > > +1. Ack, I will send a patch.
On Tue, Jan 7, 2020 at 9:16 PM Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote: > On 07/01/2020 11:25, Lukasz Majewski wrote: > >> On 07/01/2020 06:27, Lukasz Majewski wrote: > >> > >>>> As a side note, now that arch-syscall patch is upstream should we > >>>> assume that for !__ASSUME_TIME64_SYSCALLS the > >>>> __NR_timerfd_gettime64 should be defined (meaning that Linux > >>>> supports time64 for all 32-bit architectures)? > >>> > >>> Only Linux version >= 5.1 supports 64 bit time on archs with > >>> __WORDSIZE = 32. I do guess (but I may be wrong here) that the > >>> arch-syscall is supposed to reflect the exact syscalls provided by > >>> kernel headers used for building (to help with validation of Y2038 > >>> patches). > >> > >> The arch-syscall is now autogenerated from the latest kernel release > >> defined in build-many-glibcs.py. So the question is whether Linux > >> support and enforces time64 support on all and future 32-bit > >> architectures or if there is still some missing ones (as it has > >> happen on some syscall additions, where some architecture lag > >> behind some releases). > > > > This question would be best answered by Arnd (CC'ed) IMHO. From what I > > know all 32 bit architectures gained syscalls covered by > > __ASSUME_TIME64_SYSCALLS from Linux 5.1+. > > > > The arch-syscall seems to me like a mean to test for example the time > > related syscalls which use different versions (32bit time vs 64 bit) on > > different archs. Notable example - clock_gettime(). Am I right? > > The arch-syscall is a way to decouple the build from the kernel header > used on build, which might simplify the logic to use some kernel > features. > > On the clock_gettime, for instance, as Arnd has indicated we can > assume that __NR_clock_gettime64 will be always presented for > !__ASSUME_TIME64_SYSCALLS. > > It would be interesting if kernel also could enforce that new > generic syscalls would be wire-up, or at least the syscall number > reserved; once a new generic syscall is introduced. It would > simplify the __ASSUME_* macro, not requiring the arch-specific > overrides on some architectures. We currently try to enforce it through review since the introduction of the automatically generated asm/unistd.h header files, but his has already failed one time for the recent clone3 system call that accidentally was missing on one architecture. I have some plans to enforce it using tooling, but it should at least be a safe assumption that all new system calls have the same numbers across all architectures and are added at the same time. I also have some vague plans to automatically generate not only the asm/unistd.h header but also some trivial wrappers around syscall() that provide an inline function with the correct arguments, to allow calling any syscall without relying on libc to provide a wrapper for it. Arnd
Hi Adhemerval, > On 07/01/2020 11:25, Lukasz Majewski wrote: > > Hi Adhemerval, > > > >> On 07/01/2020 06:27, Lukasz Majewski wrote: > >> > >>>> As a side note, now that arch-syscall patch is upstream should we > >>>> assume that for !__ASSUME_TIME64_SYSCALLS the > >>>> __NR_timerfd_gettime64 should be defined (meaning that Linux > >>>> supports time64 for all 32-bit architectures)? > >>> > >>> Only Linux version >= 5.1 supports 64 bit time on archs with > >>> __WORDSIZE = 32. I do guess (but I may be wrong here) that the > >>> arch-syscall is supposed to reflect the exact syscalls provided by > >>> kernel headers used for building (to help with validation of Y2038 > >>> patches). > >> > >> The arch-syscall is now autogenerated from the latest kernel > >> release defined in build-many-glibcs.py. So the question is > >> whether Linux support and enforces time64 support on all and > >> future 32-bit architectures or if there is still some missing ones > >> (as it has happen on some syscall additions, where some > >> architecture lag behind some releases). > > > > This question would be best answered by Arnd (CC'ed) IMHO. From > > what I know all 32 bit architectures gained syscalls covered by > > __ASSUME_TIME64_SYSCALLS from Linux 5.1+. > > > > The arch-syscall seems to me like a mean to test for example the > > time related syscalls which use different versions (32bit time vs > > 64 bit) on different archs. Notable example - clock_gettime(). Am I > > right? > > The arch-syscall is a way to decouple the build from the kernel header > used on build, So then we will build against the newest kernel (like 5.4 now). As it was noted in the other thread - this would simplify the build-many-glibcs.py > which might simplify the logic to use some kernel > features. I must admit that I do not see such simplification... Could you give an example? > > On the clock_gettime, for instance, as Arnd has indicated we can > assume that __NR_clock_gettime64 will be always presented for > !__ASSUME_TIME64_SYSCALLS. > > It would be interesting if kernel also could enforce that new > generic syscalls would be wire-up, or at least the syscall number > reserved; once a new generic syscall is introduced. It would > simplify the __ASSUME_* macro, not requiring the arch-specific > overrides on some architectures. > > > > > The __clock_gettime64 is going to be exported (as clock_gettime > > redirection) on 32 bit archs which are going to be Y2038 safe (with > > 64 bit time_t). > > > >> clock_gettime64 would be suffice (with a {weak,strong}_alias). > >> > > > > The internal in-glibc usage (calling) of clock_gettime() shall be > > replaced by either __clock_gettime64 or clock_gettime64. I would > > prefer the former as it reflects that it is internal function (with > > __ prefix). > > It required to be the former because we also need to take in > consideration linking namespace pollution. > > > > >> However I do think we should fix it to avoid such confusion why > >> there is a hidden_proto and not a hidden_def. > > > > +1. > > Ack, I will send a patch. Thanks. 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 e5e8246eac..eb5082b4d7 100644 --- a/include/time.h +++ b/include/time.h @@ -181,9 +181,12 @@ libc_hidden_proto (__futimens64); #if __TIMESIZE == 64 # define __timer_gettime64 __timer_gettime +# define __timerfd_gettime64 __timerfd_gettime #else extern int __timer_gettime64 (timer_t timerid, struct __itimerspec64 *value); +extern int __timerfd_gettime64 (int fd, struct __itimerspec64 *value); libc_hidden_proto (__timer_gettime64); +libc_hidden_proto (__timerfd_gettime64); #endif #if __TIMESIZE == 64 diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile index f12b7b1a2d..74923740b9 100644 --- a/sysdeps/unix/sysv/linux/Makefile +++ b/sysdeps/unix/sysv/linux/Makefile @@ -60,7 +60,8 @@ sysdep_routines += adjtimex clone umount umount2 readahead \ setfsuid setfsgid epoll_pwait signalfd \ eventfd eventfd_read eventfd_write prlimit \ personality epoll_wait tee vmsplice splice \ - open_by_handle_at mlock2 pkey_mprotect pkey_set pkey_get + open_by_handle_at mlock2 pkey_mprotect pkey_set pkey_get \ + timerfd_gettime CFLAGS-gethostid.c = -fexceptions CFLAGS-tee.c = -fexceptions -fasynchronous-unwind-tables diff --git a/sysdeps/unix/sysv/linux/syscalls.list b/sysdeps/unix/sysv/linux/syscalls.list index 5f1352ad43..adb9055ce2 100644 --- a/sysdeps/unix/sysv/linux/syscalls.list +++ b/sysdeps/unix/sysv/linux/syscalls.list @@ -94,7 +94,6 @@ mq_setattr - mq_getsetattr i:ipp mq_setattr timerfd_create EXTRA timerfd_create i:ii timerfd_create timerfd_settime EXTRA timerfd_settime i:iipp timerfd_settime -timerfd_gettime EXTRA timerfd_gettime i:ip timerfd_gettime fanotify_init EXTRA fanotify_init i:ii fanotify_init diff --git a/sysdeps/unix/sysv/linux/timerfd_gettime.c b/sysdeps/unix/sysv/linux/timerfd_gettime.c new file mode 100644 index 0000000000..7d09eeb11a --- /dev/null +++ b/sysdeps/unix/sysv/linux/timerfd_gettime.c @@ -0,0 +1,68 @@ +/* timerfd_gettime -- get the timer setting referred to by file descriptor. + Copyright (C) 2020 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public License as + published by the Free Software Foundation; either version 2.1 of the + License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; see the file COPYING.LIB. If + not, see <https://www.gnu.org/licenses/>. */ + +#include <errno.h> +#include <stdlib.h> +#include <time.h> +#include <sysdep.h> +#include <kernel-features.h> + +int +__timerfd_gettime64 (int fd, struct __itimerspec64 *value) +{ +#ifdef __ASSUME_TIME64_SYSCALLS +# ifndef __NR_timerfd_gettime64 +# define __NR_timerfd_gettime64 __NR_timerfd_gettime +# endif + return INLINE_SYSCALL_CALL (timerfd_gettime64, fd, value); +#else +# ifdef __NR_timerfd_gettime64 + int ret = INLINE_SYSCALL_CALL (timerfd_gettime64, fd, value); + if (ret == 0 || errno != ENOSYS) + return ret; +# endif + struct itimerspec its32; + int retval = INLINE_SYSCALL_CALL (timerfd_gettime, fd, &its32); + if (retval == 0) + { + value->it_interval = valid_timespec_to_timespec64 (its32.it_interval); + value->it_value = valid_timespec_to_timespec64 (its32.it_value); + } + + return retval; +#endif +} + +#if __TIMESIZE != 64 +libc_hidden_def (__timerfd_gettime64) + +int +__timerfd_gettime (int fd, struct itimerspec *value) +{ + struct __itimerspec64 its64; + int retval = __timerfd_gettime64 (fd, &its64); + if (retval == 0) + { + value->it_interval = valid_timespec64_to_timespec (its64.it_interval); + value->it_value = valid_timespec64_to_timespec (its64.it_value); + } + + return retval; +} +#endif +strong_alias (__timerfd_gettime, timerfd_gettime)