[v2,07/10] Use clock_gettime to implement timespec_get.

  timespec_get is the same function as clock_gettime, with an obnoxious
coating of NIH painted on it by the ISO C committee.  In addition to
the rename, it takes its arguments in a different order, it returns 0
on *failure* or a positive number on *success*, and it requires that
all of its TIME_* constants be positive.  This last means we cannot
directly reuse the existing CLOCK_* constants for it, because
those have been allocated starting with CLOCK_REALTIME = 0 on all
existing platforms.

As such I think the most sensible GNUish policy for this function is
that we will provide it, but we will not recommend its use, we will
not go out of our way to make it efficient, and we will not attempt to
extend the set of TIME_* constants beyond the ones defined in ISO
C (and in POSIX, if it ever appears in POSIX).

This patch simply promotes the sysdeps/posix implementation to
universal, and removes the Linux-specific implementation, whose
apparent reason for existing was to cut out one function call's worth
of overhead.  Oh well.

	* sysdeps/posix/timespec_get.c
	* sysdeps/unix/sysv/linux/timespec_get.c: Delete file.
	* time/timespec_get.c: No longer a stub.  Replace implementation
	with the code formerly in sysdeps/posix/timespec_get.c.
---
 sysdeps/posix/timespec_get.c           | 32 ------------------
 sysdeps/unix/sysv/linux/timespec_get.c | 46 --------------------------
 time/timespec_get.c                    | 14 +++-----
 3 files changed, 4 insertions(+), 88 deletions(-)
 delete mode 100644 sysdeps/posix/timespec_get.c
 delete mode 100644 sysdeps/unix/sysv/linux/timespec_get.c
  

On 28/08/2019 12:32, Zack Weinberg wrote:
> timespec_get is the same function as clock_gettime, with an obnoxious
> coating of NIH painted on it by the ISO C committee.  In addition to
> the rename, it takes its arguments in a different order, it returns 0
> on *failure* or a positive number on *success*, and it requires that
> all of its TIME_* constants be positive.  This last means we cannot
> directly reuse the existing CLOCK_* constants for it, because
> those have been allocated starting with CLOCK_REALTIME = 0 on all
> existing platforms.
> 
> As such I think the most sensible GNUish policy for this function is
> that we will provide it, but we will not recommend its use, we will
> not go out of our way to make it efficient, and we will not attempt to
> extend the set of TIME_* constants beyond the ones defined in ISO
> C (and in POSIX, if it ever appears in POSIX).

I don't fully agree we should not optimize because of the mentioned reasons.
Like or not, it is the C11 standard function to get the timer in a system
independent way and I would expect that once more systems implement it
portable programs will rely on it (it seems that even Windows now support
it).

And I think we can still optimize it without adding much complexity on the
code base. One idea is to simple provide a system specific internal
header that on Linux will issue the INTERNAL_VSYSCAL as an inline
function. Something like:

--
#ifdef HAVE_CLOCK_GETTIME_VSYSCALL
# define HAVE_VSYSCALL
#endif
#include <sysdep-vdso.h>

static inline int
internal_clock_gettime (clockid_t clk_id, struct timespec *tp)
{
  return INLINE_VSYSCALL (clock_gettime, 2, clock_id, tp);
}
--

I think for timer function *specifically*, some users expects it has a low
latency for some usage patterns and I think we should provide if it is
feasible.

> 
> This patch simply promotes the sysdeps/posix implementation to
> universal, and removes the Linux-specific implementation, whose
> apparent reason for existing was to cut out one function call's worth
> of overhead.  Oh well.
> 
> 	* sysdeps/posix/timespec_get.c
> 	* sysdeps/unix/sysv/linux/timespec_get.c: Delete file.
> 	* time/timespec_get.c: No longer a stub.  Replace implementation
> 	with the code formerly in sysdeps/posix/timespec_get.c.
> ---
>  sysdeps/posix/timespec_get.c           | 32 ------------------
>  sysdeps/unix/sysv/linux/timespec_get.c | 46 --------------------------
>  time/timespec_get.c                    | 14 +++-----
>  3 files changed, 4 insertions(+), 88 deletions(-)
>  delete mode 100644 sysdeps/posix/timespec_get.c
>  delete mode 100644 sysdeps/unix/sysv/linux/timespec_get.c
> 
> diff --git a/sysdeps/posix/timespec_get.c b/sysdeps/posix/timespec_get.c
> deleted file mode 100644
> index 781a624c7d..0000000000
> --- a/sysdeps/posix/timespec_get.c
> +++ /dev/null
> @@ -1,32 +0,0 @@
> -/* timespec_get -- C11 interface to sample a clock.  Generic POSIX.1 version.
> -   Copyright (C) 2013-2019 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; if not, see
> -   <http://www.gnu.org/licenses/>.  */
> -
> -#include <time.h>
> -
> -
> -/* Set TS to calendar time based in time base BASE.  */
> -int
> -timespec_get (struct timespec *ts, int base)
> -{
> -  if (base == TIME_UTC)
> -    {
> -      __clock_gettime (CLOCK_REALTIME, ts);
> -      return base;
> -    }
> -  return 0;
> -}

Ok.

> diff --git a/sysdeps/unix/sysv/linux/timespec_get.c b/sysdeps/unix/sysv/linux/timespec_get.c
> deleted file mode 100644
> index 52080ddf08..0000000000
> --- a/sysdeps/unix/sysv/linux/timespec_get.c
> +++ /dev/null
> @@ -1,46 +0,0 @@
> -/* Copyright (C) 2011-2019 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.
> -
> -   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; if not, see
> -   <http://www.gnu.org/licenses/>.  */
> -
> -#include <time.h>
> -#include <sysdep.h>
> -#include <errno.h>
> -
> -#ifdef HAVE_CLOCK_GETTIME_VSYSCALL
> -# define HAVE_VSYSCALL
> -#endif
> -#include <sysdep-vdso.h>
> -
> -/* Set TS to calendar time based in time base BASE.  */
> -int
> -timespec_get (struct timespec *ts, int base)
> -{
> -  switch (base)
> -    {
> -      int res;
> -      INTERNAL_SYSCALL_DECL (err);
> -    case TIME_UTC:
> -      res = INTERNAL_VSYSCALL (clock_gettime, err, 2, CLOCK_REALTIME, ts);
> -      if (INTERNAL_SYSCALL_ERROR_P (res, err))
> -	return 0;
> -      break;
> -
> -    default:
> -      return 0;
> -    }
> -
> -  return base;
> -}

As before.

> diff --git a/time/timespec_get.c b/time/timespec_get.c
> index c9e58be5d8..5ac765add1 100644
> --- a/time/timespec_get.c
> +++ b/time/timespec_get.c
> @@ -22,16 +22,10 @@
>  int
>  timespec_get (struct timespec *ts, int base)
>  {
> -  switch (base)
> +  if (base == TIME_UTC)
>      {
> -    case TIME_UTC:
> -      /* Not supported.  */
> -      return 0;
> -
> -    default:
> -      return 0;
> +      __clock_gettime (CLOCK_REALTIME, ts);
> +      return base;
>      }
> -
> -  return base;
> +  return 0;
>  }
> -stub_warning (timespec_get)
> 

Ok.