[07/13] linux: nptl: Replace __futex_clocklock_wait64 with __futex_abstimed_wait64

Message ID 20201123195256.3336217-7-adhemerval.zanella@linaro.org
State Committed
Headers
Series [01/13] linux: Remove unused internal futex functions |

Commit Message

Adhemerval Zanella Netto Nov. 23, 2020, 7:52 p.m. UTC
  The __futex_abstimed_wait64 needs also to return EINVAL syscall
errors.

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 nptl/pthread_mutex_timedlock.c |  4 +--
 sysdeps/nptl/futex-internal.c  | 57 +---------------------------------
 sysdeps/nptl/futex-internal.h  |  7 ++---
 3 files changed, 5 insertions(+), 63 deletions(-)
  

Comments

Lukasz Majewski Nov. 24, 2020, 9:28 p.m. UTC | #1
Hi Adhemerval,

> The __futex_abstimed_wait64 needs also to return EINVAL syscall
> errors.
> 
> Checked on x86_64-linux-gnu and i686-linux-gnu.
> ---
>  nptl/pthread_mutex_timedlock.c |  4 +--
>  sysdeps/nptl/futex-internal.c  | 57
> +--------------------------------- sysdeps/nptl/futex-internal.h  |
> 7 ++--- 3 files changed, 5 insertions(+), 63 deletions(-)
> 
> diff --git a/nptl/pthread_mutex_timedlock.c
> b/nptl/pthread_mutex_timedlock.c index 0ec47359be..e643eab258 100644
> --- a/nptl/pthread_mutex_timedlock.c
> +++ b/nptl/pthread_mutex_timedlock.c
> @@ -391,8 +391,8 @@ __pthread_mutex_clocklock_common (pthread_mutex_t
> *mutex, /* Delay the thread until the timeout is reached. Then return
>  		   ETIMEDOUT.  */
>  		do
> -		  e = __futex_clocklock_wait64 (&(int){0}, 0,
> clockid, abstime,
> -		                                private);
> +		  e = __futex_abstimed_wait64 (&(unsigned int){0},
> 0, clockid,
> +					       abstime, private);

I think that it would be nice to have more verbose commit message - to
clearly explain why it is possible to replace "*_clocklock_*" with
"*_abstimed_*".

Reviewed-by: Lukasz Majewski <lukma@denx.de>

>  		while (e != ETIMEDOUT);
>  		return ETIMEDOUT;
>  	      }
> diff --git a/sysdeps/nptl/futex-internal.c
> b/sysdeps/nptl/futex-internal.c index 30c662547f..11031cc46a 100644
> --- a/sysdeps/nptl/futex-internal.c
> +++ b/sysdeps/nptl/futex-internal.c
> @@ -90,15 +90,13 @@ __futex_abstimed_wait_common64 (unsigned int*
> futex_word, case -EAGAIN:
>      case -EINTR:
>      case -ETIMEDOUT:
> +    case -EINVAL:
>      case -EOVERFLOW:  /* Passed absolute timeout uses 64 bit time_t
> type, but underlying kernel does not support 64 bit time_t futex
>                           syscalls.  */
>        return -err;
>  
>      case -EFAULT: /* Must have been caused by a glibc or application
> bug.  */
> -    case -EINVAL: /* Either due to wrong alignment 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:
> @@ -124,56 +122,3 @@ __futex_abstimed_wait_cancelable64 (unsigned
> int* futex_word, return __futex_abstimed_wait_common64 (futex_word,
> expected, clockid, abstime, private, true);
>  }
> -
> -int
> -__futex_clocklock_wait64 (int *futex, int val, clockid_t clockid,
> -                          const struct __timespec64 *abstime, int
> private) -{
> -  struct __timespec64 ts, *tsp = NULL;
> -
> -  if (abstime != NULL)
> -    {
> -      /* Reject invalid timeouts.  */
> -      if (! valid_nanoseconds (abstime->tv_nsec))
> -        return EINVAL;
> -
> -      /* Get the current time. This can only fail if clockid is not
> valid.  */
> -      if (__glibc_unlikely (__clock_gettime64 (clockid, &ts) != 0))
> -        return EINVAL;
> -
> -      /* Compute relative timeout.  */
> -      ts.tv_sec = abstime->tv_sec - ts.tv_sec;
> -      ts.tv_nsec = abstime->tv_nsec - ts.tv_nsec;
> -      if (ts.tv_nsec < 0)
> -        {
> -	  ts.tv_nsec += 1000000000;
> -	  --ts.tv_sec;
> -        }
> -
> -      if (ts.tv_sec < 0)
> -        return ETIMEDOUT;
> -
> -      tsp = &ts;
> -    }
> -
> -  int err = INTERNAL_SYSCALL_CALL (futex_time64, futex,
> -                                   __lll_private_flag (FUTEX_WAIT,
> private),
> -                                   val, tsp);
> -#ifndef __ASSUME_TIME64_SYSCALLS
> -  if (err == -ENOSYS)
> -    {
> -      if (tsp != NULL && ! in_time_t_range (tsp->tv_sec))
> -        return EOVERFLOW;
> -
> -      struct timespec ts32;
> -      if (tsp != NULL)
> -        ts32 = valid_timespec64_to_timespec (*tsp);
> -
> -      err = INTERNAL_SYSCALL_CALL (futex, futex,
> -                                   __lll_private_flag (FUTEX_WAIT,
> private),
> -                                   val, tsp != NULL ? &ts32 : NULL);
> -    }
> -#endif
> -
> -  return -err;
> -}
> diff --git a/sysdeps/nptl/futex-internal.h
> b/sysdeps/nptl/futex-internal.h index dcc7958d59..632051278b 100644
> --- a/sysdeps/nptl/futex-internal.h
> +++ b/sysdeps/nptl/futex-internal.h
> @@ -417,10 +417,6 @@ __futex_abstimed_wait64 (unsigned int*
> futex_word, unsigned int expected, const struct __timespec64* abstime,
>                           int private) attribute_hidden;
>  
> -int
> -__futex_clocklock_wait64 (int *futex, int val, clockid_t clockid,
> -                          const struct __timespec64 *abstime,
> -                          int private) attribute_hidden;
>  
>  static __always_inline int
>  __futex_clocklock64 (int *futex, clockid_t clockid,
> @@ -432,7 +428,8 @@ __futex_clocklock64 (int *futex, clockid_t
> clockid, {
>        while (atomic_exchange_acq (futex, 2) != 0)
>          {
> -          err = __futex_clocklock_wait64 (futex, 2, clockid,
> abstime, private);
> +          err = __futex_abstimed_wait64 ((unsigned int *) futex, 2,
> clockid,
> +					 abstime, private);
>            if (err == EINVAL || err == ETIMEDOUT || err == EOVERFLOW)
>              break;
>          }




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
  

Patch

diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
index 0ec47359be..e643eab258 100644
--- a/nptl/pthread_mutex_timedlock.c
+++ b/nptl/pthread_mutex_timedlock.c
@@ -391,8 +391,8 @@  __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
 		/* Delay the thread until the timeout is reached. Then return
 		   ETIMEDOUT.  */
 		do
-		  e = __futex_clocklock_wait64 (&(int){0}, 0, clockid, abstime,
-		                                private);
+		  e = __futex_abstimed_wait64 (&(unsigned int){0}, 0, clockid,
+					       abstime, private);
 		while (e != ETIMEDOUT);
 		return ETIMEDOUT;
 	      }
diff --git a/sysdeps/nptl/futex-internal.c b/sysdeps/nptl/futex-internal.c
index 30c662547f..11031cc46a 100644
--- a/sysdeps/nptl/futex-internal.c
+++ b/sysdeps/nptl/futex-internal.c
@@ -90,15 +90,13 @@  __futex_abstimed_wait_common64 (unsigned int* futex_word,
     case -EAGAIN:
     case -EINTR:
     case -ETIMEDOUT:
+    case -EINVAL:
     case -EOVERFLOW:  /* Passed absolute timeout uses 64 bit time_t type, but
                          underlying kernel does not support 64 bit time_t futex
                          syscalls.  */
       return -err;
 
     case -EFAULT: /* Must have been caused by a glibc or application bug.  */
-    case -EINVAL: /* Either due to wrong alignment 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:
@@ -124,56 +122,3 @@  __futex_abstimed_wait_cancelable64 (unsigned int* futex_word,
   return __futex_abstimed_wait_common64 (futex_word, expected, clockid,
                                          abstime, private, true);
 }
-
-int
-__futex_clocklock_wait64 (int *futex, int val, clockid_t clockid,
-                          const struct __timespec64 *abstime, int private)
-{
-  struct __timespec64 ts, *tsp = NULL;
-
-  if (abstime != NULL)
-    {
-      /* Reject invalid timeouts.  */
-      if (! valid_nanoseconds (abstime->tv_nsec))
-        return EINVAL;
-
-      /* Get the current time. This can only fail if clockid is not valid.  */
-      if (__glibc_unlikely (__clock_gettime64 (clockid, &ts) != 0))
-        return EINVAL;
-
-      /* Compute relative timeout.  */
-      ts.tv_sec = abstime->tv_sec - ts.tv_sec;
-      ts.tv_nsec = abstime->tv_nsec - ts.tv_nsec;
-      if (ts.tv_nsec < 0)
-        {
-	  ts.tv_nsec += 1000000000;
-	  --ts.tv_sec;
-        }
-
-      if (ts.tv_sec < 0)
-        return ETIMEDOUT;
-
-      tsp = &ts;
-    }
-
-  int err = INTERNAL_SYSCALL_CALL (futex_time64, futex,
-                                   __lll_private_flag (FUTEX_WAIT, private),
-                                   val, tsp);
-#ifndef __ASSUME_TIME64_SYSCALLS
-  if (err == -ENOSYS)
-    {
-      if (tsp != NULL && ! in_time_t_range (tsp->tv_sec))
-        return EOVERFLOW;
-
-      struct timespec ts32;
-      if (tsp != NULL)
-        ts32 = valid_timespec64_to_timespec (*tsp);
-
-      err = INTERNAL_SYSCALL_CALL (futex, futex,
-                                   __lll_private_flag (FUTEX_WAIT, private),
-                                   val, tsp != NULL ? &ts32 : NULL);
-    }
-#endif
-
-  return -err;
-}
diff --git a/sysdeps/nptl/futex-internal.h b/sysdeps/nptl/futex-internal.h
index dcc7958d59..632051278b 100644
--- a/sysdeps/nptl/futex-internal.h
+++ b/sysdeps/nptl/futex-internal.h
@@ -417,10 +417,6 @@  __futex_abstimed_wait64 (unsigned int* futex_word, unsigned int expected,
                          const struct __timespec64* abstime,
                          int private) attribute_hidden;
 
-int
-__futex_clocklock_wait64 (int *futex, int val, clockid_t clockid,
-                          const struct __timespec64 *abstime,
-                          int private) attribute_hidden;
 
 static __always_inline int
 __futex_clocklock64 (int *futex, clockid_t clockid,
@@ -432,7 +428,8 @@  __futex_clocklock64 (int *futex, clockid_t clockid,
     {
       while (atomic_exchange_acq (futex, 2) != 0)
         {
-          err = __futex_clocklock_wait64 (futex, 2, clockid, abstime, private);
+          err = __futex_abstimed_wait64 ((unsigned int *) futex, 2, clockid,
+					 abstime, private);
           if (err == EINVAL || err == ETIMEDOUT || err == EOVERFLOW)
             break;
         }