This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC v5 02/21] sysdeps/nanosleep: Use clock_nanosleep_time64 if avaliable


On Mon, Oct 14, 2019 at 7:00 AM Lukasz Majewski <lukma@denx.de> wrote:
>
> Hi Alistair,
>
> > The nanosleep syscall is not supported on newer 32-bit platforms (such
> > as RV32). To fix this issue let's use clock_nanosleep_time64 if it is
> > avaliable.
> >
> > Let's use CLOCK_REALTIME when calling clock_nanosleep_time64 as the
> > Linux specification says:
> >   "POSIX.1 specifies that nanosleep() should measure time against the
> >    CLOCK_REALTIME clock. However, Linux measures the time using the
> >    CLOCK_MONOTONIC clock. This probably does not matter, since the
> > POSIX.1 specification for clock_settime(2) says that discontinuous
> > changes in CLOCK_REALTIME should not affect nanosleep()"
> >
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> >
> >       * nptl/thrd_sleep.c: Use clock_nanosleep_time64 instead of
> > nanosleep.
> >       * sysdeps/unix/sysv/linux/nanosleep.c: Likewise.
> >       * sysdeps/unix/sysv/linux/clock_nanosleep.c: Likewise.
> >       * sysdeps/unix/sysv/linux/nanosleep_nocancel.c: Likewise.
> > ---
> >  include/time.h                               |  6 ++
> >  nptl/thrd_sleep.c                            | 69 ++++++++++++++++--
> >  sysdeps/unix/sysv/linux/clock_nanosleep.c    | 74
> > ++++++++++++++++++-- sysdeps/unix/sysv/linux/nanosleep.c          |
> > 65 ++++++++++++++++- sysdeps/unix/sysv/linux/nanosleep_nocancel.c |
> > 64 ++++++++++++++++- 5 files changed, 264 insertions(+), 14
> > deletions(-)
> >
> > diff --git a/include/time.h b/include/time.h
> > index 95b31429212..6d81f91384b 100644
> > --- a/include/time.h
> > +++ b/include/time.h
> > @@ -172,6 +172,12 @@ libc_hidden_proto (__difftime64)
> >
> >  extern double __difftime (time_t time1, time_t time0);
> >
> > +#if __TIMESIZE == 64
> > +#define __thrd_sleep_time64 thrd_sleep
> > +#define __clock_nanosleep_time64 __clock_nanosleep
> > +#define __nanosleep_time64 __nanosleep
> > +#define __nanosleep_nocancel_time64 __nanosleep_nocancel
> > +#endif
> >
> >  /* Use in the clock_* functions.  Size of the field representing the
> >     actual clock ID.  */
> > diff --git a/nptl/thrd_sleep.c b/nptl/thrd_sleep.c
> > index 07a51808df2..e85da561c68 100644
> > --- a/nptl/thrd_sleep.c
> > +++ b/nptl/thrd_sleep.c
> > @@ -22,18 +22,79 @@
> >  #include "thrd_priv.h"
> >
> >  int
> > -thrd_sleep (const struct timespec* time_point, struct timespec*
> > remaining) +__thrd_sleep_time64 (const struct timespec* time_point,
> > struct timespec* remaining) {
> >    INTERNAL_SYSCALL_DECL (err);
> > -  int ret = INTERNAL_SYSCALL_CANCEL (nanosleep, err, time_point,
> > remaining);
> > +  int ret = -1;
> > +
> > +#ifdef __ASSUME_TIME64_SYSCALLS
> > +# ifndef __NR_clock_nanosleep_time64
> > +#  define __NR_clock_nanosleep_time64 __NR_clock_nanosleep
> > +# endif
> > +  ret = INTERNAL_SYSCALL_CANCEL (clock_nanosleep_time64, err,
> > CLOCK_REALTIME,
> > +                                 0, time_point, remaining);
> > +#else
> > +# ifdef __NR_clock_nanosleep_time64
> > +  long int ret_64;
> > +
> > +  ret_64 = INTERNAL_SYSCALL_CANCEL (clock_nanosleep_time64, err,
> > CLOCK_REALTIME,
> > +                                    0, time_point, remaining);
> > +
> > +  if (ret_64 == 0 || errno != ENOSYS)
> > +    ret = ret_64;
> > +# endif /* __NR_clock_nanosleep_time64 */
> > +  if (ret < 0)
> > +    {
> > +      struct timespec tp32, tr32;
> > +
> > +      if (! in_time_t_range (time_point->tv_sec))
> > +        {
> > +          __set_errno (EOVERFLOW);
> > +          return -1;
> > +        }
> > +
> > +      valid_timespec64_to_timespec (time_point, &tp32);
> > +      ret =  INTERNAL_SYSCALL_CANCEL (nanosleep, err, &tp32, &tr32);
> > +
> > +      if ((ret == 0 || errno != ENOSYS) && remaining)
> > +        valid_timespec_to_timespec64(&tr32, remaining);
> > +    }
> > +#endif /* __ASSUME_TIME64_SYSCALLS */
> > +
> >    if (INTERNAL_SYSCALL_ERROR_P (ret, err))
> >      {
> >        /* C11 states thrd_sleep function returns -1 if it has been
> > interrupted
> > -      by a signal, or a negative value if it fails.  */
> > +         by a signal, or a negative value if it fails.  */
> >        ret = INTERNAL_SYSCALL_ERRNO (ret, err);
> >        if (ret == EINTR)
> > -     return -1;
> > +        return -1;
> >        return -2;
> >      }
> >    return 0;
> >  }
> > +
> > +#if __TIMESIZE != 64
> > +int
> > +thrd_sleep (const struct timespec* time_point, struct timespec*
> > remaining) +{
> > +  int ret;
> > +  timespec64 tp64, tr64;
> > +
> > +  valid_timespec_to_timespec64(time_point, &tp64);
> > +  ret = __thrd_sleep_time64 (&tp64, &tr64);
> > +
> > +  if (ret == 0 || errno != ENOSYS)
> > +    {
> > +      if (! in_time_t_range (tr64->tv_sec))
> > +        {
> > +          __set_errno (EOVERFLOW);
> > +          return -1;
> > +        }
> > +
> > +      if (remaining)
> > +        valid_timespec64_to_timespec(&tr32, remaining);
> > +    }
> > +
> > +  return ret;
> > +}
> > +#endif
>
> This code seems to follow the pattern introduced in clock_settime()
> conversion (already in the -master branch).

I hope so :)

>
> Do you plan to send this and the following patches:
>
> https://patchwork.ozlabs.org/patch/1155396/    --> clock_gettime
> https://patchwork.ozlabs.org/patch/1155398/    --> timespec_get

Yes

>
> As a separate patch series so we can all benefit from having them in
> the -master glibc branch?
> Those patches seems to be orthogonal to ones adding support for RISC-V.

Yep! My plan is to send them seperatley. I have been working through
the RV32 tree sending patches in small batches to get them merged. I
have a few more batches before I get to these time64_t conversion
patches though. I am unfortunately seeing failures with my time64_t
conversion patches which I haven't had a chance to look into.

>
>
> The gettimeofday() syscall handling code seems to being now converted to
> clock_gettime() in the glibc (or at least there is some ongoing effort):
>
> https://sourceware.org/git/?p=glibc.git;a=commit;h=b30d257ea8f557bdadca09f5b112538e7b807eb9

Yep, I have rebased all of my work on top of this.

>
> Similar situation is with stat and statfs (which are now under the
> discussion/review):
>
> https://patchwork.ozlabs.org/patch/1155399/    --> stat
> https://patchwork.ozlabs.org/patch/1155401/    --> statfs

Yep, I'm working on the next version right now.

If you want you are welcome to split out any patches and get them
upstreaed. Even just fixing failing test cases would speed things up.
My branch is available here:
https://github.com/alistair23/glibc/commits/alistair/rv32.next

Alistair

>
> > diff --git a/sysdeps/unix/sysv/linux/clock_nanosleep.c
> > b/sysdeps/unix/sysv/linux/clock_nanosleep.c index
> > 0cb6614dc92..64302fbcc69 100644 ---
> > a/sysdeps/unix/sysv/linux/clock_nanosleep.c +++
> > b/sysdeps/unix/sysv/linux/clock_nanosleep.c @@ -21,13 +21,14 @@
> >  #include <sysdep-cancel.h>
> >  #include "kernel-posix-cpu-timers.h"
> >
> > -
> >  /* We can simply use the syscall.  The CPU clocks are not supported
> >     with this function.  */
> >  int
> > -__clock_nanosleep (clockid_t clock_id, int flags, const struct
> > timespec *req,
> > -                struct timespec *rem)
> > +__clock_nanosleep_time64 (clockid_t clock_id, int flags, const
> > struct timespec *req,
> > +                          struct timespec *rem)
> >  {
> > +  int r = -1;
> > +
> >    if (clock_id == CLOCK_THREAD_CPUTIME_ID)
> >      return EINVAL;
> >    if (clock_id == CLOCK_PROCESS_CPUTIME_ID)
> > @@ -36,9 +37,70 @@ __clock_nanosleep (clockid_t clock_id, int flags,
> > const struct timespec *req, /* If the call is interrupted by a signal
> > handler or encounters an error, it returns a positive value similar
> > to errno.  */ INTERNAL_SYSCALL_DECL (err);
> > -  int r = INTERNAL_SYSCALL_CANCEL (clock_nanosleep, err, clock_id,
> > flags,
> > -                                req, rem);
> > +
> > +#ifdef __ASSUME_TIME64_SYSCALLS
> > +# ifndef __NR_clock_nanosleep_time64
> > +#  define __NR_clock_nanosleep_time64 __NR_clock_nanosleep
> > +# endif
> > +  r = INTERNAL_SYSCALL_CANCEL (clock_nanosleep_time64, err, clock_id,
> > +                               flags, req, rem);
> > +#else
> > +# ifdef __NR_clock_nanosleep_time64
> > +  long int ret_64;
> > +
> > +  ret_64 = INTERNAL_SYSCALL_CANCEL (clock_nanosleep_time64, err,
> > clock_id,
> > +                                    flags, req, rem);
> > +
> > +  if (ret_64 == 0 || errno != ENOSYS)
> > +    r = ret_64;
> > +# endif /* __NR_clock_nanosleep_time64 */
> > +  if (r < 0)
> > +    {
> > +      struct timespec ts32, tr32;
> > +
> > +      if (! in_time_t_range (req->tv_sec))
> > +        {
> > +          __set_errno (EOVERFLOW);
> > +          return -1;
> > +        }
> > +
> > +      valid_timespec64_to_timespec (req, &ts32);
> > +      r =  INTERNAL_SYSCALL_CANCEL (clock_nanosleep, err, &ts32,
> > &tr32); +
> > +      if ((r == 0 || errno != ENOSYS) && rem)
> > +        valid_timespec_to_timespec64(&tr32, rem);
> > +    }
> > +#endif /* __ASSUME_TIME64_SYSCALLS */
> > +
> >    return (INTERNAL_SYSCALL_ERROR_P (r, err)
> > -       ? INTERNAL_SYSCALL_ERRNO (r, err) : 0);
> > +          ? INTERNAL_SYSCALL_ERRNO (r, err) : 0);
> >  }
> > +
> > +#if __TIMESIZE != 64
> > +int
> > +__clock_nanosleep (clockid_t clock_id, int flags, const struct
> > timespec *req,
> > +                   struct timespec *rem)
> > +{
> > +  int r;
> > +  timespec64 treq64, trem64;
> > +
> > +  valid_timespec_to_timespec64(req, &treq64)
> > +  r = __clock_nanosleep_time64 (clock_id, flags, &treq64, &trem64);
> > +
> > +  if (r == 0 || errno != ENOSYS)
> > +    {
> > +      if (! in_time_t_range (trem64->tv_sec))
> > +        {
> > +          __set_errno (EOVERFLOW);
> > +          return -1;
> > +        }
> > +
> > +      if (remaining)
> > +        valid_timespec64_to_timespec(&tr32, remaining);
> > +    }
> > +
> > +  return r;
> > +}
> > +#endif
> > +
> >  weak_alias (__clock_nanosleep, clock_nanosleep)
> > diff --git a/sysdeps/unix/sysv/linux/nanosleep.c
> > b/sysdeps/unix/sysv/linux/nanosleep.c index f14ae565af5..9a8dd5a4b82
> > 100644 --- a/sysdeps/unix/sysv/linux/nanosleep.c
> > +++ b/sysdeps/unix/sysv/linux/nanosleep.c
> > @@ -22,10 +22,71 @@
> >
> >  /* Pause execution for a number of nanoseconds.  */
> >  int
> > +__nanosleep_time64 (const struct timespec *requested_time,
> > +                    struct timespec *remaining)
> > +{
> > +#if defined(__ASSUME_TIME64_SYSCALLS)
> > +# ifndef __NR_clock_nanosleep_time64
> > +#  define __NR_clock_nanosleep_time64 __NR_clock_nanosleep
> > +# endif
> > +  return SYSCALL_CANCEL (clock_nanosleep_time64, CLOCK_REALTIME, 0,
> > +                         requested_time, remaining);
> > +#else
> > +# ifdef __NR_clock_nanosleep_time64
> > +  long int ret_64;
> > +
> > +  ret_64 = SYSCALL_CANCEL (clock_nanosleep_time64, CLOCK_REALTIME, 0,
> > +                           requested_time, remaining);
> > +
> > +  if (ret_64 == 0 || errno != ENOSYS)
> > +    return ret_64;
> > +# endif /* __NR_clock_nanosleep_time64 */
> > +  int ret;
> > +  struct timespec ts32, tr32;
> > +
> > +  if (! in_time_t_range (requested_time->tv_sec))
> > +    {
> > +      __set_errno (EOVERFLOW);
> > +      return -1;
> > +    }
> > +
> > +  valid_timespec64_to_timespec (requested_time, &ts32);
> > +  ret = SYSCALL_CANCEL (nanosleep, &ts32, &tr32);
> > +
> > +  if ((ret == 0 || errno != ENOSYS) && remaining)
> > +    valid_timespec_to_timespec64(&tr32, remaining);
> > +
> > +  return ret;
> > +#endif /* __ASSUME_TIME64_SYSCALLS */
> > +}
> > +
> > +#if __TIMESIZE != 64
> > +int
> >  __nanosleep (const struct timespec *requested_time,
> > -          struct timespec *remaining)
> > +             struct timespec *remaining)
> >  {
> > -  return SYSCALL_CANCEL (nanosleep, requested_time, remaining);
> > +  int r;
> > +  timespec64 treq64, trem64;
> > +
> > +
> > +  valid_timespec_to_timespec64(req, &treq64);
> > +  r = __nanosleep_time64 (&treq64, &trem64);
> > +
> > +  if (r == 0 || errno != ENOSYS)
> > +    {
> > +      if (! in_time_t_range (trem64->tv_sec))
> > +        {
> > +          __set_errno (EOVERFLOW);
> > +          return -1;
> > +        }
> > +
> > +      if (remaining)
> > +        valid_timespec64_to_timespec(&tr32, remaining);
> > +    }
> > +
> > +  return r;
> >  }
> > +#endif
> > +
> >  hidden_def (__nanosleep)
> >  weak_alias (__nanosleep, nanosleep)
> > diff --git a/sysdeps/unix/sysv/linux/nanosleep_nocancel.c
> > b/sysdeps/unix/sysv/linux/nanosleep_nocancel.c index
> > 122ba627ff3..03f5be2ff9e 100644 ---
> > a/sysdeps/unix/sysv/linux/nanosleep_nocancel.c +++
> > b/sysdeps/unix/sysv/linux/nanosleep_nocancel.c @@ -20,10 +20,70 @@
> >  #include <sysdep-cancel.h>
> >  #include <not-cancel.h>
> >
> > +int
> > +__nanosleep_nocancel_time64 (const struct timespec *requested_time,
> > +                             struct timespec *remaining)
> > +{
> > +#ifdef __ASSUME_TIME64_SYSCALLS
> > +# ifndef __NR_clock_nanosleep_time64
> > +#  define __NR_clock_nanosleep_time64 __NR_clock_nanosleep
> > +# endif
> > +  return INLINE_SYSCALL_CALL (clock_nanosleep_time64,
> > CLOCK_REALTIME, 0,
> > +                              requested_time, remaining);
> > +#else
> > +# ifdef __NR_clock_nanosleep_time64
> > +  long int ret_64;
> > +
> > +  ret_64 = INLINE_SYSCALL_CALL (clock_nanosleep_time64,
> > CLOCK_REALTIME, 0,
> > +                                requested_time, remaining);
> > +
> > +  if (ret_64 == 0 || errno != ENOSYS)
> > +    return ret_64;
> > +# endif /* __NR_clock_nanosleep_time64 */
> > +  int ret;
> > +  struct timespec ts32, tr32;
> > +
> > +  if (! in_time_t_range (requested_time->tv_sec))
> > +    {
> > +      __set_errno (EOVERFLOW);
> > +      return -1;
> > +    }
> > +
> > +  valid_timespec64_to_timespec (requested_time, &ts32);
> > +  ret = INLINE_SYSCALL_CALL (nanosleep, &ts32, &tr32);
> > +
> > +  if (ret == 0 || errno != ENOSYS)
> > +    valid_timespec_to_timespec64(&tr32, remaining);
> > +
> > +  return ret;
> > +#endif /* __ASSUME_TIME64_SYSCALLS */
> > +}
> > +
> > +#if __TIMESIZE != 64
> >  int
> >  __nanosleep_nocancel (const struct timespec *requested_time,
> > -                   struct timespec *remaining)
> > +                      struct timespec *remaining)
> >  {
> > -  return INLINE_SYSCALL_CALL (nanosleep, requested_time, remaining);
> > +  int ret;
> > +  timespec64 treq64, trem64;
> > +
> > +  valid_timespec_to_timespec64(req, &treq64)
> > +  ret = __nanosleep_nocancel_time64 (&treq64, &trem64);
> > +
> > +  if (ret == 0 || errno != ENOSYS)
> > +    {
> > +      if (! in_time_t_range (trem64->tv_sec))
> > +        {
> > +          __set_errno (EOVERFLOW);
> > +          return -1;
> > +        }
> > +
> > +      if (remaining)
> > +        valid_timespec64_to_timespec(&tr32, remaining);
> > +    }
> > +
> > +  return ret;
> >  }
> > +#endif
> > +
> >  hidden_def (__nanosleep_nocancel)
>
>
>
>
> 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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]