[RFC,v4,24/24] timerfd_settime: Use 64-bit call if avaliable
Commit Message
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
sysdeps/unix/sysv/linux/Makefile | 2 +
sysdeps/unix/sysv/linux/sys/timerfd.h | 21 +++--
sysdeps/unix/sysv/linux/syscalls.list | 1 -
sysdeps/unix/sysv/linux/timerfd_settime.c | 104 ++++++++++++++++++++++
4 files changed, 120 insertions(+), 8 deletions(-)
create mode 100644 sysdeps/unix/sysv/linux/timerfd_settime.c
Comments
On Fri, Aug 9, 2019 at 6:04 PM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
This commit was clearly not tested very well (it's missing
semicolons), just see it more as pseudo code.
Alistair
> sysdeps/unix/sysv/linux/Makefile | 2 +
> sysdeps/unix/sysv/linux/sys/timerfd.h | 21 +++--
> sysdeps/unix/sysv/linux/syscalls.list | 1 -
> sysdeps/unix/sysv/linux/timerfd_settime.c | 104 ++++++++++++++++++++++
> 4 files changed, 120 insertions(+), 8 deletions(-)
> create mode 100644 sysdeps/unix/sysv/linux/timerfd_settime.c
>
> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> index 1ab6bcbfc81..89128e80868 100644
> --- a/sysdeps/unix/sysv/linux/Makefile
> +++ b/sysdeps/unix/sysv/linux/Makefile
> @@ -19,6 +19,8 @@ sysdep_routines += clone umount umount2 readahead \
> eventfd eventfd_read eventfd_write prlimit \
> personality epoll_wait tee vmsplice splice \
> open_by_handle_at mlock2 pkey_mprotect pkey_set pkey_get
> + timerfd_settime
> +
>
> CFLAGS-gethostid.c = -fexceptions
> CFLAGS-tee.c = -fexceptions -fasynchronous-unwind-tables
> diff --git a/sysdeps/unix/sysv/linux/sys/timerfd.h b/sysdeps/unix/sysv/linux/sys/timerfd.h
> index 5e5ad351a0d..51f63b357c1 100644
> --- a/sysdeps/unix/sysv/linux/sys/timerfd.h
> +++ b/sysdeps/unix/sysv/linux/sys/timerfd.h
> @@ -24,6 +24,13 @@
> /* Get the platform-dependent flags. */
> #include <bits/timerfd.h>
>
> +#if __TIMESIZE == 32
> +struct __itimerspec64
> + {
> + struct __timespec64 it_interval;
> + struct __timespec64 it_value;
> + };
> +#endif
>
> /* Bits to be set in the FLAGS parameter of `timerfd_settime'. */
> enum
> @@ -40,16 +47,16 @@ __BEGIN_DECLS
> /* Return file descriptor for new interval timer source. */
> extern int timerfd_create (__clockid_t __clock_id, int __flags) __THROW;
>
> -/* Set next expiration time of interval timer source UFD to UTMR. If
> - FLAGS has the TFD_TIMER_ABSTIME flag set the timeout value is
> - absolute. Optionally return the old expiration time in OTMR. */
> -extern int timerfd_settime (int __ufd, int __flags,
> - const struct itimerspec *__utmr,
> - struct itimerspec *__otmr) __THROW;
> -
> /* Return the next expiration time of UFD. */
> extern int timerfd_gettime (int __ufd, struct itimerspec *__otmr) __THROW;
>
> __END_DECLS
>
> +/* Set next expiration time of interval timer source UFD to UTMR. If
> + FLAGS has the TFD_TIMER_ABSTIME flag set the timeout value is
> + absolute. Optionally return the old expiration time in OTMR. */
> +int timerfd_settime (int __ufd, int __flags,
> + const struct itimerspec *__utmr,
> + struct itimerspec *__otmr) __THROW;
> +
> #endif /* sys/timerfd.h */
> diff --git a/sysdeps/unix/sysv/linux/syscalls.list b/sysdeps/unix/sysv/linux/syscalls.list
> index 4844d1a9a3b..3fa1a5de63a 100644
> --- a/sysdeps/unix/sysv/linux/syscalls.list
> +++ b/sysdeps/unix/sysv/linux/syscalls.list
> @@ -96,7 +96,6 @@ fremovexattr - fremovexattr i:is fremovexattr
> mq_setattr - mq_getsetattr i:ipp mq_setattr
>
> timerfd_create EXTRA timerfd_create i:ii timerfd_create
> -timerfd_settime EXTRA timerfd_settime64 i:iipp timerfd_settime
> timerfd_gettime EXTRA timerfd_gettime64 i:ip timerfd_gettime
>
> fanotify_init EXTRA fanotify_init i:ii fanotify_init
> diff --git a/sysdeps/unix/sysv/linux/timerfd_settime.c b/sysdeps/unix/sysv/linux/timerfd_settime.c
> new file mode 100644
> index 00000000000..830faeada77
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/timerfd_settime.c
> @@ -0,0 +1,104 @@
> +/* Copyright (C) 2003-2019 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> + Contributed by Ulrich Drepper <drepper@redhat.com>, 2003.
> +
> + 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 <http://www.gnu.org/licenses/>. */
> +
> +#include <errno.h>
> +#include <stdlib.h>
> +#include <time.h>
> +#include <sysdep.h>
> +
> +int
> +timerfd_settime (int __ufd, int __flags, const struct itimerspec *__utmr,
> + struct itimerspec *__otmr)
> +{
> +#ifdef __ASSUME_TIME64_SYSCALLS
> + /* Delete the kernel timer object. */
> + return INLINE_SYSCALL (timerfd_settime64, 4, __ufd, __flags, __utmr, __otmr);
> +#else
> + int ret;
> +# ifdef __NR_timerfd_settime64
> +# if __TIMESIZE == 64
> + ret = INLINE_SYSCALL (timerfd_settime64, 4, __ufd, __flags, __utmr, __otmr);
> +
> + if (ret == 0 || errno != ENOSYS)
> + {
> + return ret;
> + }
> +# else
> + struct __itimerspec64 ts64;
> + ret = INLINE_SYSCALL (timerfd_settime64, 4, __ufd, __flags, __utmr, &ts64);
> +
> + if (ret == 0 || errno != ENOSYS)
> + {
> + __utmr->it_interval.tv_sec = ts64.it_interval.tv_sec
> + __utmr->it_interval.tv_nsec = ts64.it_interval.tv_nsec
> + if (! in_time_t_range (__utmr->it_interval.tv_sec))
> + {
> + __set_errno (EOVERFLOW);
> + return -1;
> + }
> +
> + __utmr->it_value.tv_sec = ts64.it_value.tv_sec
> + __utmr->it_value.tv_nsec = ts64.it_value.tv_nsec
> + if (! in_time_t_range (__utmr->it_value.tv_sec))
> + {
> + __set_errno (EOVERFLOW);
> + return -1;
> + }
> +
> + return 0;
> + }
> +# endif /* __TIMESIZE == 64 */
> +# endif /* __NR_timerfd_settime */
> +# if __TIMESIZE == 64
> + struct itimerspec ts32;
> +
> + if (! in_time_t_range (__utmr->it_interval.tv_sec) ||
> + ! in_time_t_range (__utmr->it_value.tv_sec))
> + {
> + __set_errno (EOVERFLOW);
> + return -1;
> + }
> +
> + ret = INLINE_SYSCALL (timerfd_settime, 4, __ufd, __flags, __utmr, &ts32);
> +
> + if (ret == 0 || errno != ENOSYS)
> + {
> + __utmr->it_interval.tv_sec = ts32.it_interval.tv_sec
> + __utmr->it_interval.tv_nsec = ts32.it_interval.tv_nsec
> + if (! in_time_t_range (__utmr->it_interval.tv_sec))
> + {
> + __set_errno (EOVERFLOW);
> + return -1;
> + }
> +
> + __utmr->it_value.tv_sec = ts32.it_value.tv_sec
> + __utmr->it_value.tv_nsec = ts32.it_value.tv_nsec
> + if (! in_time_t_range (__utmr->it_value.tv_sec))
> + {
> + __set_errno (EOVERFLOW);
> + return -1;
> + }
> +
> + return 0;
> + }
> + return ret;
> +# else
> + return INLINE_SYSCALL (timerfd_settime, 4, __ufd, __flags, __utmr, __otmr);
> +# endif /* __TIMESIZE == 64 */
> +#endif /* __ASSUME_TIME64_SYSCALLS */
> +}
> --
> 2.22.0
>
On Fri, 9 Aug 2019, Alistair Francis wrote:
> diff --git a/sysdeps/unix/sysv/linux/sys/timerfd.h b/sysdeps/unix/sysv/linux/sys/timerfd.h
> index 5e5ad351a0d..51f63b357c1 100644
> --- a/sysdeps/unix/sysv/linux/sys/timerfd.h
> +++ b/sysdeps/unix/sysv/linux/sys/timerfd.h
> @@ -24,6 +24,13 @@
> /* Get the platform-dependent flags. */
> #include <bits/timerfd.h>
>
> +#if __TIMESIZE == 32
> +struct __itimerspec64
> + {
> + struct __timespec64 it_interval;
> + struct __timespec64 it_value;
> + };
> +#endif
This does not belong in an installed header (until we have actual
_TIME_BITS support added to all installed headers).
> enum
> @@ -40,16 +47,16 @@ __BEGIN_DECLS
> /* Return file descriptor for new interval timer source. */
> extern int timerfd_create (__clockid_t __clock_id, int __flags) __THROW;
>
> -/* Set next expiration time of interval timer source UFD to UTMR. If
> - FLAGS has the TFD_TIMER_ABSTIME flag set the timeout value is
> - absolute. Optionally return the old expiration time in OTMR. */
> -extern int timerfd_settime (int __ufd, int __flags,
> - const struct itimerspec *__utmr,
> - struct itimerspec *__otmr) __THROW;
> -
> /* Return the next expiration time of UFD. */
> extern int timerfd_gettime (int __ufd, struct itimerspec *__otmr) __THROW;
>
> __END_DECLS
>
> +/* Set next expiration time of interval timer source UFD to UTMR. If
> + FLAGS has the TFD_TIMER_ABSTIME flag set the timeout value is
> + absolute. Optionally return the old expiration time in OTMR. */
> +int timerfd_settime (int __ufd, int __flags,
> + const struct itimerspec *__utmr,
> + struct itimerspec *__otmr) __THROW;
This move is wrong. All function declarations in installed headers must
be between __BEGIN_DECLS and __END_DECLS so they have C linkage in C++.
This patch has no commit message explaining what it's supposed to be doing
and why. Please include a proper explanation with *every* patch of what
it's doing, how it's been tested, etc.
> diff --git a/sysdeps/unix/sysv/linux/timerfd_settime.c b/sysdeps/unix/sysv/linux/timerfd_settime.c
> new file mode 100644
> index 00000000000..830faeada77
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/timerfd_settime.c
> @@ -0,0 +1,104 @@
> +/* Copyright (C) 2003-2019 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> + Contributed by Ulrich Drepper <drepper@redhat.com>, 2003.
We don't use "Contributed by" in new source files.
What file with copyrightable content dating from 2003 is this based on?
You should state that in your ChangeLog entry, if you're copying
significant content from another file (if not, of course the copyright
years should just be 2019).
On Fri, 9 Aug 2019, Alistair Francis wrote:
> This commit was clearly not tested very well (it's missing
> semicolons), just see it more as pseudo code.
[...]
> > diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> > index 1ab6bcbfc81..89128e80868 100644
> > --- a/sysdeps/unix/sysv/linux/Makefile
> > +++ b/sysdeps/unix/sysv/linux/Makefile
> > @@ -19,6 +19,8 @@ sysdep_routines += clone umount umount2 readahead \
> > eventfd eventfd_read eventfd_write prlimit \
> > personality epoll_wait tee vmsplice splice \
> > open_by_handle_at mlock2 pkey_mprotect pkey_set pkey_get
Missing trailing backslash here too, which breaks building.
> > + timerfd_settime
Maciej
@@ -19,6 +19,8 @@ sysdep_routines += clone umount umount2 readahead \
eventfd eventfd_read eventfd_write prlimit \
personality epoll_wait tee vmsplice splice \
open_by_handle_at mlock2 pkey_mprotect pkey_set pkey_get
+ timerfd_settime
+
CFLAGS-gethostid.c = -fexceptions
CFLAGS-tee.c = -fexceptions -fasynchronous-unwind-tables
@@ -24,6 +24,13 @@
/* Get the platform-dependent flags. */
#include <bits/timerfd.h>
+#if __TIMESIZE == 32
+struct __itimerspec64
+ {
+ struct __timespec64 it_interval;
+ struct __timespec64 it_value;
+ };
+#endif
/* Bits to be set in the FLAGS parameter of `timerfd_settime'. */
enum
@@ -40,16 +47,16 @@ __BEGIN_DECLS
/* Return file descriptor for new interval timer source. */
extern int timerfd_create (__clockid_t __clock_id, int __flags) __THROW;
-/* Set next expiration time of interval timer source UFD to UTMR. If
- FLAGS has the TFD_TIMER_ABSTIME flag set the timeout value is
- absolute. Optionally return the old expiration time in OTMR. */
-extern int timerfd_settime (int __ufd, int __flags,
- const struct itimerspec *__utmr,
- struct itimerspec *__otmr) __THROW;
-
/* Return the next expiration time of UFD. */
extern int timerfd_gettime (int __ufd, struct itimerspec *__otmr) __THROW;
__END_DECLS
+/* Set next expiration time of interval timer source UFD to UTMR. If
+ FLAGS has the TFD_TIMER_ABSTIME flag set the timeout value is
+ absolute. Optionally return the old expiration time in OTMR. */
+int timerfd_settime (int __ufd, int __flags,
+ const struct itimerspec *__utmr,
+ struct itimerspec *__otmr) __THROW;
+
#endif /* sys/timerfd.h */
@@ -96,7 +96,6 @@ fremovexattr - fremovexattr i:is fremovexattr
mq_setattr - mq_getsetattr i:ipp mq_setattr
timerfd_create EXTRA timerfd_create i:ii timerfd_create
-timerfd_settime EXTRA timerfd_settime64 i:iipp timerfd_settime
timerfd_gettime EXTRA timerfd_gettime64 i:ip timerfd_gettime
fanotify_init EXTRA fanotify_init i:ii fanotify_init
new file mode 100644
@@ -0,0 +1,104 @@
+/* Copyright (C) 2003-2019 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+ Contributed by Ulrich Drepper <drepper@redhat.com>, 2003.
+
+ 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 <http://www.gnu.org/licenses/>. */
+
+#include <errno.h>
+#include <stdlib.h>
+#include <time.h>
+#include <sysdep.h>
+
+int
+timerfd_settime (int __ufd, int __flags, const struct itimerspec *__utmr,
+ struct itimerspec *__otmr)
+{
+#ifdef __ASSUME_TIME64_SYSCALLS
+ /* Delete the kernel timer object. */
+ return INLINE_SYSCALL (timerfd_settime64, 4, __ufd, __flags, __utmr, __otmr);
+#else
+ int ret;
+# ifdef __NR_timerfd_settime64
+# if __TIMESIZE == 64
+ ret = INLINE_SYSCALL (timerfd_settime64, 4, __ufd, __flags, __utmr, __otmr);
+
+ if (ret == 0 || errno != ENOSYS)
+ {
+ return ret;
+ }
+# else
+ struct __itimerspec64 ts64;
+ ret = INLINE_SYSCALL (timerfd_settime64, 4, __ufd, __flags, __utmr, &ts64);
+
+ if (ret == 0 || errno != ENOSYS)
+ {
+ __utmr->it_interval.tv_sec = ts64.it_interval.tv_sec
+ __utmr->it_interval.tv_nsec = ts64.it_interval.tv_nsec
+ if (! in_time_t_range (__utmr->it_interval.tv_sec))
+ {
+ __set_errno (EOVERFLOW);
+ return -1;
+ }
+
+ __utmr->it_value.tv_sec = ts64.it_value.tv_sec
+ __utmr->it_value.tv_nsec = ts64.it_value.tv_nsec
+ if (! in_time_t_range (__utmr->it_value.tv_sec))
+ {
+ __set_errno (EOVERFLOW);
+ return -1;
+ }
+
+ return 0;
+ }
+# endif /* __TIMESIZE == 64 */
+# endif /* __NR_timerfd_settime */
+# if __TIMESIZE == 64
+ struct itimerspec ts32;
+
+ if (! in_time_t_range (__utmr->it_interval.tv_sec) ||
+ ! in_time_t_range (__utmr->it_value.tv_sec))
+ {
+ __set_errno (EOVERFLOW);
+ return -1;
+ }
+
+ ret = INLINE_SYSCALL (timerfd_settime, 4, __ufd, __flags, __utmr, &ts32);
+
+ if (ret == 0 || errno != ENOSYS)
+ {
+ __utmr->it_interval.tv_sec = ts32.it_interval.tv_sec
+ __utmr->it_interval.tv_nsec = ts32.it_interval.tv_nsec
+ if (! in_time_t_range (__utmr->it_interval.tv_sec))
+ {
+ __set_errno (EOVERFLOW);
+ return -1;
+ }
+
+ __utmr->it_value.tv_sec = ts32.it_value.tv_sec
+ __utmr->it_value.tv_nsec = ts32.it_value.tv_nsec
+ if (! in_time_t_range (__utmr->it_value.tv_sec))
+ {
+ __set_errno (EOVERFLOW);
+ return -1;
+ }
+
+ return 0;
+ }
+ return ret;
+# else
+ return INLINE_SYSCALL (timerfd_settime, 4, __ufd, __flags, __utmr, __otmr);
+# endif /* __TIMESIZE == 64 */
+#endif /* __ASSUME_TIME64_SYSCALLS */
+}