[v2] y2038: nptl: Convert pthread_{clock|timed}join_np to support 64 bit time

  The pthread_clockjoin_np and pthread_timedjoin_np have been converted to
support 64 bit time.

This change introduces new futex_timed_wait_cancel64 function in
./sysdeps/nptl/futex-internal.h, which uses futex_time64 where possible
and tries to replace low-level preprocessor macros from
lowlevellock-futex.h
The pthread_{timed|clock}join_np only accept absolute time. Moreover,
there is no need to check for NULL passed as *abstime pointer as
clockwait_tid() always passes struct __timespec64.

For systems with __TIMESIZE != 64 && __WORDSIZE == 32:
- Conversions between 64 bit time to 32 bit are necessary
- Redirection to __pthread_{clock|timed}join_np64 will provide support
  for 64 bit time

Build tests:
./src/scripts/build-many-glibcs.py glibcs

Run-time tests:
- Run specific tests on ARM/x86 32bit systems (qemu):
  https://github.com/lmajewski/meta-y2038 and run tests:
  https://github.com/lmajewski/y2038-tests/commits/master

Above tests were performed with Y2038 redirection applied as well as without
to test the proper usage of both __pthread_{timed|clock}join_np64 and
__pthread_{timed|clock}join_np.

---
Changes for v2:
- Update the commit message
---
 nptl/pthreadP.h               | 16 +++++++++++++++-
 nptl/pthread_clockjoin.c      | 19 ++++++++++++++++---
 nptl/pthread_join_common.c    | 19 +++++++++++--------
 nptl/pthread_timedjoin.c      | 18 ++++++++++++++++--
 sysdeps/nptl/futex-internal.h | 31 +++++++++++++++++++++++++++++++
 5 files changed, 89 insertions(+), 14 deletions(-)
  

On 22/07/2020 04:37, Lukasz Majewski wrote:
> The pthread_clockjoin_np and pthread_timedjoin_np have been converted to
> support 64 bit time.
> 
> This change introduces new futex_timed_wait_cancel64 function in
> ./sysdeps/nptl/futex-internal.h, which uses futex_time64 where possible
> and tries to replace low-level preprocessor macros from
> lowlevellock-futex.h
> The pthread_{timed|clock}join_np only accept absolute time. Moreover,
> there is no need to check for NULL passed as *abstime pointer as
> clockwait_tid() always passes struct __timespec64.
> 
> For systems with __TIMESIZE != 64 && __WORDSIZE == 32:
> - Conversions between 64 bit time to 32 bit are necessary
> - Redirection to __pthread_{clock|timed}join_np64 will provide support
>   for 64 bit time
> 
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
> 
> Run-time tests:
> - Run specific tests on ARM/x86 32bit systems (qemu):
>   https://github.com/lmajewski/meta-y2038 and run tests:
>   https://github.com/lmajewski/y2038-tests/commits/master
> 
> Above tests were performed with Y2038 redirection applied as well as without
> to test the proper usage of both __pthread_{timed|clock}join_np64 and
> __pthread_{timed|clock}join_np.
> 
> ---
> Changes for v2:
> - Update the commit message

Some comments below regarding the futex-internal additions, the other
changes looks ok.

> ---
>  nptl/pthreadP.h               | 16 +++++++++++++++-
>  nptl/pthread_clockjoin.c      | 19 ++++++++++++++++---
>  nptl/pthread_join_common.c    | 19 +++++++++++--------
>  nptl/pthread_timedjoin.c      | 18 ++++++++++++++++--
>  sysdeps/nptl/futex-internal.h | 31 +++++++++++++++++++++++++++++++
>  5 files changed, 89 insertions(+), 14 deletions(-)
> 
> diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
> index 6f94d6be31..99713c8447 100644
> --- a/nptl/pthreadP.h
> +++ b/nptl/pthreadP.h
> @@ -458,6 +458,20 @@ extern int __pthread_cond_init (pthread_cond_t *cond,
>  libc_hidden_proto (__pthread_cond_init)
>  extern int __pthread_cond_signal (pthread_cond_t *cond);
>  extern int __pthread_cond_wait (pthread_cond_t *cond, pthread_mutex_t *mutex);
> +
> +#if __TIMESIZE == 64
> +# define __pthread_clockjoin_np64 __pthread_clockjoin_np
> +# define __pthread_timedjoin_np64 __pthread_timedjoin_np
> +#else
> +extern int __pthread_clockjoin_np64 (pthread_t threadid, void **thread_return,
> +                                     clockid_t clockid,
> +                                     const struct __timespec64 *abstime);
> +libc_hidden_proto (__pthread_clockjoin_np64)
> +extern int __pthread_timedjoin_np64 (pthread_t threadid, void **thread_return,
> +                                     const struct __timespec64 *abstime);
> +libc_hidden_proto (__pthread_timedjoin_np64)
> +#endif
> +
>  extern int __pthread_cond_timedwait (pthread_cond_t *cond,
>  				     pthread_mutex_t *mutex,
>  				     const struct timespec *abstime);
> @@ -488,7 +502,7 @@ extern int __pthread_enable_asynccancel (void) attribute_hidden;
>  extern void __pthread_disable_asynccancel (int oldtype) attribute_hidden;
>  extern void __pthread_testcancel (void);
>  extern int __pthread_clockjoin_ex (pthread_t, void **, clockid_t,
> -				   const struct timespec *, bool)
> +				   const struct __timespec64 *, bool)
>    attribute_hidden;
>  extern int __pthread_sigmask (int, const sigset_t *, sigset_t *);
>  libc_hidden_proto (__pthread_sigmask);

Ok.

> diff --git a/nptl/pthread_clockjoin.c b/nptl/pthread_clockjoin.c
> index a3e7f37e3b..3cd780f688 100644
> --- a/nptl/pthread_clockjoin.c
> +++ b/nptl/pthread_clockjoin.c
> @@ -16,14 +16,27 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> +#include <time.h>
>  #include "pthreadP.h"
>  
>  int
> -__pthread_clockjoin_np (pthread_t threadid, void **thread_return,
> -			clockid_t clockid,
> -			const struct timespec *abstime)
> +__pthread_clockjoin_np64 (pthread_t threadid, void **thread_return,
> +                          clockid_t clockid, const struct __timespec64 *abstime)
>  {
>    return __pthread_clockjoin_ex (threadid, thread_return,
>                                   clockid, abstime, true);
>  }
> +
> +#if __TIMESIZE != 64
> +libc_hidden_def (__pthread_clockjoin_np64)
> +
> +int
> +__pthread_clockjoin_np (pthread_t threadid, void **thread_return,
> +                        clockid_t clockid, const struct timespec *abstime)
> +{
> +  struct __timespec64 ts64 = valid_timespec_to_timespec64 (*abstime);
> +
> +  return __pthread_clockjoin_np64 (threadid, thread_return, clockid, &ts64);
> +}
> +#endif
>  weak_alias (__pthread_clockjoin_np, pthread_clockjoin_np)

Ok.

> diff --git a/nptl/pthread_join_common.c b/nptl/pthread_join_common.c
> index a96ceafde4..67d8e2b780 100644
> --- a/nptl/pthread_join_common.c
> +++ b/nptl/pthread_join_common.c
> @@ -20,6 +20,7 @@
>  #include <atomic.h>
>  #include <stap-probe.h>
>  #include <time.h>
> +#include <futex-internal.h>
>  
>  static void
>  cleanup (void *arg)
> @@ -37,9 +38,11 @@ cleanup (void *arg)
>     afterwards.  The kernel up to version 3.16.3 does not use the private futex
>     operations for futex wake-up when the clone terminates.  */
>  static int
> -clockwait_tid (pid_t *tidp, clockid_t clockid, const struct timespec *abstime)
> +clockwait_tid (pid_t *tidp, clockid_t clockid,
> +               const struct __timespec64 *abstime)
>  {
>    pid_t tid;
> +  int ret;
>  
>    if (! valid_nanoseconds (abstime->tv_nsec))
>      return EINVAL;
> @@ -47,11 +50,11 @@ clockwait_tid (pid_t *tidp, clockid_t clockid, const struct timespec *abstime)
>    /* Repeat until thread terminated.  */
>    while ((tid = *tidp) != 0)
>      {
> -      struct timespec rt;
> +      struct __timespec64 rt;
>  
>        /* Get the current time. This can only fail if clockid is
>           invalid. */
> -      if (__glibc_unlikely (__clock_gettime (clockid, &rt)))
> +      if (__glibc_unlikely (__clock_gettime64 (clockid, &rt)))
>          return EINVAL;
>  
>        /* Compute relative timeout.  */
> @@ -70,9 +73,9 @@ clockwait_tid (pid_t *tidp, clockid_t clockid, const struct timespec *abstime)
>        /* If *tidp == tid, wait until thread terminates or the wait times out.
>           The kernel up to version 3.16.3 does not use the private futex
>           operations for futex wake-up when the clone terminates.  */
> -      if (lll_futex_timed_wait_cancel (tidp, tid, &rt, LLL_SHARED)
> -	  == -ETIMEDOUT)
> -        return ETIMEDOUT;
> +      ret = futex_timed_wait_cancel64 (tidp, tid, &rt, LLL_SHARED);
> +      if (ret == -ETIMEDOUT || ret == -EOVERFLOW)
> +        return -ret;
>      }
>  
>    return 0;
> @@ -80,8 +83,8 @@ clockwait_tid (pid_t *tidp, clockid_t clockid, const struct timespec *abstime)
>  
>  int
>  __pthread_clockjoin_ex (pthread_t threadid, void **thread_return,
> -			clockid_t clockid,
> -			const struct timespec *abstime, bool block)
> +                        clockid_t clockid,
> +                        const struct __timespec64 *abstime, bool block)
>  {
>    struct pthread *pd = (struct pthread *) threadid;
>  

Ok, this is an internal interface so we free to change it.

> diff --git a/nptl/pthread_timedjoin.c b/nptl/pthread_timedjoin.c
> index dd7038dcf7..6164ae7060 100644
> --- a/nptl/pthread_timedjoin.c
> +++ b/nptl/pthread_timedjoin.c
> @@ -16,13 +16,27 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> +#include <time.h>
>  #include "pthreadP.h"
>  
>  int
> -__pthread_timedjoin_np (pthread_t threadid, void **thread_return,
> -			const struct timespec *abstime)
> +__pthread_timedjoin_np64 (pthread_t threadid, void **thread_return,
> +                          const struct __timespec64 *abstime)
>  {
>    return __pthread_clockjoin_ex (threadid, thread_return,
>                                   CLOCK_REALTIME, abstime, true);
>  }
> +
> +#if __TIMESIZE != 64
> +libc_hidden_def (__pthread_timedjoin_np64)
> +
> +int
> +__pthread_timedjoin_np (pthread_t threadid, void **thread_return,
> +                        const struct timespec *abstime)
> +{
> +  struct __timespec64 ts64 = valid_timespec_to_timespec64 (*abstime);
> +
> +  return __pthread_timedjoin_np64 (threadid, thread_return, &ts64);
> +}
> +#endif
>  weak_alias (__pthread_timedjoin_np, pthread_timedjoin_np)

Ok.

> diff --git a/sysdeps/nptl/futex-internal.h b/sysdeps/nptl/futex-internal.h
> index d622122ddc..e0a539ae9e 100644
> --- a/sysdeps/nptl/futex-internal.h
> +++ b/sysdeps/nptl/futex-internal.h
> @@ -467,4 +467,35 @@ futex_unlock_pi (unsigned int *futex_word, int private)
>      }
>  }
>  
> +#ifndef __NR_futex_time64
> +# define __NR_futex_time64 __NR_futex
> +#endif
> +
> +static __always_inline int
> +futex_timed_wait_cancel64 (pid_t *tidp,  pid_t tid,
> +                           struct __timespec64 *timeout, int private)

Should we set the timeout as 'const'? Afaik the kernel won't modify its value.

> +{
> +  int oldtype = CANCEL_ASYNC ();
> +  long int ret = INTERNAL_SYSCALL (futex_time64, 4, tidp,
> +                                   __lll_private_flag (FUTEX_WAIT, private),
> +                                   tid, timeout);

We have INTERNAL_SYSCALL_CALL, although I don't have a strong preference
here.  It allows to not need to handle the reset state on early return,
as done in EOVERFLOW below.  In any case use INTERNAL_SYSCALL_CALL.

> +#ifndef __ASSUME_TIME64_SYSCALLS
> +  if (ret == -ENOSYS)
> +    {
> +      struct timespec ts32;
> +      if (! in_time_t_range (timeout->tv_sec))
> +        {
> +          CANCEL_RESET (oldtype);
> +          return -EOVERFLOW;
> +	}
> +      ts32 = valid_timespec64_to_timespec (*timeout);
> +
> +      ret = INTERNAL_SYSCALL (futex, 4, tidp,
> +                              __lll_private_flag (FUTEX_WAIT, private),
> +                              tid, &ts32);

Use INTERNAL_SYCALL_CALL here.

> +    }
> +#endif
> +    CANCEL_RESET (oldtype);
> +    return ret;
> +}
>  #endif  /* futex-internal.h */
> 

I think we should follow the other futex_ routines regarding the possible
return values, i.e, document the possible return values and explicit fail
with futex_fatal_error for non expected ones.
  

Hi Adhemerval,

> On 22/07/2020 04:37, Lukasz Majewski wrote:
> > The pthread_clockjoin_np and pthread_timedjoin_np have been
> > converted to support 64 bit time.
> > 
> > This change introduces new futex_timed_wait_cancel64 function in
> > ./sysdeps/nptl/futex-internal.h, which uses futex_time64 where
> > possible and tries to replace low-level preprocessor macros from
> > lowlevellock-futex.h
> > The pthread_{timed|clock}join_np only accept absolute time.
> > Moreover, there is no need to check for NULL passed as *abstime
> > pointer as clockwait_tid() always passes struct __timespec64.
> > 
> > For systems with __TIMESIZE != 64 && __WORDSIZE == 32:
> > - Conversions between 64 bit time to 32 bit are necessary
> > - Redirection to __pthread_{clock|timed}join_np64 will provide
> > support for 64 bit time
> > 
> > Build tests:
> > ./src/scripts/build-many-glibcs.py glibcs
> > 
> > Run-time tests:
> > - Run specific tests on ARM/x86 32bit systems (qemu):
> >   https://github.com/lmajewski/meta-y2038 and run tests:
> >   https://github.com/lmajewski/y2038-tests/commits/master
> > 
> > Above tests were performed with Y2038 redirection applied as well
> > as without to test the proper usage of both
> > __pthread_{timed|clock}join_np64 and __pthread_{timed|clock}join_np.
> > 
> > ---
> > Changes for v2:
> > - Update the commit message  
> 
> Some comments below regarding the futex-internal additions, the other
> changes looks ok.
> 
> > ---
> >  nptl/pthreadP.h               | 16 +++++++++++++++-
> >  nptl/pthread_clockjoin.c      | 19 ++++++++++++++++---
> >  nptl/pthread_join_common.c    | 19 +++++++++++--------
> >  nptl/pthread_timedjoin.c      | 18 ++++++++++++++++--
> >  sysdeps/nptl/futex-internal.h | 31 +++++++++++++++++++++++++++++++
> >  5 files changed, 89 insertions(+), 14 deletions(-)
> > 
> > diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
> > index 6f94d6be31..99713c8447 100644
> > --- a/nptl/pthreadP.h
> > +++ b/nptl/pthreadP.h
> > @@ -458,6 +458,20 @@ extern int __pthread_cond_init (pthread_cond_t
> > *cond, libc_hidden_proto (__pthread_cond_init)
> >  extern int __pthread_cond_signal (pthread_cond_t *cond);
> >  extern int __pthread_cond_wait (pthread_cond_t *cond,
> > pthread_mutex_t *mutex); +
> > +#if __TIMESIZE == 64
> > +# define __pthread_clockjoin_np64 __pthread_clockjoin_np
> > +# define __pthread_timedjoin_np64 __pthread_timedjoin_np
> > +#else
> > +extern int __pthread_clockjoin_np64 (pthread_t threadid, void
> > **thread_return,
> > +                                     clockid_t clockid,
> > +                                     const struct __timespec64
> > *abstime); +libc_hidden_proto (__pthread_clockjoin_np64)
> > +extern int __pthread_timedjoin_np64 (pthread_t threadid, void
> > **thread_return,
> > +                                     const struct __timespec64
> > *abstime); +libc_hidden_proto (__pthread_timedjoin_np64)
> > +#endif
> > +
> >  extern int __pthread_cond_timedwait (pthread_cond_t *cond,
> >  				     pthread_mutex_t *mutex,
> >  				     const struct timespec
> > *abstime); @@ -488,7 +502,7 @@ extern int
> > __pthread_enable_asynccancel (void) attribute_hidden; extern void
> > __pthread_disable_asynccancel (int oldtype) attribute_hidden;
> > extern void __pthread_testcancel (void); extern int
> > __pthread_clockjoin_ex (pthread_t, void **, clockid_t,
> > -				   const struct timespec *, bool)
> > +				   const struct __timespec64 *,
> > bool) attribute_hidden;
> >  extern int __pthread_sigmask (int, const sigset_t *, sigset_t *);
> >  libc_hidden_proto (__pthread_sigmask);  
> 
> Ok.
> 
> > diff --git a/nptl/pthread_clockjoin.c b/nptl/pthread_clockjoin.c
> > index a3e7f37e3b..3cd780f688 100644
> > --- a/nptl/pthread_clockjoin.c
> > +++ b/nptl/pthread_clockjoin.c
> > @@ -16,14 +16,27 @@
> >     License along with the GNU C Library; if not, see
> >     <http://www.gnu.org/licenses/>.  */
> >  
> > +#include <time.h>
> >  #include "pthreadP.h"
> >  
> >  int
> > -__pthread_clockjoin_np (pthread_t threadid, void **thread_return,
> > -			clockid_t clockid,
> > -			const struct timespec *abstime)
> > +__pthread_clockjoin_np64 (pthread_t threadid, void **thread_return,
> > +                          clockid_t clockid, const struct
> > __timespec64 *abstime) {
> >    return __pthread_clockjoin_ex (threadid, thread_return,
> >                                   clockid, abstime, true);
> >  }
> > +
> > +#if __TIMESIZE != 64
> > +libc_hidden_def (__pthread_clockjoin_np64)
> > +
> > +int
> > +__pthread_clockjoin_np (pthread_t threadid, void **thread_return,
> > +                        clockid_t clockid, const struct timespec
> > *abstime) +{
> > +  struct __timespec64 ts64 = valid_timespec_to_timespec64
> > (*abstime); +
> > +  return __pthread_clockjoin_np64 (threadid, thread_return,
> > clockid, &ts64); +}
> > +#endif
> >  weak_alias (__pthread_clockjoin_np, pthread_clockjoin_np)  
> 
> Ok.
> 
> > diff --git a/nptl/pthread_join_common.c b/nptl/pthread_join_common.c
> > index a96ceafde4..67d8e2b780 100644
> > --- a/nptl/pthread_join_common.c
> > +++ b/nptl/pthread_join_common.c
> > @@ -20,6 +20,7 @@
> >  #include <atomic.h>
> >  #include <stap-probe.h>
> >  #include <time.h>
> > +#include <futex-internal.h>
> >  
> >  static void
> >  cleanup (void *arg)
> > @@ -37,9 +38,11 @@ cleanup (void *arg)
> >     afterwards.  The kernel up to version 3.16.3 does not use the
> > private futex operations for futex wake-up when the clone
> > terminates.  */ static int
> > -clockwait_tid (pid_t *tidp, clockid_t clockid, const struct
> > timespec *abstime) +clockwait_tid (pid_t *tidp, clockid_t clockid,
> > +               const struct __timespec64 *abstime)
> >  {
> >    pid_t tid;
> > +  int ret;
> >  
> >    if (! valid_nanoseconds (abstime->tv_nsec))
> >      return EINVAL;
> > @@ -47,11 +50,11 @@ clockwait_tid (pid_t *tidp, clockid_t clockid,
> > const struct timespec *abstime) /* Repeat until thread terminated.
> > */ while ((tid = *tidp) != 0)
> >      {
> > -      struct timespec rt;
> > +      struct __timespec64 rt;
> >  
> >        /* Get the current time. This can only fail if clockid is
> >           invalid. */
> > -      if (__glibc_unlikely (__clock_gettime (clockid, &rt)))
> > +      if (__glibc_unlikely (__clock_gettime64 (clockid, &rt)))
> >          return EINVAL;
> >  
> >        /* Compute relative timeout.  */
> > @@ -70,9 +73,9 @@ clockwait_tid (pid_t *tidp, clockid_t clockid,
> > const struct timespec *abstime) /* If *tidp == tid, wait until
> > thread terminates or the wait times out. The kernel up to version
> > 3.16.3 does not use the private futex operations for futex wake-up
> > when the clone terminates.  */
> > -      if (lll_futex_timed_wait_cancel (tidp, tid, &rt, LLL_SHARED)
> > -	  == -ETIMEDOUT)
> > -        return ETIMEDOUT;
> > +      ret = futex_timed_wait_cancel64 (tidp, tid, &rt, LLL_SHARED);
> > +      if (ret == -ETIMEDOUT || ret == -EOVERFLOW)
> > +        return -ret;
> >      }
> >  
> >    return 0;
> > @@ -80,8 +83,8 @@ clockwait_tid (pid_t *tidp, clockid_t clockid,
> > const struct timespec *abstime) 
> >  int
> >  __pthread_clockjoin_ex (pthread_t threadid, void **thread_return,
> > -			clockid_t clockid,
> > -			const struct timespec *abstime, bool block)
> > +                        clockid_t clockid,
> > +                        const struct __timespec64 *abstime, bool
> > block) {
> >    struct pthread *pd = (struct pthread *) threadid;
> >    
> 
> Ok, this is an internal interface so we free to change it.
> 
> > diff --git a/nptl/pthread_timedjoin.c b/nptl/pthread_timedjoin.c
> > index dd7038dcf7..6164ae7060 100644
> > --- a/nptl/pthread_timedjoin.c
> > +++ b/nptl/pthread_timedjoin.c
> > @@ -16,13 +16,27 @@
> >     License along with the GNU C Library; if not, see
> >     <https://www.gnu.org/licenses/>.  */
> >  
> > +#include <time.h>
> >  #include "pthreadP.h"
> >  
> >  int
> > -__pthread_timedjoin_np (pthread_t threadid, void **thread_return,
> > -			const struct timespec *abstime)
> > +__pthread_timedjoin_np64 (pthread_t threadid, void **thread_return,
> > +                          const struct __timespec64 *abstime)
> >  {
> >    return __pthread_clockjoin_ex (threadid, thread_return,
> >                                   CLOCK_REALTIME, abstime, true);
> >  }
> > +
> > +#if __TIMESIZE != 64
> > +libc_hidden_def (__pthread_timedjoin_np64)
> > +
> > +int
> > +__pthread_timedjoin_np (pthread_t threadid, void **thread_return,
> > +                        const struct timespec *abstime)
> > +{
> > +  struct __timespec64 ts64 = valid_timespec_to_timespec64
> > (*abstime); +
> > +  return __pthread_timedjoin_np64 (threadid, thread_return, &ts64);
> > +}
> > +#endif
> >  weak_alias (__pthread_timedjoin_np, pthread_timedjoin_np)  
> 
> Ok.
> 
> > diff --git a/sysdeps/nptl/futex-internal.h
> > b/sysdeps/nptl/futex-internal.h index d622122ddc..e0a539ae9e 100644
> > --- a/sysdeps/nptl/futex-internal.h
> > +++ b/sysdeps/nptl/futex-internal.h
> > @@ -467,4 +467,35 @@ futex_unlock_pi (unsigned int *futex_word, int
> > private) }
> >  }
> >  
> > +#ifndef __NR_futex_time64
> > +# define __NR_futex_time64 __NR_futex
> > +#endif
> > +
> > +static __always_inline int
> > +futex_timed_wait_cancel64 (pid_t *tidp,  pid_t tid,
> > +                           struct __timespec64 *timeout, int
> > private)  
> 
> Should we set the timeout as 'const'? Afaik the kernel won't modify
> its value.

Yes, correct - the original pthread_timedjoin_np [1] passes the timeout
(abstime) as const.

> 
> > +{
> > +  int oldtype = CANCEL_ASYNC ();
> > +  long int ret = INTERNAL_SYSCALL (futex_time64, 4, tidp,
> > +                                   __lll_private_flag (FUTEX_WAIT,
> > private),
> > +                                   tid, timeout);  
> 

I've followed the convention in the original code.

> We have INTERNAL_SYSCALL_CALL, although I don't have a strong
> preference here.  It allows to not need to handle the reset state on
> early return, as done in EOVERFLOW below.  In any case use
> INTERNAL_SYSCALL_CALL.

That would look as:
int oldtype = LIBC_CANCEL_ASYNC ();
long int ret = INTERNAL_SYSCALL_CALL (futex_time64, tidp,
			..... );
LIBC_CANCEL_RESET (oldtype);

And then we could replace CANCEL_RESET() from [*].
(as similar approach is in futex_reltimed_wait_cancelable() function in
the same file).

> > +#ifndef __ASSUME_TIME64_SYSCALLS
> > +  if (ret == -ENOSYS)
> > +    {
> > +      struct timespec ts32;
> > +      if (! in_time_t_range (timeout->tv_sec))
> > +        {
> > +          CANCEL_RESET (oldtype);
		^^^^^^^^^^^^ - [*]

> > +          return -EOVERFLOW;
> > +	}
> > +      ts32 = valid_timespec64_to_timespec (*timeout);
> > +
> > +      ret = INTERNAL_SYSCALL (futex, 4, tidp,
> > +                              __lll_private_flag (FUTEX_WAIT,
> > private),
> > +                              tid, &ts32);  
> 
> Use INTERNAL_SYCALL_CALL here.
> 
> > +    }
> > +#endif
> > +    CANCEL_RESET (oldtype);
		^^^^^^^^^^^^ - [*]

> > +    return ret;
> > +}
> >  #endif  /* futex-internal.h */
> >   
> 
> I think we should follow the other futex_ routines regarding the
> possible return values, i.e, document the possible return values and
> explicit fail with futex_fatal_error for non expected ones.

Ok.

I will send v3 shortly.

Links:

[1] - https://linux.die.net/man/3/pthread_timedjoin_np

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