[RFC,v5,05/21] sysdeps/clock_gettime: Use clock_gettime64 if avaliable

  Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

	* sysdeps/unix/sysv/linux/clock_gettime.c: Use clock_gettime64 if avaliable.
---
 include/time.h                          |  1 +
 sysdeps/unix/sysv/linux/clock_gettime.c | 48 ++++++++++++++++++++++++-
 2 files changed, 48 insertions(+), 1 deletion(-)
  

Hi Alistair,

> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> 
> 	* sysdeps/unix/sysv/linux/clock_gettime.c: Use
> clock_gettime64 if avaliable. ---
>  include/time.h                          |  1 +
>  sysdeps/unix/sysv/linux/clock_gettime.c | 48
> ++++++++++++++++++++++++- 2 files changed, 48 insertions(+), 1
> deletion(-)
> 
> diff --git a/include/time.h b/include/time.h
> index 6d81f91384b..1e33f34e1f6 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -177,6 +177,7 @@ extern double __difftime (time_t time1, time_t
> time0); #define __clock_nanosleep_time64 __clock_nanosleep
>  #define __nanosleep_time64 __nanosleep
>  #define __nanosleep_nocancel_time64 __nanosleep_nocancel
> +#define __clock_gettime64 __clock_gettime
>  #endif
>  
>  /* Use in the clock_* functions.  Size of the field representing the
> diff --git a/sysdeps/unix/sysv/linux/clock_gettime.c
> b/sysdeps/unix/sysv/linux/clock_gettime.c index
> 5fc47fb7dc7..ea98af9bf1a 100644 ---
> a/sysdeps/unix/sysv/linux/clock_gettime.c +++
> b/sysdeps/unix/sysv/linux/clock_gettime.c @@ -28,9 +28,55 @@
>  
>  /* Get current value of CLOCK and store it in TP.  */
>  int
> +__clock_gettime64 (clockid_t clock_id, struct timespec *tp)
> +{
> +#ifdef __ASSUME_TIME64_SYSCALLS
> +# ifndef __NR_clock_gettime64
> +#  define __NR_clock_gettime64 __NR_clock_gettime
> +# endif
> +   return INLINE_VSYSCALL (clock_gettime64, 2, clock_id, tp);
> +#else
> +   int ret;
> +# ifdef __NR_clock_gettime64
> +  ret = INLINE_VSYSCALL (clock_gettime64, 2, clock_id, tp);
> +
> +  if (ret == 0 || errno != ENOSYS)
> +    return ret;
> +# endif /* __NR_clock_gettime64 */
> +  struct timespec tp32;
> +
> +  ret = INLINE_VSYSCALL (clock_gettime, 2, clock_id, &tp32);
> +
> +  if (ret == 0 || errno != ENOSYS)
> +    valid_timespec_to_timespec64(tp32, tp);
> +
> +  return ret;
> +#endif /* __ASSUME_TIME64_SYSCALLS */
> +}
> +
> +#if __TIMESIZE != 64
> +int
>  __clock_gettime (clockid_t clock_id, struct timespec *tp)
>  {
> -  return INLINE_VSYSCALL (clock_gettime, 2, clock_id, tp);
> +  int ret;
> +  struct __timespec64 tp64;
> +
> +  ret = __clock_gettime64 (clock_id, &tp64);
> +
> +  if (ret == 0 || errno != ENOSYS)
> +    {
> +      if (! in_time_t_range (tp64.tv_sec))
> +        {
> +          __set_errno (EOVERFLOW);
> +          return -1;
> +        }
> +
> +      valid_timespec64_to_timespec(&tp64, tp);
> +
> +      return ret;
> +    }
>  }
> +#endif
> +
>  weak_alias (__clock_gettime, clock_gettime)
>  libc_hidden_def (__clock_gettime)

In your github repository - I've noticed the updated version of patch
providing __clock_gettime64 [1] conversion.

Do you plan to re-send this patch soon?


Please find some comments regarding this particular patch:

1. Please add following code instead the line 182 [2] (@include/time.h):

+#if __TIMESIZE == 64
+# define __clock_gettime64 __clock_gettime
+#else
+extern int __clock_gettime64 (clockid_t clock_id,
+                              struct __timespec64 *tp);
+libc_hidden_proto (__clock_gettime64);
+#endif


2. The line 44 (@sysv/linux/clock_gettime.c) is a bit tricky

I've grepped the recent kernel (-master branch) tag: v5.4-rc5 with
git grep -n "vdso_clock_gettime64"

And the __vdso_clock_gettime64 is available for mips, aarch64 and x86
(plain ARM 32 bits is missing though).

Considering the above - the vDSO may be a bit tricky for some archs as
the 'clock_gettime64' "name" argument to INLINE_VSYSCALL () macro is
then extended to __vdso_##name (@ sysdeps/unix/sysv/linux/sysdep-vdso.h)
It seems like one would need a global (i.e. at clock_gettime.c file)
__vdso_clock_gettime64 fallback.


IMHO the condition '&& defined HAVE_CLOCK_GETTIME64_VSYSCALL' in Line
44 [3] could be removed.


3. Line 50 - replace tp32 with ts32 (as we define structure, not
pointer).

4. Line 54 - IMHO the 'if (ret == 0 || errno != ENOSYS)' shall be
replaced with 'if (! ret && tp)' 
(as we reference *tp, which may be NULL). The same situation is in Line
70.


Links:
[1] -
https://github.com/alistair23/glibc/commit/b7682ca51f6d7ea702eeb9066c7710f5a40f6590

[2] -
https://github.com/alistair23/glibc/commit/b7682ca51f6d7ea702eeb9066c7710f5a40f6590#diff-5b9f1c6457e0e10079f657f283c19861R182

[3] -
https://github.com/alistair23/glibc/commit/b7682ca51f6d7ea702eeb9066c7710f5a40f6590#diff-cb7e59ab5b305ba007394bee79cdd6d6R44



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