Re: [RFC v3 04/23] sysdeps/clock_gettime: Use clock_gettime64 if avaliable

[Alistair Francis <alistair23 at gmail dot com>]

On Tue, Jul 16, 2019 at 10:39 PM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Alistair Francis:
>
> > diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
> > index 52c258e33d..8d9cae7d87 100644
> > --- a/nptl/pthread_mutex_timedlock.c
> > +++ b/nptl/pthread_mutex_timedlock.c
> > @@ -402,10 +402,17 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
> >                   /* Delay the thread until the timeout is reached.
> >                      Then return ETIMEDOUT.  */
> >                   struct timespec reltime;
> > +#ifdef __NR_clock_gettime64
> > +                 struct __timespec64 now;
> > +
> > +                 INTERNAL_SYSCALL (clock_gettime64, __err, 2, CLOCK_REALTIME,
> > +                                   &now);
> > +#else
> >                   struct timespec now;
> >
> >                   INTERNAL_SYSCALL (clock_gettime, __err, 2, clockid,
> >                                     &now);
> > +#endif
> >                   reltime.tv_sec = abstime->tv_sec - now.tv_sec;
> >                   reltime.tv_nsec = abstime->tv_nsec - now.tv_nsec;
> >                   if (reltime.tv_nsec < 0)
>
> I believe this needs to be updated for correctness (truncation of
> tv_sec) if ever ported to architectures where __nanosleep_nocancel takes
> a 32-bit time_t argument.  I don't know what our plans are regarding to
> that.
>
> If you had
>
> #define __NR_clock_gettime64  __NR_clock_gettime
>
> in <sysdep.h>, you wouldn't need this change.

Yep, you are right. I have used the #define and removed this change.

>
> > diff --git a/sysdeps/unix/sysv/linux/clock_gettime.c b/sysdeps/unix/sysv/linux/clock_gettime.c
> > index 5fc47fb7dc..4832069c34 100644
> > --- a/sysdeps/unix/sysv/linux/clock_gettime.c
> > +++ b/sysdeps/unix/sysv/linux/clock_gettime.c
> > @@ -27,10 +27,40 @@
> >  #include <sysdep-vdso.h>
> >
> >  /* Get current value of CLOCK and store it in TP.  */
> > +
> > +#if __WORDSIZE == 32
> > +int
> > +__clock_gettime (clockid_t clock_id, struct timespec *tp)
> > +{
> > +   int ret;
> > +
> > +#ifdef __NR_clock_gettime64
> > +  struct __timespec64 tp64;
> > +  ret = INLINE_VSYSCALL (clock_gettime64, 2, clock_id, &tp64);
> > +
> > +  tp->tv_sec = tp64.tv_sec;
> > +  tp->tv_nsec = tp64.tv_nsec;
> > +
> > +  if (! in_time_t_range (tp->tv_sec))
> > +    {
> > +      __set_errno (EOVERFLOW);
> > +      return -1;
> > +    }
> > +#endif
> > +
> > +#ifndef __ASSUME_TIME64_SYSCALLS
> > +  ret = INLINE_VSYSCALL (clock_gettime, 2, clock_id, tp);
> > +#endif
> > +
> > +  return ret;
> > +}
>
> I think this has the same problems as the timespec_get patch.

I'm still unclea what that problem is. Lukasz pointed out these don't
match the other time64_t conversions so I'll update it to use that
style anyway.

Alistair

>
> Thanks,
> Florian