Re: [PATCH v2 2/3] Y2038: nptl: Provide futex_abstimed_wait64 supporting 64 bit time

[Lukasz Majewski <lukma@denx.de>]

[-- Attachment #1: Type: text/plain, Size: 5729 bytes --]

Dear Community,

> This is the helper function, which uses struct __timespec64
> to provide 64 bit absolute time to futex syscalls.
> 
> The aim of this function is to move convoluted pre-processor
> macro code from sysdeps/nptl/lowlevellock-futex.h to C
> function in futex-internal.c
> 
> The futex_abstimed_wait64 function has been put into a separate
> file on the purpose - to avoid issues apparent on the m68k
> architecture related to small number of available registers (there
> is not enough registers to put all necessary arguments in them if
> the above function would be added to futex-internal.h with
> __always_inline attribute).
> 
> Additional precautions for m68k port have been taken - the
> futex-internal.c file will be compiled with -fno-inline flag.
> 

Do we have more comments regarding this patch? Or shall I pull it to
-master?

> ---
> Changes for v2:
> - Handle the case when *abstime pointer is NULL
> ---
>  sysdeps/nptl/futex-internal.c         | 70
> +++++++++++++++++++++++++++ sysdeps/nptl/futex-internal.h         |
> 6 +++ sysdeps/unix/sysv/linux/m68k/Makefile |  2 +
>  3 files changed, 78 insertions(+)
> 
> diff --git a/sysdeps/nptl/futex-internal.c
> b/sysdeps/nptl/futex-internal.c index 3366aac162..3211b4c94f 100644
> --- a/sysdeps/nptl/futex-internal.c
> +++ b/sysdeps/nptl/futex-internal.c
> @@ -45,6 +45,29 @@ __futex_abstimed_wait_cancelable32 (unsigned int*
> futex_word, abstime != NULL ? &ts32 : NULL,
>                                    NULL /* Unused.  */,
> FUTEX_BITSET_MATCH_ANY); }
> +
> +static int
> +__futex_abstimed_wait32 (unsigned int* futex_word,
> +                         unsigned int expected, clockid_t clockid,
> +                         const struct __timespec64* abstime,
> +                         int private)
> +{
> +  struct timespec ts32;
> +
> +  if (abstime != NULL && ! in_time_t_range (abstime->tv_sec))
> +    return -EOVERFLOW;
> +
> +  unsigned int clockbit = (clockid == CLOCK_REALTIME) ?
> +    FUTEX_CLOCK_REALTIME : 0;
> +  int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit,
> private); +
> +  if (abstime != NULL)
> +    ts32 = valid_timespec64_to_timespec (*abstime);
> +
> +  return INTERNAL_SYSCALL_CALL (futex, futex_word, op, expected,
> +                                abstime != NULL ? &ts32 : NULL,
> +                                NULL /* Unused.  */,
> FUTEX_BITSET_MATCH_ANY); +}
>  #endif
>  
>  int
> @@ -97,3 +120,50 @@ __futex_abstimed_wait_cancelable64 (unsigned int*
> futex_word, futex_fatal_error ();
>      }
>  }
> +
> +int
> +__futex_abstimed_wait64 (unsigned int* futex_word, unsigned int
> expected,
> +                         clockid_t clockid,
> +                         const struct __timespec64* abstime, int
> private) +{
> +  unsigned int clockbit;
> +  int err;
> +
> +  /* Work around the fact that the kernel rejects negative timeout
> values
> +     despite them being valid.  */
> +  if (__glibc_unlikely ((abstime != NULL) && (abstime->tv_sec < 0)))
> +    return ETIMEDOUT;
> +
> +  if (! lll_futex_supported_clockid(clockid))
> +    return EINVAL;
> +
> +  clockbit = (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0;
> +  int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit,
> private); +
> +  err = INTERNAL_SYSCALL_CALL (futex_time64, futex_word, op,
> expected,
> +                               abstime, NULL /* Unused.  */,
> +                               FUTEX_BITSET_MATCH_ANY);
> +#ifndef __ASSUME_TIME64_SYSCALLS
> +  if (err == -ENOSYS)
> +    err = __futex_abstimed_wait32 (futex_word, expected,
> +                                   clockid, abstime, private);
> +#endif
> +  switch (err)
> +    {
> +    case 0:
> +    case -EAGAIN:
> +    case -EINTR:
> +    case -ETIMEDOUT:
> +      return -err;
> +
> +    case -EFAULT: /* Must have been caused by a glibc or application
> bug.  */
> +    case -EINVAL: /* Either due to wrong alignment, unsupported
> +		     clockid or due to the timeout not being
> +		     normalized. Must have been caused by a glibc or
> +		     application bug.  */
> +    case -ENOSYS: /* Must have been caused by a glibc bug.  */
> +    /* No other errors are documented at this time.  */
> +    default:
> +      futex_fatal_error ();
> +    }
> +}
> diff --git a/sysdeps/nptl/futex-internal.h
> b/sysdeps/nptl/futex-internal.h index 7f3910ad98..1ba0d61938 100644
> --- a/sysdeps/nptl/futex-internal.h
> +++ b/sysdeps/nptl/futex-internal.h
> @@ -529,4 +529,10 @@ __futex_abstimed_wait_cancelable64 (unsigned
> int* futex_word, const struct __timespec64* abstime,
>                                      int private) attribute_hidden;
>  
> +int
> +__futex_abstimed_wait64 (unsigned int* futex_word, unsigned int
> expected,
> +                         clockid_t clockid,
> +                         const struct __timespec64* abstime,
> +                         int private) attribute_hidden;
> +
>  #endif  /* futex-internal.h */
> diff --git a/sysdeps/unix/sysv/linux/m68k/Makefile
> b/sysdeps/unix/sysv/linux/m68k/Makefile index be40fae68a..65164c5752
> 100644 --- a/sysdeps/unix/sysv/linux/m68k/Makefile
> +++ b/sysdeps/unix/sysv/linux/m68k/Makefile
> @@ -21,3 +21,5 @@ sysdep-dl-routines += dl-static
>  sysdep-others += lddlibc4
>  install-bin += lddlibc4
>  endif
> +
> +CFLAGS-futex-internal.c += -fno-inline




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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]