[RFC,v4,24/24] timerfd_settime: Use 64-bit call if avaliable
diff mbox series

Message ID ef25779c773ca08b586f27ee80d61acde0b9bd7f.1565398514.git.alistair.francis@wdc.com
State New
Headers show
Series
  • RISC-V glibc port for the 32-bit
Related show

Commit Message

Alistair Francis Aug. 10, 2019, 1 a.m. UTC
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

Alistair Francis Aug. 10, 2019, 1:27 a.m. UTC | #1
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
>
Joseph Myers Aug. 12, 2019, 8:08 p.m. UTC | #2
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).
Maciej W. Rozycki Aug. 20, 2019, 8:11 p.m. UTC | #3
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

Patch
diff mbox series

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 */
+}