[PATCH 01/13] linux: Remove unused internal futex functions

[PATCH 01/13] linux: Remove unused internal futex functions

[Adhemerval Zanella via Libc-alpha]

The __futex_abstimed_wait usage was remove with 3102e28bd11 and the
__futex_abstimed_wait_cancelable by 323592fdc92 and b8d3e8fbaac.
The futex_lock_pi can be replaced by a futex_lock_pi64.

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 nptl/pthread_mutex_lock.c     |  3 +-
 sysdeps/nptl/futex-internal.h | 98 -----------------------------------
 2 files changed, 1 insertion(+), 100 deletions(-)

diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c
index fac774e608..0439002454 100644
--- a/nptl/pthread_mutex_lock.c
+++ b/nptl/pthread_mutex_lock.c
@@ -413,8 +413,7 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
 	    int private = (robust
 			   ? PTHREAD_ROBUST_MUTEX_PSHARED (mutex)
 			   : PTHREAD_MUTEX_PSHARED (mutex));
-	    int e = futex_lock_pi ((unsigned int *) &mutex->__data.__lock,
-				   NULL, private);
+	    int e = futex_lock_pi64 (&mutex->__data.__lock, NULL, private);
 	    if (e == ESRCH || e == EDEADLK)
 	      {
 		assert (e != EDEADLK
diff --git a/sysdeps/nptl/futex-internal.h b/sysdeps/nptl/futex-internal.h
index c27d0cdac8..21e3b74be6 100644
--- a/sysdeps/nptl/futex-internal.h
+++ b/sysdeps/nptl/futex-internal.h
@@ -275,76 +275,6 @@ futex_abstimed_supported_clockid (clockid_t clockid)
   return lll_futex_supported_clockid (clockid);
 }
 
-/* Like futex_reltimed_wait, but the provided timeout (ABSTIME) is an
-   absolute point in time; a call will time out after this point in time.  */
-static __always_inline int
-futex_abstimed_wait (unsigned int* futex_word, unsigned int expected,
-		     clockid_t clockid,
-		     const struct timespec* abstime, int private)
-{
-  /* 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;
-  int err = lll_futex_clock_wait_bitset (futex_word, expected,
-					 clockid, abstime,
-					 private);
-  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 ();
-    }
-}
-
-/* Like futex_reltimed_wait but is a POSIX cancellation point.  */
-static __always_inline int
-futex_abstimed_wait_cancelable (unsigned int* futex_word,
-				unsigned int expected,
-				clockid_t clockid,
-			        const struct timespec* abstime, int private)
-{
-  /* 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;
-  int oldtype;
-  oldtype = __pthread_enable_asynccancel ();
-  int err = lll_futex_clock_wait_bitset (futex_word, expected,
-					clockid, abstime,
-					private);
-  __pthread_disable_asynccancel (oldtype);
-  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 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 ();
-    }
-}
-
 /* Atomically wrt other futex operations on the same futex, this unblocks the
    specified number of processes, or all processes blocked on this futex if
    there are fewer than the specified number.  Semantically, this is
@@ -410,34 +340,6 @@ futex_wake (unsigned int* futex_word, int processes_to_wake, int private)
        futex.
      - ETIMEDOUT if the ABSTIME expires.
 */
-static __always_inline int
-futex_lock_pi (unsigned int *futex_word, const struct timespec *abstime,
-	       int private)
-{
-  int err = lll_futex_timed_lock_pi (futex_word, abstime, private);
-  switch (err)
-    {
-    case 0:
-    case -EAGAIN:
-    case -EINTR:
-    case -ETIMEDOUT:
-    case -ESRCH:
-    case -EDEADLK:
-    case -EINVAL: /* This indicates either state corruption or that the kernel
-		     found a waiter on futex address which is waiting via
-		     FUTEX_WAIT or FUTEX_WAIT_BITSET.  This is reported on
-		     some futex_lock_pi usage (pthread_mutex_timedlock for
-		     instance).  */
-      return -err;
-
-    case -EFAULT: /* 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 ();
-    }
-}
-
 static __always_inline int
 futex_lock_pi64 (int *futex_word, const struct __timespec64 *abstime,
                  int private)
-- 
2.25.1

[PATCH 02/13] nptl: Remove futex_wait_cancelable

[Adhemerval Zanella via Libc-alpha]

It is used solely on __pthread_cond_wait_common and the call can be
replaced by a __futex_abstimed_wait_cancelable64 one.

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 nptl/pthread_cond_wait.c      | 22 ++--------------------
 sysdeps/nptl/futex-internal.h | 29 -----------------------------
 2 files changed, 2 insertions(+), 49 deletions(-)

diff --git a/nptl/pthread_cond_wait.c b/nptl/pthread_cond_wait.c
index 7d158d553f..685dbca32f 100644
--- a/nptl/pthread_cond_wait.c
+++ b/nptl/pthread_cond_wait.c
@@ -501,26 +501,8 @@ __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex,
 	  cbuffer.private = private;
 	  __pthread_cleanup_push (&buffer, __condvar_cleanup_waiting, &cbuffer);
 
-	  if (abstime == NULL)
-	    {
-	      /* Block without a timeout.  */
-	      err = futex_wait_cancelable (
-		  cond->__data.__g_signals + g, 0, private);
-	    }
-	  else
-	    {
-	      /* Block, but with a timeout.
-		 Work around the fact that the kernel rejects negative timeout
-		 values despite them being valid.  */
-	      if (__glibc_unlikely (abstime->tv_sec < 0))
-	        err = ETIMEDOUT;
-	      else
-		{
-		  err = __futex_abstimed_wait_cancelable64
-                    (cond->__data.__g_signals + g, 0, clockid, abstime,
-                     private);
-		}
-	    }
+	  err = __futex_abstimed_wait_cancelable64 (
+	    cond->__data.__g_signals + g, 0, clockid, abstime, private);
 
 	  __pthread_cleanup_pop (&buffer, 0);
 
diff --git a/sysdeps/nptl/futex-internal.h b/sysdeps/nptl/futex-internal.h
index 21e3b74be6..96d1318091 100644
--- a/sysdeps/nptl/futex-internal.h
+++ b/sysdeps/nptl/futex-internal.h
@@ -177,35 +177,6 @@ futex_wait_simple (unsigned int *futex_word, unsigned int expected,
   ignore_value (futex_wait (futex_word, expected, private));
 }
 
-
-/* Like futex_wait but is a POSIX cancellation point.  */
-static __always_inline int
-futex_wait_cancelable (unsigned int *futex_word, unsigned int expected,
-		       int private)
-{
-  int oldtype;
-  oldtype = __pthread_enable_asynccancel ();
-  int err = lll_futex_timed_wait (futex_word, expected, NULL, private);
-  __pthread_disable_asynccancel (oldtype);
-  switch (err)
-    {
-    case 0:
-    case -EAGAIN:
-    case -EINTR:
-      return -err;
-
-    case -ETIMEDOUT: /* Cannot have happened as we provided no timeout.  */
-    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:
-      futex_fatal_error ();
-    }
-}
-
 /* Like futex_wait, but will eventually time out (i.e., stop being
    blocked) after the duration of time provided (i.e., RELTIME) has
    passed.  The caller must provide a normalized RELTIME.  RELTIME can also
-- 
2.25.1

[PATCH 03/13] nptl: Remove clockwait_tid

[Adhemerval Zanella via Libc-alpha]

It can be replaced with a __futex_abstimed_wait_cancelable64 call,
with the advantage that there is no need to further clock adjustments
to create a absolute timeout.  It allows to remove the now ununsed
futex_timed_wait_cancel64 internal function.

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 nptl/pthread_join_common.c    | 70 ++++++-----------------------------
 sysdeps/nptl/futex-internal.h | 49 ------------------------
 2 files changed, 12 insertions(+), 107 deletions(-)

diff --git a/nptl/pthread_join_common.c b/nptl/pthread_join_common.c
index 67d8e2b780..8a95c58ff3 100644
--- a/nptl/pthread_join_common.c
+++ b/nptl/pthread_join_common.c
@@ -32,55 +32,6 @@ cleanup (void *arg)
   atomic_compare_exchange_weak_acquire (&arg, &self, NULL);
 }
 
-/* The kernel notifies a process which uses CLONE_CHILD_CLEARTID via futex
-   wake-up when the clone terminates.  The memory location contains the
-   thread ID while the clone is running and is reset to zero by the kernel
-   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 __timespec64 *abstime)
-{
-  pid_t tid;
-  int ret;
-
-  if (! valid_nanoseconds (abstime->tv_nsec))
-    return EINVAL;
-
-  /* Repeat until thread terminated.  */
-  while ((tid = *tidp) != 0)
-    {
-      struct __timespec64 rt;
-
-      /* Get the current time. This can only fail if clockid is
-         invalid. */
-      if (__glibc_unlikely (__clock_gettime64 (clockid, &rt)))
-        return EINVAL;
-
-      /* Compute relative timeout.  */
-      rt.tv_sec = abstime->tv_sec - rt.tv_sec;
-      rt.tv_nsec = abstime->tv_nsec - rt.tv_nsec;
-      if (rt.tv_nsec < 0)
-        {
-          rt.tv_nsec += 1000000000;
-          --rt.tv_sec;
-        }
-
-      /* Already timed out?  */
-      if (rt.tv_sec < 0)
-        return ETIMEDOUT;
-
-      /* 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.  */
-      ret = futex_timed_wait_cancel64 (tidp, tid, &rt, LLL_SHARED);
-      if (ret == -ETIMEDOUT || ret == -EOVERFLOW)
-        return -ret;
-    }
-
-  return 0;
-}
-
 int
 __pthread_clockjoin_ex (pthread_t threadid, void **thread_return,
                         clockid_t clockid,
@@ -137,15 +88,18 @@ __pthread_clockjoin_ex (pthread_t threadid, void **thread_return,
 	 un-wait-ed for again.  */
       pthread_cleanup_push (cleanup, &pd->joinid);
 
-      if (abstime != NULL)
-	result = clockwait_tid (&pd->tid, clockid, abstime);
-      else
-	{
-	  pid_t tid;
-	  /* We need acquire MO here so that we synchronize with the
-	     kernel's store to 0 when the clone terminates. (see above)  */
-	  while ((tid = atomic_load_acquire (&pd->tid)) != 0)
-	    lll_futex_wait_cancel (&pd->tid, tid, LLL_SHARED);
+     /* We need acquire MO here so that we synchronize with the
+        kernel's store to 0 when the clone terminates. (see above)  */
+      pid_t tid;
+      while ((tid = atomic_load_acquire (&pd->tid)) != 0)
+        {
+	  int ret = __futex_abstimed_wait_cancelable64 (
+	    (unsigned int *) &pd->tid, tid, clockid, abstime, LLL_SHARED);
+	  if (ret == ETIMEDOUT || ret == EOVERFLOW)
+	    {
+	      result = ret;
+	      break;
+	    }
 	}
 
       pthread_cleanup_pop (0);
diff --git a/sysdeps/nptl/futex-internal.h b/sysdeps/nptl/futex-internal.h
index 96d1318091..d5f13d15fb 100644
--- a/sysdeps/nptl/futex-internal.h
+++ b/sysdeps/nptl/futex-internal.h
@@ -390,55 +390,6 @@ futex_unlock_pi (unsigned int *futex_word, int private)
     }
 }
 
-static __always_inline int
-futex_timed_wait_cancel64 (pid_t *tidp,  pid_t tid,
-                           const struct __timespec64 *timeout, int private)
-{
-  int err = INTERNAL_SYSCALL_CANCEL (futex_time64, tidp,
-                                     __lll_private_flag (FUTEX_WAIT, private),
-                                     tid, timeout);
-#ifndef __ASSUME_TIME64_SYSCALLS
-  if (err == -ENOSYS)
-    {
-      if (in_time_t_range (timeout->tv_sec))
-        {
-          struct timespec ts32 = valid_timespec64_to_timespec (*timeout);
-
-          err = INTERNAL_SYSCALL_CANCEL (futex, tidp,
-                                         __lll_private_flag (FUTEX_WAIT,
-                                                             private),
-                                         tid, &ts32);
-        }
-      else
-        err = -EOVERFLOW;
-    }
-#endif
-  switch (err)
-    {
-    case 0:
-    case -EAGAIN:
-    case -EINTR:
-    case -ETIMEDOUT:
-    case -EDEADLK:
-    case -ENOSYS:
-    case -EOVERFLOW:  /* Passed absolute timeout uses 64 bit time_t type, but
-                         underlying kernel does not support 64 bit time_t futex
-                         syscalls.  */
-    case -EPERM:  /*  The caller is not allowed to attach itself to the futex.
-		      Used to check if PI futexes are supported by the
-		      kernel.  */
-      return -err;
-
-    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 -EFAULT: /* Must have been caused by a glibc or application bug.  */
-    /* No other errors are documented at this time.  */
-    default:
-      futex_fatal_error ();
-    }
-}
-
 /* The futex_abstimed_wait_cancelable64 has been moved to a separate file
    to avoid problems with exhausting available registers on some architectures
    - e.g. on m68k architecture.  */
-- 
2.25.1

[PATCH 04/13] linux: Extend __futex_abstimed_wait_cancelable64 comment

[Adhemerval Zanella via Libc-alpha]

And add a small optimization to avoid setting the operation for the
32-bit time fallback operation.

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 sysdeps/nptl/futex-internal.c |  8 ++------
 sysdeps/nptl/futex-internal.h | 18 +++++++++++++++---
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/sysdeps/nptl/futex-internal.c b/sysdeps/nptl/futex-internal.c
index 457cd3cd69..e4a14b477c 100644
--- a/sysdeps/nptl/futex-internal.c
+++ b/sysdeps/nptl/futex-internal.c
@@ -25,7 +25,7 @@
 #ifndef __ASSUME_TIME64_SYSCALLS
 static int
 __futex_abstimed_wait_cancelable32 (unsigned int* futex_word,
-                                    unsigned int expected, clockid_t clockid,
+                                    unsigned int expected, int op,
                                     const struct __timespec64* abstime,
                                     int private)
 {
@@ -39,10 +39,6 @@ __futex_abstimed_wait_cancelable32 (unsigned int* futex_word,
       pts32 = &ts32;
     }
 
-  unsigned int clockbit = (clockid == CLOCK_REALTIME)
-	  ? FUTEX_CLOCK_REALTIME : 0;
-  int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, private);
-
   return INTERNAL_SYSCALL_CANCEL (futex, futex_word, op, expected,
                                   pts32, NULL /* Unused.  */,
                                   FUTEX_BITSET_MATCH_ANY);
@@ -119,7 +115,7 @@ __futex_abstimed_wait_cancelable64 (unsigned int* futex_word,
 #ifndef __ASSUME_TIME64_SYSCALLS
   if (err == -ENOSYS)
     err = __futex_abstimed_wait_cancelable32 (futex_word, expected,
-                                              clockid, abstime, private);
+                                              op, abstime, private);
 #endif
 
   switch (err)
diff --git a/sysdeps/nptl/futex-internal.h b/sysdeps/nptl/futex-internal.h
index d5f13d15fb..cefab74301 100644
--- a/sysdeps/nptl/futex-internal.h
+++ b/sysdeps/nptl/futex-internal.h
@@ -390,9 +390,21 @@ futex_unlock_pi (unsigned int *futex_word, int private)
     }
 }
 
-/* The futex_abstimed_wait_cancelable64 has been moved to a separate file
-   to avoid problems with exhausting available registers on some architectures
-   - e.g. on m68k architecture.  */
+/* Like futex_wait, but will eventually time out (i.e., stop being blocked)
+   after the duration of time provided (i.e., ABSTIME) has passed using the
+   clock specified by CLOCKID (currently only CLOCK_REALTIME and
+   CLOCK_MONOTONIC, the ones support by lll_futex_supported_clockid). ABSTIME
+   can also equal NULL, in which case this function behaves equivalent to
+   futex_wait.
+
+   Returns the same values as futex_wait under those same conditions;
+   additionally, returns ETIMEDOUT if the timeout expired.
+
+   The call acts a cancellation entrypoint.
+
+   (The implementation has been moved to a separate file to avoid problems
+   with exhausting available registers on some architectures - e.g. on
+   m68k).  */
 int
 __futex_abstimed_wait_cancelable64 (unsigned int* futex_word,
                                     unsigned int expected, clockid_t clockid,
-- 
2.25.1

[PATCH 05/13] linux: nptl: Consolidate __futex_abstimed_wait_{cancelable}64

[Adhemerval Zanella via Libc-alpha]

The only different is how to issue the syscall.

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 sysdeps/nptl/futex-internal.c | 111 +++++++++++-----------------------
 1 file changed, 35 insertions(+), 76 deletions(-)

diff --git a/sysdeps/nptl/futex-internal.c b/sysdeps/nptl/futex-internal.c
index e4a14b477c..f9a2e28ee6 100644
--- a/sysdeps/nptl/futex-internal.c
+++ b/sysdeps/nptl/futex-internal.c
@@ -24,10 +24,10 @@
 
 #ifndef __ASSUME_TIME64_SYSCALLS
 static int
-__futex_abstimed_wait_cancelable32 (unsigned int* futex_word,
-                                    unsigned int expected, int op,
-                                    const struct __timespec64* abstime,
-                                    int private)
+__futex_abstimed_wait_common32 (unsigned int* futex_word,
+                                unsigned int expected, int op,
+                                const struct __timespec64* abstime,
+                                int private, bool cancel)
 {
   struct timespec ts32, *pts32 = NULL;
   if (abstime != NULL)
@@ -39,34 +39,16 @@ __futex_abstimed_wait_cancelable32 (unsigned int* futex_word,
       pts32 = &ts32;
     }
 
-  return INTERNAL_SYSCALL_CANCEL (futex, futex_word, op, expected,
+  if (cancel)
+    return INTERNAL_SYSCALL_CANCEL (futex, futex_word, op, expected,
+                                    pts32, NULL /* Unused.  */,
+                                    FUTEX_BITSET_MATCH_ANY);
+  else
+    return INTERNAL_SYSCALL_CALL (futex, futex_word, op, expected,
                                   pts32, 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);
-}
-
 static int
 __futex_clock_wait_bitset32 (int *futexp, int val, clockid_t clockid,
                              const struct __timespec64 *abstime, int private)
@@ -89,11 +71,11 @@ __futex_clock_wait_bitset32 (int *futexp, int val, clockid_t clockid,
 }
 #endif /* ! __ASSUME_TIME64_SYSCALLS */
 
-int
-__futex_abstimed_wait_cancelable64 (unsigned int* futex_word,
-                                    unsigned int expected, clockid_t clockid,
-                                    const struct __timespec64* abstime,
-                                    int private)
+static int
+__futex_abstimed_wait_common64 (unsigned int* futex_word,
+                                unsigned int expected, clockid_t clockid,
+                                const struct __timespec64* abstime,
+                                int private, bool cancel)
 {
   unsigned int clockbit;
   int err;
@@ -109,13 +91,18 @@ __futex_abstimed_wait_cancelable64 (unsigned int* futex_word,
   clockbit = (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0;
   int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, private);
 
-  err = INTERNAL_SYSCALL_CANCEL (futex_time64, futex_word, op, expected,
-                                 abstime, NULL /* Unused.  */,
+  if (cancel)
+    err = INTERNAL_SYSCALL_CANCEL (futex_time64, futex_word, op, expected,
+                                   abstime, NULL /* Unused.  */,
+                                   FUTEX_BITSET_MATCH_ANY);
+  else
+    err = INTERNAL_SYSCALL_CALL (futex_time64, futex_word, op, expected,
+                                 abstime, NULL /* Ununsed.  */,
                                  FUTEX_BITSET_MATCH_ANY);
 #ifndef __ASSUME_TIME64_SYSCALLS
   if (err == -ENOSYS)
-    err = __futex_abstimed_wait_cancelable32 (futex_word, expected,
-                                              op, abstime, private);
+    err = __futex_abstimed_wait_common32 (futex_word, expected, op, abstime,
+                                          private, cancel);
 #endif
 
   switch (err)
@@ -145,46 +132,18 @@ __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;
+  return __futex_abstimed_wait_common64 (futex_word, expected, clockid,
+                                         abstime, private, false);
+}
 
-    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 ();
-    }
+int
+__futex_abstimed_wait_cancelable64 (unsigned int* futex_word,
+                                    unsigned int expected, clockid_t clockid,
+                                    const struct __timespec64* abstime,
+                                    int private)
+{
+  return __futex_abstimed_wait_common64 (futex_word, expected, clockid,
+                                         abstime, private, true);
 }
 
 int
-- 
2.25.1

[PATCH 06/13] linux: nptl: Remove _futex_clock_wait_bitset64

[Adhemerval Zanella via Libc-alpha]

It can be replaced with a __futex_abstimed_wait64 call.

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

diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
index de88e9fc25..0ec47359be 100644
--- a/nptl/pthread_mutex_timedlock.c
+++ b/nptl/pthread_mutex_timedlock.c
@@ -265,12 +265,13 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
 	  assume_other_futex_waiters |= FUTEX_WAITERS;
 
 	  /* Block using the futex.  */
-	  int err = __futex_clock_wait_bitset64 (&mutex->__data.__lock,
+	  int err = __futex_abstimed_wait64 (
+	      (unsigned int *) &mutex->__data.__lock,
 	      oldval, clockid, abstime,
 	      PTHREAD_ROBUST_MUTEX_PSHARED (mutex));
 	  /* The futex call timed out.  */
-	  if (err == -ETIMEDOUT)
-	    return -err;
+	  if (err == ETIMEDOUT)
+	    return err;
 	  /* Reload current lock value.  */
 	  oldval = mutex->__data.__lock;
 	}
diff --git a/sysdeps/nptl/futex-internal.c b/sysdeps/nptl/futex-internal.c
index f9a2e28ee6..30c662547f 100644
--- a/sysdeps/nptl/futex-internal.c
+++ b/sysdeps/nptl/futex-internal.c
@@ -48,27 +48,6 @@ __futex_abstimed_wait_common32 (unsigned int* futex_word,
                                   pts32, NULL /* Unused.  */,
                                   FUTEX_BITSET_MATCH_ANY);
 }
-
-static int
-__futex_clock_wait_bitset32 (int *futexp, int val, clockid_t clockid,
-                             const struct __timespec64 *abstime, int private)
-{
-  struct timespec ts32;
-
-  if (abstime != NULL && ! in_time_t_range (abstime->tv_sec))
-    return -EOVERFLOW;
-
-  const unsigned int clockbit =
-    (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0;
-  const int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, private);
-
-  if (abstime != NULL)
-    ts32 = valid_timespec64_to_timespec (*abstime);
-
-  return INTERNAL_SYSCALL_CALL (futex, futexp, op, val,
-                                abstime != NULL ? &ts32 : NULL,
-                                NULL /* Unused.  */, FUTEX_BITSET_MATCH_ANY);
-}
 #endif /* ! __ASSUME_TIME64_SYSCALLS */
 
 static int
@@ -198,28 +177,3 @@ __futex_clocklock_wait64 (int *futex, int val, clockid_t clockid,
 
   return -err;
 }
-
-int
-__futex_clock_wait_bitset64 (int *futexp, int val, clockid_t clockid,
-                             const struct __timespec64 *abstime,
-                             int private)
-{
-  int ret;
-  if (! lll_futex_supported_clockid (clockid))
-    {
-      return -EINVAL;
-    }
-
-  const unsigned int clockbit =
-    (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0;
-  const int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, private);
-
-  ret = INTERNAL_SYSCALL_CALL (futex_time64, futexp, op, val,
-                               abstime, NULL /* Unused.  */,
-                               FUTEX_BITSET_MATCH_ANY);
-#ifndef __ASSUME_TIME64_SYSCALLS
-  if (ret == -ENOSYS)
-    ret = __futex_clock_wait_bitset32 (futexp, val, clockid, abstime, private);
-#endif
-  return ret;
-}
diff --git a/sysdeps/nptl/futex-internal.h b/sysdeps/nptl/futex-internal.h
index cefab74301..dcc7958d59 100644
--- a/sysdeps/nptl/futex-internal.h
+++ b/sysdeps/nptl/futex-internal.h
@@ -440,9 +440,4 @@ __futex_clocklock64 (int *futex, clockid_t clockid,
   return err;
 }
 
-int
-__futex_clock_wait_bitset64 (int *futexp, int val, clockid_t clockid,
-                             const struct __timespec64 *abstime,
-                             int private) attribute_hidden;
-
 #endif  /* futex-internal.h */
-- 
2.25.1

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

[Adhemerval Zanella via Libc-alpha]

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);
 		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;
         }
-- 
2.25.1

[PATCH 08/13] linux: nptl: Replace lll_timedwait with __futex_abstimed_wait64

[Adhemerval Zanella via Libc-alpha]

Checked with x86_64-linux-gnu and i686-linux-gnu.
---
 nptl/Makefile               |  1 -
 nptl/lll_timedlock_wait.c   | 62 -------------------------------------
 nptl/pthread_mutex_lock.c   |  4 +--
 sysdeps/nptl/lowlevellock.h | 32 -------------------
 4 files changed, 2 insertions(+), 97 deletions(-)
 delete mode 100644 nptl/lll_timedlock_wait.c

diff --git a/nptl/Makefile b/nptl/Makefile
index 74ab758c12..968768d33b 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -147,7 +147,6 @@ libpthread-routines = nptl-init nptlfreeres vars events version pt-interp \
 		      pt-longjmp pt-cleanup\
 		      cancellation \
 		      lowlevellock \
-		      lll_timedlock_wait \
 		      pt-fork pt-fcntl \
 		      $(pthread-compat-wrappers) \
 		      pt-raise pt-system \
diff --git a/nptl/lll_timedlock_wait.c b/nptl/lll_timedlock_wait.c
deleted file mode 100644
index eabdca70c8..0000000000
--- a/nptl/lll_timedlock_wait.c
+++ /dev/null
@@ -1,62 +0,0 @@
-/* Timed low level locking for pthread library.  Generic futex-using version.
-   Copyright (C) 2003-2020 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-   Contributed by Paul Mackerras <paulus@au.ibm.com>, 2003.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <https://www.gnu.org/licenses/>.  */
-
-#include <atomic.h>
-#include <errno.h>
-#include <lowlevellock.h>
-#include <sys/time.h>
-#include <time.h>
-
-
-int
-__lll_clocklock_wait (int *futex, int val, clockid_t clockid,
-		      const struct timespec *abstime, int private)
-{
-  struct timespec 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_gettime (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;
-    }
-
-  /* If *futex == val, wait until woken or timeout.  */
-  lll_futex_timed_wait (futex, val, tsp, private);
-
-  return 0;
-}
diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c
index 0439002454..1f137f6201 100644
--- a/nptl/pthread_mutex_lock.c
+++ b/nptl/pthread_mutex_lock.c
@@ -425,8 +425,8 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
 
 		/* Delay the thread indefinitely.  */
 		while (1)
-		  lll_timedwait (&(int){0}, 0, 0 /* ignored */, NULL,
-				 private);
+		  __futex_abstimed_wait64 (&(unsigned int){0}, 0,
+					   0 /* ignored */, NULL, private);
 	      }
 
 	    oldval = mutex->__data.__lock;
diff --git a/sysdeps/nptl/lowlevellock.h b/sysdeps/nptl/lowlevellock.h
index 68b3be8819..3de87d31a9 100644
--- a/sysdeps/nptl/lowlevellock.h
+++ b/sysdeps/nptl/lowlevellock.h
@@ -122,38 +122,6 @@ extern void __lll_lock_wait (int *futex, int private) attribute_hidden;
 #define lll_cond_lock(futex, private) __lll_cond_lock (&(futex), private)
 
 
-extern int __lll_clocklock_wait (int *futex, int val, clockid_t,
-				 const struct timespec *,
-				 int private) attribute_hidden;
-
-#define lll_timedwait(futex, val, clockid, abstime, private)		\
-  __lll_clocklock_wait (futex, val, clockid, abstime, private)
-
-/* As __lll_lock, but with an absolute timeout measured against the clock
-   specified in CLOCKID.  If the timeout occurs then return ETIMEDOUT. If
-   ABSTIME is invalid, return EINVAL.  */
-#define __lll_clocklock(futex, clockid, abstime, private)       \
-  ({                                                            \
-    int *__futex = (futex);                                     \
-    int __val = 0;                                              \
-                                                                \
-    if (__glibc_unlikely                                        \
-        (atomic_compare_and_exchange_bool_acq (__futex, 1, 0))) \
-      {								\
-	while (atomic_exchange_acq (futex, 2) != 0)		\
-	  {							\
-	    __val = __lll_clocklock_wait (__futex, 2, clockid, 	\
-					  abstime, private);	\
-	    if (__val == EINVAL || __val == ETIMEDOUT)		\
-	      break;						\
-	  }							\
-      }								\
-    __val;                                                      \
-  })
-#define lll_clocklock(futex, clockid, abstime, private)         \
-  __lll_clocklock (&(futex), clockid, abstime, private)
-
-
 /* This is an expression rather than a statement even though its value is
    void, so that it can be used in a comma expression or as an expression
    that's cast to void.  */
-- 
2.25.1

[PATCH 09/13] linux: nptl: Replace lll_futex_{timed_}wait by futex-internal.h

[Adhemerval Zanella via Libc-alpha]

The idea is to make NPTL implementation to use on the functions
provided by futex-internal.h.

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 nptl/lowlevellock.c                 | 6 +++---
 nptl/pthread_mutex_lock.c           | 9 +++++----
 nptl/pthread_mutex_setprioceiling.c | 5 +++--
 nptl/pthread_mutex_timedlock.c      | 6 +++---
 4 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/nptl/lowlevellock.c b/nptl/lowlevellock.c
index f69547a235..973df4d03a 100644
--- a/nptl/lowlevellock.c
+++ b/nptl/lowlevellock.c
@@ -18,7 +18,7 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <sysdep.h>
-#include <lowlevellock.h>
+#include <futex-internal.h>
 #include <atomic.h>
 #include <stap-probe.h>
 
@@ -32,7 +32,7 @@ __lll_lock_wait_private (int *futex)
     {
     futex:
       LIBC_PROBE (lll_lock_wait_private, 1, futex);
-      lll_futex_wait (futex, 2, LLL_PRIVATE); /* Wait if *futex == 2.  */
+      futex_wait ((unsigned int *) futex, 2, LLL_PRIVATE); /* Wait if *futex == 2.  */
     }
 }
 
@@ -49,7 +49,7 @@ __lll_lock_wait (int *futex, int private)
     {
     futex:
       LIBC_PROBE (lll_lock_wait, 1, futex);
-      lll_futex_wait (futex, 2, private); /* Wait if *futex == 2.  */
+      futex_wait ((unsigned int *) futex, 2, private); /* Wait if *futex == 2.  */
     }
 }
 #endif
diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c
index 1f137f6201..965c5a3f4a 100644
--- a/nptl/pthread_mutex_lock.c
+++ b/nptl/pthread_mutex_lock.c
@@ -307,8 +307,8 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
 	  assume_other_futex_waiters |= FUTEX_WAITERS;
 
 	  /* Block using the futex and reload current lock value.  */
-	  lll_futex_wait (&mutex->__data.__lock, oldval,
-			  PTHREAD_ROBUST_MUTEX_PSHARED (mutex));
+	  futex_wait ((unsigned int *) &mutex->__data.__lock, oldval,
+		      PTHREAD_ROBUST_MUTEX_PSHARED (mutex));
 	  oldval = mutex->__data.__lock;
 	}
 
@@ -568,8 +568,9 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
 		  break;
 
 		if (oldval != ceilval)
-		  lll_futex_wait (&mutex->__data.__lock, ceilval | 2,
-				  PTHREAD_MUTEX_PSHARED (mutex));
+		  futex_wait ((unsigned int * ) &mutex->__data.__lock,
+			      ceilval | 2,
+			      PTHREAD_MUTEX_PSHARED (mutex));
 	      }
 	    while (atomic_compare_and_exchange_val_acq (&mutex->__data.__lock,
 							ceilval | 2, ceilval)
diff --git a/nptl/pthread_mutex_setprioceiling.c b/nptl/pthread_mutex_setprioceiling.c
index 521da72622..cbef202579 100644
--- a/nptl/pthread_mutex_setprioceiling.c
+++ b/nptl/pthread_mutex_setprioceiling.c
@@ -21,6 +21,7 @@
 #include <errno.h>
 #include <pthreadP.h>
 #include <atomic.h>
+#include <futex-internal.h>
 
 
 int
@@ -84,8 +85,8 @@ pthread_mutex_setprioceiling (pthread_mutex_t *mutex, int prioceiling,
 	      break;
 
 	    if (oldval != ceilval)
-	      lll_futex_wait (&mutex->__data.__lock, ceilval | 2,
-			      PTHREAD_MUTEX_PSHARED (mutex));
+	      futex_wait ((unsigned int *) &mutex->__data.__lock, ceilval | 2,
+			  PTHREAD_MUTEX_PSHARED (mutex));
 	  }
 	while (atomic_compare_and_exchange_val_acq (&mutex->__data.__lock,
 						    ceilval | 2, ceilval)
diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
index e643eab258..343acf6107 100644
--- a/nptl/pthread_mutex_timedlock.c
+++ b/nptl/pthread_mutex_timedlock.c
@@ -561,9 +561,9 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
 			goto failpp;
 		      }
 
-		    lll_futex_timed_wait (&mutex->__data.__lock,
-					  ceilval | 2, &rt,
-					  PTHREAD_MUTEX_PSHARED (mutex));
+		    __futex_abstimed_wait64 (
+		      (unsigned int *) &mutex->__data.__lock, clockid,
+		      ceilval | 2, &rt, PTHREAD_MUTEX_PSHARED (mutex));
 		  }
 	      }
 	    while (atomic_compare_and_exchange_val_acq (&mutex->__data.__lock,
-- 
2.25.1

[PATCH 10/13] linux: nptl: Replace lll_futex_supported_clockid with futex-internal.h

[Adhemerval Zanella via Libc-alpha]

The idea is to make NPTL implementation to use on the functions
provided by futex-internal.h.

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 nptl/pthread_mutex_timedlock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
index 343acf6107..b42862193a 100644
--- a/nptl/pthread_mutex_timedlock.c
+++ b/nptl/pthread_mutex_timedlock.c
@@ -600,7 +600,7 @@ __pthread_mutex_clocklock64 (pthread_mutex_t *mutex,
 			     clockid_t clockid,
 			     const struct __timespec64 *abstime)
 {
-  if (__glibc_unlikely (!lll_futex_supported_clockid (clockid)))
+  if (__glibc_unlikely (!futex_abstimed_supported_clockid (clockid)))
     return EINVAL;
 
   LIBC_PROBE (mutex_clocklock_entry, 3, mutex, clockid, abstime);
-- 
2.25.1

[PATCH 11/13] nptl: Replace lll_futex_wake with futex-internal.h

[Adhemerval Zanella via Libc-alpha]

The idea is to make NPTL implementation to use on the functions
provided by futex-internal.h.

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 nptl/pthread_mutex_setprioceiling.c | 4 ++--
 nptl/pthread_mutex_unlock.c         | 6 +++---
 nptl/sem_post.c                     | 9 ++-------
 3 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/nptl/pthread_mutex_setprioceiling.c b/nptl/pthread_mutex_setprioceiling.c
index cbef202579..8f1d6e1326 100644
--- a/nptl/pthread_mutex_setprioceiling.c
+++ b/nptl/pthread_mutex_setprioceiling.c
@@ -116,8 +116,8 @@ pthread_mutex_setprioceiling (pthread_mutex_t *mutex, int prioceiling,
 			 | (prioceiling << PTHREAD_MUTEX_PRIO_CEILING_SHIFT);
   atomic_full_barrier ();
 
-  lll_futex_wake (&mutex->__data.__lock, INT_MAX,
-		  PTHREAD_MUTEX_PSHARED (mutex));
+  futex_wake ((unsigned int *)&mutex->__data.__lock, INT_MAX,
+	      PTHREAD_MUTEX_PSHARED (mutex));
 
   return 0;
 }
diff --git a/nptl/pthread_mutex_unlock.c b/nptl/pthread_mutex_unlock.c
index 2b4abb8ebe..56f1732e6d 100644
--- a/nptl/pthread_mutex_unlock.c
+++ b/nptl/pthread_mutex_unlock.c
@@ -162,7 +162,7 @@ __pthread_mutex_unlock_full (pthread_mutex_t *mutex, int decr)
       private = PTHREAD_ROBUST_MUTEX_PSHARED (mutex);
       if (__glibc_unlikely ((atomic_exchange_rel (&mutex->__data.__lock, 0)
 			     & FUTEX_WAITERS) != 0))
-	lll_futex_wake (&mutex->__data.__lock, 1, private);
+	futex_wake ((unsigned int *) &mutex->__data.__lock, 1, private);
 
       /* We must clear op_pending after we release the mutex.
 	 FIXME However, this violates the mutex destruction requirements
@@ -332,8 +332,8 @@ __pthread_mutex_unlock_full (pthread_mutex_t *mutex, int decr)
 						    &oldval, newval));
 
       if ((oldval & ~PTHREAD_MUTEX_PRIO_CEILING_MASK) > 1)
-	lll_futex_wake (&mutex->__data.__lock, 1,
-			PTHREAD_MUTEX_PSHARED (mutex));
+	futex_wake ((unsigned int *)&mutex->__data.__lock, 1,
+		    PTHREAD_MUTEX_PSHARED (mutex));
 
       int oldprio = newval >> PTHREAD_MUTEX_PRIO_CEILING_SHIFT;
 
diff --git a/nptl/sem_post.c b/nptl/sem_post.c
index 88cfc24b30..5dbfb3a214 100644
--- a/nptl/sem_post.c
+++ b/nptl/sem_post.c
@@ -84,19 +84,14 @@ int
 attribute_compat_text_section
 __old_sem_post (sem_t *sem)
 {
-  int *futex = (int *) sem;
+  unsigned int *futex = (unsigned int *) sem;
 
   /* We must need to synchronize with consumers of this token, so the atomic
      increment must have release MO semantics.  */
   atomic_write_barrier ();
   (void) atomic_increment_val (futex);
   /* We always have to assume it is a shared semaphore.  */
-  int err = lll_futex_wake (futex, 1, LLL_SHARED);
-  if (__builtin_expect (err, 0) < 0)
-    {
-      __set_errno (-err);
-      return -1;
-    }
+  futex_wake (futex, 1, LLL_SHARED);
   return 0;
 }
 compat_symbol (libpthread, __old_sem_post, sem_post, GLIBC_2_0);
-- 
2.25.1

[PATCH v2 12/13] nptl: Return EINVAL for pthread_mutex_clocklock/PI with CLOCK_MONOTONIC [BZ #26801]

[Adhemerval Zanella via Libc-alpha]

Changes from previous version:

  - Replace ENOTSUP by EINVAL

--

Linux futex FUTEX_LOCK_PI operation only supports CLOCK_REALTIME,
so pthread_mutex_clocklock operation with priority aware mutexes
may fail depending of the input timeout.

Also, it is not possible to convert a CLOCK_MONOTONIC to a
CLOCK_REALTIME due the possible wall clock time change which might
invalid the requested timeout.

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 nptl/Makefile                  |  2 +-
 nptl/pthread_mutex_timedlock.c |  7 ++++
 nptl/tst-mutexpi10.c           | 68 ++++++++++++++++++++++++++++++++++
 sysdeps/pthread/tst-mutex5.c   |  2 +
 sysdeps/pthread/tst-mutex9.c   |  2 +
 5 files changed, 80 insertions(+), 1 deletion(-)
 create mode 100644 nptl/tst-mutexpi10.c

diff --git a/nptl/Makefile b/nptl/Makefile
index 968768d33b..a48426a396 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -265,7 +265,7 @@ tests = tst-attr2 tst-attr3 tst-default-attr \
 	tst-mutex5a tst-mutex7a \
 	tst-mutexpi1 tst-mutexpi2 tst-mutexpi3 tst-mutexpi4 \
 	tst-mutexpi5 tst-mutexpi5a tst-mutexpi6 tst-mutexpi7 tst-mutexpi7a \
-	tst-mutexpi9 \
+	tst-mutexpi9 tst-mutexpi10 \
 	tst-cond22 tst-cond26 \
 	tst-robustpi1 tst-robustpi2 tst-robustpi3 tst-robustpi4 tst-robustpi5 \
 	tst-robustpi6 tst-robustpi7 tst-robustpi9 \
diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
index b42862193a..aaaafa21ce 100644
--- a/nptl/pthread_mutex_timedlock.c
+++ b/nptl/pthread_mutex_timedlock.c
@@ -313,6 +313,13 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
     case PTHREAD_MUTEX_PI_ROBUST_NORMAL_NP:
     case PTHREAD_MUTEX_PI_ROBUST_ADAPTIVE_NP:
       {
+	/* Currently futex FUTEX_LOCK_PI operation only provides support for
+	   CLOCK_REALTIME and trying to emulate by converting a
+	   CLOCK_MONOTONIC to CLOCK_REALTIME will take in account possible
+	   changes to the wall clock.  */
+	if (__glibc_unlikely (clockid != CLOCK_REALTIME))
+	  return EINVAL;
+
 	int kind, robust;
 	{
 	  /* See concurrency notes regarding __kind in struct __pthread_mutex_s
diff --git a/nptl/tst-mutexpi10.c b/nptl/tst-mutexpi10.c
new file mode 100644
index 0000000000..84ba1dfa97
--- /dev/null
+++ b/nptl/tst-mutexpi10.c
@@ -0,0 +1,68 @@
+/* Check if pthread_mutex_clocklock with PRIO_INHERIT fails with clock
+   different than CLOCK_REALTIME.
+   Copyright (C) 2015-2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <pthread.h>
+#include <errno.h>
+#include <array_length.h>
+
+#include <support/check.h>
+#include <support/xthread.h>
+#include <support/timespec.h>
+
+static int
+do_test (void)
+{
+  const int types[] = {
+    PTHREAD_MUTEX_NORMAL,
+    PTHREAD_MUTEX_ERRORCHECK,
+    PTHREAD_MUTEX_RECURSIVE,
+    PTHREAD_MUTEX_ADAPTIVE_NP
+  };
+  const int robust[] = {
+    PTHREAD_MUTEX_STALLED,
+    PTHREAD_MUTEX_ROBUST
+  };
+
+
+  for (int t = 0; t < array_length (types); t++)
+    for (int r = 0; r < array_length (robust); r++)
+      {
+	pthread_mutexattr_t attr;
+
+	xpthread_mutexattr_init (&attr);
+	xpthread_mutexattr_setprotocol (&attr, PTHREAD_PRIO_INHERIT);
+	xpthread_mutexattr_settype (&attr, types[t]);
+	xpthread_mutexattr_setrobust (&attr, robust[r]);
+
+	pthread_mutex_t mtx;
+	xpthread_mutex_init (&mtx, &attr);
+
+	struct timespec tmo = timespec_add (xclock_now (CLOCK_MONOTONIC),
+					    make_timespec (0, 100000000));
+
+	TEST_COMPARE (pthread_mutex_clocklock (&mtx, CLOCK_MONOTONIC, &tmo),
+		      EINVAL);
+
+	xpthread_mutex_destroy (&mtx);
+      }
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/pthread/tst-mutex5.c b/sysdeps/pthread/tst-mutex5.c
index 14490768c3..bfe1a79fa4 100644
--- a/sysdeps/pthread/tst-mutex5.c
+++ b/sysdeps/pthread/tst-mutex5.c
@@ -112,7 +112,9 @@ static int do_test (void)
 {
   do_test_clock (CLOCK_USE_TIMEDLOCK, "timedlock");
   do_test_clock (CLOCK_REALTIME, "clocklock(realtime)");
+#ifndef ENABLE_PI
   do_test_clock (CLOCK_MONOTONIC, "clocklock(monotonic)");
+#endif
   return 0;
 }
 
diff --git a/sysdeps/pthread/tst-mutex9.c b/sysdeps/pthread/tst-mutex9.c
index 2d7927b7c2..bfc01f8c75 100644
--- a/sysdeps/pthread/tst-mutex9.c
+++ b/sysdeps/pthread/tst-mutex9.c
@@ -133,7 +133,9 @@ do_test (void)
 {
   do_test_clock (CLOCK_USE_TIMEDLOCK);
   do_test_clock (CLOCK_REALTIME);
+#ifndef ENABLE_PI
   do_test_clock (CLOCK_MONOTONIC);
+#endif
   return 0;
 }
 
-- 
2.25.1

[PATCH 13/13] linux: Return EINVAL for invalid clock for pthread_clockjoin_np

[Adhemerval Zanella via Libc-alpha]

The align the GNU extension with the others one that accept specify
which clock to wait for (such as pthread_mutex_clocklock).

Check on x86_64-linux-gnu.
---
 nptl/pthread_clockjoin.c     |  4 ++
 sysdeps/pthread/Makefile     |  2 +-
 sysdeps/pthread/tst-join15.c | 80 ++++++++++++++++++++++++++++++++++++
 3 files changed, 85 insertions(+), 1 deletion(-)
 create mode 100644 sysdeps/pthread/tst-join15.c

diff --git a/nptl/pthread_clockjoin.c b/nptl/pthread_clockjoin.c
index 0baba1e83d..3d54fe588f 100644
--- a/nptl/pthread_clockjoin.c
+++ b/nptl/pthread_clockjoin.c
@@ -17,12 +17,16 @@
    <http://www.gnu.org/licenses/>.  */
 
 #include <time.h>
+#include <futex-internal.h>
 #include "pthreadP.h"
 
 int
 __pthread_clockjoin_np64 (pthread_t threadid, void **thread_return,
                           clockid_t clockid, const struct __timespec64 *abstime)
 {
+  if (!futex_abstimed_supported_clockid (clockid))
+    return EINVAL;
+
   return __pthread_clockjoin_ex (threadid, thread_return,
                                  clockid, abstime, true);
 }
diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile
index 45a15b0b1a..8f335c13b5 100644
--- a/sysdeps/pthread/Makefile
+++ b/sysdeps/pthread/Makefile
@@ -77,7 +77,7 @@ tests += tst-cnd-basic tst-mtx-trylock tst-cnd-broadcast \
 	 tst-getpid3 \
 	 tst-join1 tst-join2 tst-join3 tst-join4 tst-join5 tst-join6 tst-join7 \
 	 tst-join8 tst-join9 tst-join10 tst-join11 tst-join12 tst-join13 \
-	 tst-join14 \
+	 tst-join14 tst-join15 \
 	 tst-key1 tst-key2 tst-key3 tst-key4 \
 	 tst-kill1 tst-kill2 tst-kill3 tst-kill4 tst-kill5 tst-kill6 \
 	 tst-locale1 tst-locale2 \
diff --git a/sysdeps/pthread/tst-join15.c b/sysdeps/pthread/tst-join15.c
new file mode 100644
index 0000000000..4ed767e733
--- /dev/null
+++ b/sysdeps/pthread/tst-join15.c
@@ -0,0 +1,80 @@
+/* Check pthread_clockjoin_np clock support.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <pthread.h>
+#include <sys/time.h>
+#include <unistd.h>
+#include <errno.h>
+
+#include <array_length.h>
+#include <support/check.h>
+#include <support/timespec.h>
+#include <support/xthread.h>
+
+static void *
+tf (void *arg)
+{
+  pause ();
+  return NULL;
+}
+
+
+static int
+do_test (void)
+{
+  const clockid_t clocks[] = {
+    CLOCK_REALTIME,
+    CLOCK_MONOTONIC,
+    CLOCK_PROCESS_CPUTIME_ID,
+    CLOCK_THREAD_CPUTIME_ID,
+    CLOCK_THREAD_CPUTIME_ID,
+    CLOCK_MONOTONIC_RAW,
+    CLOCK_REALTIME_COARSE,
+    CLOCK_MONOTONIC_COARSE,
+#ifdef CLOCK_BOOTTIME
+    CLOCK_BOOTTIME,
+#endif
+#ifdef CLOCK_REALTIME_ALARM
+    CLOCK_REALTIME_ALARM,
+#endif
+#ifdef CLOCK_BOOTTIME_ALARM
+    CLOCK_BOOTTIME_ALARM,
+#endif
+#ifdef CLOCK_TAI
+    CLOCK_TAI
+#endif
+  };
+
+  pthread_t thr = xpthread_create (NULL, tf, NULL);
+
+  for (int t = 0; t < array_length (clocks); t++)
+    {
+      /* A valid timeout so valid clock timeout.  */
+      struct timespec tmo = timespec_add (xclock_now (clocks[t]),
+					  make_timespec (0, 100000000));
+
+      int ret = clocks[t] == CLOCK_REALTIME || clocks[t] == CLOCK_MONOTONIC
+		? ETIMEDOUT : EINVAL;
+
+      TEST_COMPARE (pthread_clockjoin_np (thr, NULL, clocks[t], &tmo), ret);
+    }
+
+  return 0;
+}
+
+#include <support/test-driver.c>
-- 
2.25.1

Re: [PATCH 01/13] linux: Remove unused internal futex functions

[Lukasz Majewski]

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

Hi Adhemerval,

> The __futex_abstimed_wait usage was remove with 3102e28bd11 and the
> __futex_abstimed_wait_cancelable by 323592fdc92 and b8d3e8fbaac.
> The futex_lock_pi can be replaced by a futex_lock_pi64.
> 
> Checked on x86_64-linux-gnu and i686-linux-gnu.
> ---
>  nptl/pthread_mutex_lock.c     |  3 +-
>  sysdeps/nptl/futex-internal.h | 98
> ----------------------------------- 2 files changed, 1 insertion(+),
> 100 deletions(-)
> 
> diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c
> index fac774e608..0439002454 100644
> --- a/nptl/pthread_mutex_lock.c
> +++ b/nptl/pthread_mutex_lock.c
> @@ -413,8 +413,7 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
>  	    int private = (robust
>  			   ? PTHREAD_ROBUST_MUTEX_PSHARED (mutex)
>  			   : PTHREAD_MUTEX_PSHARED (mutex));
> -	    int e = futex_lock_pi ((unsigned int *)
> &mutex->__data.__lock,
> -				   NULL, private);
> +	    int e = futex_lock_pi64 (&mutex->__data.__lock, NULL,
> private); if (e == ESRCH || e == EDEADLK)
>  	      {
>  		assert (e != EDEADLK
> diff --git a/sysdeps/nptl/futex-internal.h
> b/sysdeps/nptl/futex-internal.h index c27d0cdac8..21e3b74be6 100644
> --- a/sysdeps/nptl/futex-internal.h
> +++ b/sysdeps/nptl/futex-internal.h
> @@ -275,76 +275,6 @@ futex_abstimed_supported_clockid (clockid_t
> clockid) return lll_futex_supported_clockid (clockid);
>  }
>  
> -/* Like futex_reltimed_wait, but the provided timeout (ABSTIME) is an
> -   absolute point in time; a call will time out after this point in
> time.  */ -static __always_inline int
> -futex_abstimed_wait (unsigned int* futex_word, unsigned int expected,
> -		     clockid_t clockid,
> -		     const struct timespec* abstime, int private)
> -{
> -  /* 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;
> -  int err = lll_futex_clock_wait_bitset (futex_word, expected,
> -					 clockid, abstime,
> -					 private);
> -  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 ();
> -    }
> -}
> -
> -/* Like futex_reltimed_wait but is a POSIX cancellation point.  */
> -static __always_inline int
> -futex_abstimed_wait_cancelable (unsigned int* futex_word,
> -				unsigned int expected,
> -				clockid_t clockid,
> -			        const struct timespec* abstime, int
> private) -{
> -  /* 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;
> -  int oldtype;
> -  oldtype = __pthread_enable_asynccancel ();
> -  int err = lll_futex_clock_wait_bitset (futex_word, expected,
> -					clockid, abstime,
> -					private);
> -  __pthread_disable_asynccancel (oldtype);
> -  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 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 ();
> -    }
> -}
> -
>  /* Atomically wrt other futex operations on the same futex, this
> unblocks the specified number of processes, or all processes blocked
> on this futex if there are fewer than the specified number.
> Semantically, this is @@ -410,34 +340,6 @@ futex_wake (unsigned int*
> futex_word, int processes_to_wake, int private) futex.
>       - ETIMEDOUT if the ABSTIME expires.
>  */
> -static __always_inline int
> -futex_lock_pi (unsigned int *futex_word, const struct timespec
> *abstime,
> -	       int private)
> -{
> -  int err = lll_futex_timed_lock_pi (futex_word, abstime, private);
> -  switch (err)
> -    {
> -    case 0:
> -    case -EAGAIN:
> -    case -EINTR:
> -    case -ETIMEDOUT:
> -    case -ESRCH:
> -    case -EDEADLK:
> -    case -EINVAL: /* This indicates either state corruption or that
> the kernel
> -		     found a waiter on futex address which is
> waiting via
> -		     FUTEX_WAIT or FUTEX_WAIT_BITSET.  This is
> reported on
> -		     some futex_lock_pi usage
> (pthread_mutex_timedlock for
> -		     instance).  */
> -      return -err;
> -
> -    case -EFAULT: /* 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 ();
> -    }
> -}
> -
>  static __always_inline int
>  futex_lock_pi64 (int *futex_word, const struct __timespec64 *abstime,
>                   int private)

It is great that we can consolidate the thread code and remove one not
needed.

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


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 --]

Re: [PATCH 02/13] nptl: Remove futex_wait_cancelable

[Lukasz Majewski]

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

Hi Adhemerval,

> It is used solely on __pthread_cond_wait_common and the call can be
> replaced by a __futex_abstimed_wait_cancelable64 one.
> 
> Checked on x86_64-linux-gnu and i686-linux-gnu.
> ---
>  nptl/pthread_cond_wait.c      | 22 ++--------------------
>  sysdeps/nptl/futex-internal.h | 29 -----------------------------
>  2 files changed, 2 insertions(+), 49 deletions(-)
> 
> diff --git a/nptl/pthread_cond_wait.c b/nptl/pthread_cond_wait.c
> index 7d158d553f..685dbca32f 100644
> --- a/nptl/pthread_cond_wait.c
> +++ b/nptl/pthread_cond_wait.c
> @@ -501,26 +501,8 @@ __pthread_cond_wait_common (pthread_cond_t
> *cond, pthread_mutex_t *mutex, cbuffer.private = private;
>  	  __pthread_cleanup_push (&buffer,
> __condvar_cleanup_waiting, &cbuffer); 
> -	  if (abstime == NULL)
> -	    {
> -	      /* Block without a timeout.  */
> -	      err = futex_wait_cancelable (
> -		  cond->__data.__g_signals + g, 0, private);
> -	    }
> -	  else
> -	    {
> -	      /* Block, but with a timeout.
> -		 Work around the fact that the kernel rejects
> negative timeout
> -		 values despite them being valid.  */
> -	      if (__glibc_unlikely (abstime->tv_sec < 0))
> -	        err = ETIMEDOUT;
> -	      else
> -		{
> -		  err = __futex_abstimed_wait_cancelable64
> -                    (cond->__data.__g_signals + g, 0, clockid,
> abstime,
> -                     private);
> -		}
> -	    }
> +	  err = __futex_abstimed_wait_cancelable64 (
> +	    cond->__data.__g_signals + g, 0, clockid, abstime,
> private); 

The __futex_abstimed_wait_cancelable64 can handle NULL abstime. Thanks
for cleaning this up :-).

>  	  __pthread_cleanup_pop (&buffer, 0);
>  
> diff --git a/sysdeps/nptl/futex-internal.h
> b/sysdeps/nptl/futex-internal.h index 21e3b74be6..96d1318091 100644
> --- a/sysdeps/nptl/futex-internal.h
> +++ b/sysdeps/nptl/futex-internal.h
> @@ -177,35 +177,6 @@ futex_wait_simple (unsigned int *futex_word,
> unsigned int expected, ignore_value (futex_wait (futex_word,
> expected, private)); }
>  
> -
> -/* Like futex_wait but is a POSIX cancellation point.  */
> -static __always_inline int
> -futex_wait_cancelable (unsigned int *futex_word, unsigned int
> expected,
> -		       int private)
> -{
> -  int oldtype;
> -  oldtype = __pthread_enable_asynccancel ();
> -  int err = lll_futex_timed_wait (futex_word, expected, NULL,
> private);
> -  __pthread_disable_asynccancel (oldtype);
> -  switch (err)
> -    {
> -    case 0:
> -    case -EAGAIN:
> -    case -EINTR:
> -      return -err;
> -
> -    case -ETIMEDOUT: /* Cannot have happened as we provided no
> timeout.  */
> -    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:
> -      futex_fatal_error ();
> -    }
> -}
> -
>  /* Like futex_wait, but will eventually time out (i.e., stop being
>     blocked) after the duration of time provided (i.e., RELTIME) has
>     passed.  The caller must provide a normalized RELTIME.  RELTIME
> can also

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


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 --]

Re: [PATCH 03/13] nptl: Remove clockwait_tid

[Lukasz Majewski]

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

On Mon, 23 Nov 2020 16:52:46 -0300
Adhemerval Zanella via Libc-alpha <libc-alpha@sourceware.org> wrote:

> It can be replaced with a __futex_abstimed_wait_cancelable64 call,
> with the advantage that there is no need to further clock adjustments
> to create a absolute timeout.  It allows to remove the now ununsed
> futex_timed_wait_cancel64 internal function.
> 
> Checked on x86_64-linux-gnu and i686-linux-gnu.
> ---
>  nptl/pthread_join_common.c    | 70
> ++++++----------------------------- sysdeps/nptl/futex-internal.h |
> 49 ------------------------ 2 files changed, 12 insertions(+), 107
> deletions(-)
> 
> diff --git a/nptl/pthread_join_common.c b/nptl/pthread_join_common.c
> index 67d8e2b780..8a95c58ff3 100644
> --- a/nptl/pthread_join_common.c
> +++ b/nptl/pthread_join_common.c
> @@ -32,55 +32,6 @@ cleanup (void *arg)
>    atomic_compare_exchange_weak_acquire (&arg, &self, NULL);
>  }
>  
> -/* The kernel notifies a process which uses CLONE_CHILD_CLEARTID via
> futex
> -   wake-up when the clone terminates.  The memory location contains
> the
> -   thread ID while the clone is running and is reset to zero by the
> kernel
> -   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 __timespec64 *abstime)
> -{
> -  pid_t tid;
> -  int ret;
> -
> -  if (! valid_nanoseconds (abstime->tv_nsec))
> -    return EINVAL;
> -
> -  /* Repeat until thread terminated.  */
> -  while ((tid = *tidp) != 0)
> -    {
> -      struct __timespec64 rt;
> -
> -      /* Get the current time. This can only fail if clockid is
> -         invalid. */
> -      if (__glibc_unlikely (__clock_gettime64 (clockid, &rt)))
> -        return EINVAL;
> -
> -      /* Compute relative timeout.  */
> -      rt.tv_sec = abstime->tv_sec - rt.tv_sec;
> -      rt.tv_nsec = abstime->tv_nsec - rt.tv_nsec;
> -      if (rt.tv_nsec < 0)
> -        {
> -          rt.tv_nsec += 1000000000;
> -          --rt.tv_sec;
> -        }
> -
> -      /* Already timed out?  */
> -      if (rt.tv_sec < 0)
> -        return ETIMEDOUT;
> -
> -      /* 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.  */
> -      ret = futex_timed_wait_cancel64 (tidp, tid, &rt, LLL_SHARED);
> -      if (ret == -ETIMEDOUT || ret == -EOVERFLOW)
> -        return -ret;
> -    }
> -
> -  return 0;
> -}
> -
>  int
>  __pthread_clockjoin_ex (pthread_t threadid, void **thread_return,
>                          clockid_t clockid,
> @@ -137,15 +88,18 @@ __pthread_clockjoin_ex (pthread_t threadid, void
> **thread_return, un-wait-ed for again.  */
>        pthread_cleanup_push (cleanup, &pd->joinid);
>  
> -      if (abstime != NULL)
> -	result = clockwait_tid (&pd->tid, clockid, abstime);
> -      else
> -	{
> -	  pid_t tid;
> -	  /* We need acquire MO here so that we synchronize with the
> -	     kernel's store to 0 when the clone terminates. (see
> above)  */
> -	  while ((tid = atomic_load_acquire (&pd->tid)) != 0)
> -	    lll_futex_wait_cancel (&pd->tid, tid, LLL_SHARED);
> +     /* We need acquire MO here so that we synchronize with the
> +        kernel's store to 0 when the clone terminates. (see above)
> */
> +      pid_t tid;
> +      while ((tid = atomic_load_acquire (&pd->tid)) != 0)
> +        {
> +	  int ret = __futex_abstimed_wait_cancelable64 (
> +	    (unsigned int *) &pd->tid, tid, clockid, abstime,
> LLL_SHARED);
> +	  if (ret == ETIMEDOUT || ret == EOVERFLOW)
> +	    {
> +	      result = ret;
> +	      break;
> +	    }
>  	}
>  
>        pthread_cleanup_pop (0);
> diff --git a/sysdeps/nptl/futex-internal.h
> b/sysdeps/nptl/futex-internal.h index 96d1318091..d5f13d15fb 100644
> --- a/sysdeps/nptl/futex-internal.h
> +++ b/sysdeps/nptl/futex-internal.h
> @@ -390,55 +390,6 @@ futex_unlock_pi (unsigned int *futex_word, int
> private) }
>  }
>  
> -static __always_inline int
> -futex_timed_wait_cancel64 (pid_t *tidp,  pid_t tid,
> -                           const struct __timespec64 *timeout, int
> private) -{
> -  int err = INTERNAL_SYSCALL_CANCEL (futex_time64, tidp,
> -                                     __lll_private_flag (FUTEX_WAIT,
> private),
> -                                     tid, timeout);
> -#ifndef __ASSUME_TIME64_SYSCALLS
> -  if (err == -ENOSYS)
> -    {
> -      if (in_time_t_range (timeout->tv_sec))
> -        {
> -          struct timespec ts32 = valid_timespec64_to_timespec
> (*timeout); -
> -          err = INTERNAL_SYSCALL_CANCEL (futex, tidp,
> -                                         __lll_private_flag
> (FUTEX_WAIT,
> -
> private),
> -                                         tid, &ts32);
> -        }
> -      else
> -        err = -EOVERFLOW;
> -    }
> -#endif
> -  switch (err)
> -    {
> -    case 0:
> -    case -EAGAIN:
> -    case -EINTR:
> -    case -ETIMEDOUT:
> -    case -EDEADLK:
> -    case -ENOSYS:
> -    case -EOVERFLOW:  /* Passed absolute timeout uses 64 bit time_t
> type, but
> -                         underlying kernel does not support 64 bit
> time_t futex
> -                         syscalls.  */
> -    case -EPERM:  /*  The caller is not allowed to attach itself to
> the futex.
> -		      Used to check if PI futexes are supported by
> the
> -		      kernel.  */
> -      return -err;
> -
> -    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 -EFAULT: /* Must have been caused by a glibc or application
> bug.  */
> -    /* No other errors are documented at this time.  */
> -    default:
> -      futex_fatal_error ();
> -    }
> -}
> -
>  /* The futex_abstimed_wait_cancelable64 has been moved to a separate
> file to avoid problems with exhausting available registers on some
> architectures
>     - e.g. on m68k architecture.  */

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


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 --]

Re: [PATCH 04/13] linux: Extend __futex_abstimed_wait_cancelable64 comment

[Lukasz Majewski]

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

Hi Adhemerval,

> And add a small optimization to avoid setting the operation for the
> 32-bit time fallback operation.
> 
> Checked on x86_64-linux-gnu and i686-linux-gnu.
> ---
>  sysdeps/nptl/futex-internal.c |  8 ++------
>  sysdeps/nptl/futex-internal.h | 18 +++++++++++++++---
>  2 files changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/sysdeps/nptl/futex-internal.c
> b/sysdeps/nptl/futex-internal.c index 457cd3cd69..e4a14b477c 100644
> --- a/sysdeps/nptl/futex-internal.c
> +++ b/sysdeps/nptl/futex-internal.c
> @@ -25,7 +25,7 @@
>  #ifndef __ASSUME_TIME64_SYSCALLS
>  static int
>  __futex_abstimed_wait_cancelable32 (unsigned int* futex_word,
> -                                    unsigned int expected, clockid_t
> clockid,
> +                                    unsigned int expected, int op,
>                                      const struct __timespec64*
> abstime, int private)
>  {
> @@ -39,10 +39,6 @@ __futex_abstimed_wait_cancelable32 (unsigned int*
> futex_word, pts32 = &ts32;
>      }
>  
> -  unsigned int clockbit = (clockid == CLOCK_REALTIME)
> -	  ? FUTEX_CLOCK_REALTIME : 0;
> -  int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit,
> private); -
>    return INTERNAL_SYSCALL_CANCEL (futex, futex_word, op, expected,
>                                    pts32, NULL /* Unused.  */,
>                                    FUTEX_BITSET_MATCH_ANY);
> @@ -119,7 +115,7 @@ __futex_abstimed_wait_cancelable64 (unsigned int*
> futex_word, #ifndef __ASSUME_TIME64_SYSCALLS
>    if (err == -ENOSYS)
>      err = __futex_abstimed_wait_cancelable32 (futex_word, expected,
> -                                              clockid, abstime,
> private);
> +                                              op, abstime, private);
>  #endif
>  
>    switch (err)
> diff --git a/sysdeps/nptl/futex-internal.h
> b/sysdeps/nptl/futex-internal.h index d5f13d15fb..cefab74301 100644
> --- a/sysdeps/nptl/futex-internal.h
> +++ b/sysdeps/nptl/futex-internal.h
> @@ -390,9 +390,21 @@ futex_unlock_pi (unsigned int *futex_word, int
> private) }
>  }
>  
> -/* The futex_abstimed_wait_cancelable64 has been moved to a separate
> file
> -   to avoid problems with exhausting available registers on some
> architectures
> -   - e.g. on m68k architecture.  */
> +/* Like futex_wait, but will eventually time out (i.e., stop being
> blocked)
> +   after the duration of time provided (i.e., ABSTIME) has passed
> using the
> +   clock specified by CLOCKID (currently only CLOCK_REALTIME and
> +   CLOCK_MONOTONIC, the ones support by
> lll_futex_supported_clockid). ABSTIME
> +   can also equal NULL, in which case this function behaves
> equivalent to
> +   futex_wait.
> +
> +   Returns the same values as futex_wait under those same conditions;
> +   additionally, returns ETIMEDOUT if the timeout expired.
> +
> +   The call acts a cancellation entrypoint.
> +
> +   (The implementation has been moved to a separate file to avoid
> problems
> +   with exhausting available registers on some architectures - e.g.
> on
> +   m68k).  */
>  int
>  __futex_abstimed_wait_cancelable64 (unsigned int* futex_word,
>                                      unsigned int expected, clockid_t
> clockid,

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


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 --]

Re: [PATCH 05/13] linux: nptl: Consolidate __futex_abstimed_wait_{cancelable}64

[Lukasz Majewski]

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

Hi Adhemerval,

> The only different is how to issue the syscall.
> 
> Checked on x86_64-linux-gnu and i686-linux-gnu.
> ---
>  sysdeps/nptl/futex-internal.c | 111
> +++++++++++----------------------- 1 file changed, 35 insertions(+),
> 76 deletions(-)
> 
> diff --git a/sysdeps/nptl/futex-internal.c
> b/sysdeps/nptl/futex-internal.c index e4a14b477c..f9a2e28ee6 100644
> --- a/sysdeps/nptl/futex-internal.c
> +++ b/sysdeps/nptl/futex-internal.c
> @@ -24,10 +24,10 @@
>  
>  #ifndef __ASSUME_TIME64_SYSCALLS
>  static int
> -__futex_abstimed_wait_cancelable32 (unsigned int* futex_word,
> -                                    unsigned int expected, int op,
> -                                    const struct __timespec64*
> abstime,
> -                                    int private)
> +__futex_abstimed_wait_common32 (unsigned int* futex_word,
> +                                unsigned int expected, int op,
> +                                const struct __timespec64* abstime,
> +                                int private, bool cancel)
>  {
>    struct timespec ts32, *pts32 = NULL;
>    if (abstime != NULL)
> @@ -39,34 +39,16 @@ __futex_abstimed_wait_cancelable32 (unsigned int*
> futex_word, pts32 = &ts32;
>      }
>  
> -  return INTERNAL_SYSCALL_CANCEL (futex, futex_word, op, expected,
> +  if (cancel)
> +    return INTERNAL_SYSCALL_CANCEL (futex, futex_word, op, expected,
> +                                    pts32, NULL /* Unused.  */,
> +                                    FUTEX_BITSET_MATCH_ANY);
> +  else
> +    return INTERNAL_SYSCALL_CALL (futex, futex_word, op, expected,
>                                    pts32, 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); -}
> -
>  static int
>  __futex_clock_wait_bitset32 (int *futexp, int val, clockid_t clockid,
>                               const struct __timespec64 *abstime, int
> private) @@ -89,11 +71,11 @@ __futex_clock_wait_bitset32 (int
> *futexp, int val, clockid_t clockid, }
>  #endif /* ! __ASSUME_TIME64_SYSCALLS */
>  
> -int
> -__futex_abstimed_wait_cancelable64 (unsigned int* futex_word,
> -                                    unsigned int expected, clockid_t
> clockid,
> -                                    const struct __timespec64*
> abstime,
> -                                    int private)
> +static int
> +__futex_abstimed_wait_common64 (unsigned int* futex_word,
> +                                unsigned int expected, clockid_t
> clockid,
> +                                const struct __timespec64* abstime,
> +                                int private, bool cancel)
>  {
>    unsigned int clockbit;
>    int err;
> @@ -109,13 +91,18 @@ __futex_abstimed_wait_cancelable64 (unsigned
> int* futex_word, clockbit = (clockid == CLOCK_REALTIME) ?
> FUTEX_CLOCK_REALTIME : 0; int op = __lll_private_flag
> (FUTEX_WAIT_BITSET | clockbit, private); 
> -  err = INTERNAL_SYSCALL_CANCEL (futex_time64, futex_word, op,
> expected,
> -                                 abstime, NULL /* Unused.  */,
> +  if (cancel)
> +    err = INTERNAL_SYSCALL_CANCEL (futex_time64, futex_word, op,
> expected,
> +                                   abstime, NULL /* Unused.  */,
> +                                   FUTEX_BITSET_MATCH_ANY);
> +  else
> +    err = INTERNAL_SYSCALL_CALL (futex_time64, futex_word, op,
> expected,
> +                                 abstime, NULL /* Ununsed.  */,
>                                   FUTEX_BITSET_MATCH_ANY);
>  #ifndef __ASSUME_TIME64_SYSCALLS
>    if (err == -ENOSYS)
> -    err = __futex_abstimed_wait_cancelable32 (futex_word, expected,
> -                                              op, abstime, private);
> +    err = __futex_abstimed_wait_common32 (futex_word, expected, op,
> abstime,
> +                                          private, cancel);
>  #endif
>  
>    switch (err)
> @@ -145,46 +132,18 @@ __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;
> +  return __futex_abstimed_wait_common64 (futex_word, expected,
> clockid,
> +                                         abstime, private, false);
> +}
>  
> -    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 ();
> -    }
> +int
> +__futex_abstimed_wait_cancelable64 (unsigned int* futex_word,
> +                                    unsigned int expected, clockid_t
> clockid,
> +                                    const struct __timespec64*
> abstime,
> +                                    int private)
> +{
> +  return __futex_abstimed_wait_common64 (futex_word, expected,
> clockid,
> +                                         abstime, private, true);
>  }
>  
>  int

Thanks for making this consolidation. Indeed there is now only
difference in issuing the syscall.

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


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 --]

Re: [PATCH 06/13] linux: nptl: Remove _futex_clock_wait_bitset64

[Lukasz Majewski]

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

Hi Adhemerval,

> It can be replaced with a __futex_abstimed_wait64 call.
> 
> Checked on x86_64-linux-gnu and i686-linux-gnu.
> ---
>  nptl/pthread_mutex_timedlock.c |  7 +++---
>  sysdeps/nptl/futex-internal.c  | 46
> ---------------------------------- sysdeps/nptl/futex-internal.h  |
> 5 ---- 3 files changed, 4 insertions(+), 54 deletions(-)
> 
> diff --git a/nptl/pthread_mutex_timedlock.c
> b/nptl/pthread_mutex_timedlock.c index de88e9fc25..0ec47359be 100644
> --- a/nptl/pthread_mutex_timedlock.c
> +++ b/nptl/pthread_mutex_timedlock.c
> @@ -265,12 +265,13 @@ __pthread_mutex_clocklock_common
> (pthread_mutex_t *mutex, assume_other_futex_waiters |= FUTEX_WAITERS;
>  
>  	  /* Block using the futex.  */
> -	  int err = __futex_clock_wait_bitset64
> (&mutex->__data.__lock,
> +	  int err = __futex_abstimed_wait64 (
> +	      (unsigned int *) &mutex->__data.__lock,
>  	      oldval, clockid, abstime,
>  	      PTHREAD_ROBUST_MUTEX_PSHARED (mutex));
>  	  /* The futex call timed out.  */
> -	  if (err == -ETIMEDOUT)
> -	    return -err;
> +	  if (err == ETIMEDOUT)
> +	    return err;
>  	  /* Reload current lock value.  */
>  	  oldval = mutex->__data.__lock;
>  	}
> diff --git a/sysdeps/nptl/futex-internal.c
> b/sysdeps/nptl/futex-internal.c index f9a2e28ee6..30c662547f 100644
> --- a/sysdeps/nptl/futex-internal.c
> +++ b/sysdeps/nptl/futex-internal.c
> @@ -48,27 +48,6 @@ __futex_abstimed_wait_common32 (unsigned int*
> futex_word, pts32, NULL /* Unused.  */,
>                                    FUTEX_BITSET_MATCH_ANY);
>  }
> -
> -static int
> -__futex_clock_wait_bitset32 (int *futexp, int val, clockid_t clockid,
> -                             const struct __timespec64 *abstime, int
> private) -{
> -  struct timespec ts32;
> -
> -  if (abstime != NULL && ! in_time_t_range (abstime->tv_sec))
> -    return -EOVERFLOW;
> -
> -  const unsigned int clockbit =
> -    (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0;
> -  const int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit,
> private); -
> -  if (abstime != NULL)
> -    ts32 = valid_timespec64_to_timespec (*abstime);
> -
> -  return INTERNAL_SYSCALL_CALL (futex, futexp, op, val,
> -                                abstime != NULL ? &ts32 : NULL,
> -                                NULL /* Unused.  */,
> FUTEX_BITSET_MATCH_ANY); -}
>  #endif /* ! __ASSUME_TIME64_SYSCALLS */
>  
>  static int
> @@ -198,28 +177,3 @@ __futex_clocklock_wait64 (int *futex, int val,
> clockid_t clockid, 
>    return -err;
>  }
> -
> -int
> -__futex_clock_wait_bitset64 (int *futexp, int val, clockid_t clockid,
> -                             const struct __timespec64 *abstime,
> -                             int private)
> -{
> -  int ret;
> -  if (! lll_futex_supported_clockid (clockid))
> -    {
> -      return -EINVAL;
> -    }
> -
> -  const unsigned int clockbit =
> -    (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0;
> -  const int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit,
> private); -
> -  ret = INTERNAL_SYSCALL_CALL (futex_time64, futexp, op, val,
> -                               abstime, NULL /* Unused.  */,
> -                               FUTEX_BITSET_MATCH_ANY);
> -#ifndef __ASSUME_TIME64_SYSCALLS
> -  if (ret == -ENOSYS)
> -    ret = __futex_clock_wait_bitset32 (futexp, val, clockid,
> abstime, private); -#endif
> -  return ret;
> -}
> diff --git a/sysdeps/nptl/futex-internal.h
> b/sysdeps/nptl/futex-internal.h index cefab74301..dcc7958d59 100644
> --- a/sysdeps/nptl/futex-internal.h
> +++ b/sysdeps/nptl/futex-internal.h
> @@ -440,9 +440,4 @@ __futex_clocklock64 (int *futex, clockid_t
> clockid, return err;
>  }
>  
> -int
> -__futex_clock_wait_bitset64 (int *futexp, int val, clockid_t clockid,
> -                             const struct __timespec64 *abstime,
> -                             int private) attribute_hidden;
> -
>  #endif  /* futex-internal.h */

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


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 --]

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

[Lukasz Majewski]

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

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

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

Re: [PATCH 08/13] linux: nptl: Replace lll_timedwait with __futex_abstimed_wait64

[Lukasz Majewski]

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

Hi Adhemerval,

> Checked with x86_64-linux-gnu and i686-linux-gnu.
> ---
>  nptl/Makefile               |  1 -
>  nptl/lll_timedlock_wait.c   | 62
> ------------------------------------- nptl/pthread_mutex_lock.c   |
> 4 +-- sysdeps/nptl/lowlevellock.h | 32 -------------------
>  4 files changed, 2 insertions(+), 97 deletions(-)
>  delete mode 100644 nptl/lll_timedlock_wait.c
> 
> diff --git a/nptl/Makefile b/nptl/Makefile
> index 74ab758c12..968768d33b 100644
> --- a/nptl/Makefile
> +++ b/nptl/Makefile
> @@ -147,7 +147,6 @@ libpthread-routines = nptl-init nptlfreeres vars
> events version pt-interp \ pt-longjmp pt-cleanup\
>  		      cancellation \
>  		      lowlevellock \
> -		      lll_timedlock_wait \
>  		      pt-fork pt-fcntl \
>  		      $(pthread-compat-wrappers) \
>  		      pt-raise pt-system \
> diff --git a/nptl/lll_timedlock_wait.c b/nptl/lll_timedlock_wait.c
> deleted file mode 100644
> index eabdca70c8..0000000000
> --- a/nptl/lll_timedlock_wait.c
> +++ /dev/null
> @@ -1,62 +0,0 @@
> -/* Timed low level locking for pthread library.  Generic futex-using
> version.
> -   Copyright (C) 2003-2020 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -   Contributed by Paul Mackerras <paulus@au.ibm.com>, 2003.
> -
> -   The GNU C Library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public
> -   License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   The GNU C Library is distributed in the hope that it will be
> useful,
> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with the GNU C Library; if not, see
> -   <https://www.gnu.org/licenses/>.  */
> -
> -#include <atomic.h>
> -#include <errno.h>
> -#include <lowlevellock.h>
> -#include <sys/time.h>
> -#include <time.h>
> -
> -
> -int
> -__lll_clocklock_wait (int *futex, int val, clockid_t clockid,
> -		      const struct timespec *abstime, int private)
> -{
> -  struct timespec 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_gettime (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;
> -    }
> -
> -  /* If *futex == val, wait until woken or timeout.  */
> -  lll_futex_timed_wait (futex, val, tsp, private);
> -
> -  return 0;
> -}
> diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c
> index 0439002454..1f137f6201 100644
> --- a/nptl/pthread_mutex_lock.c
> +++ b/nptl/pthread_mutex_lock.c
> @@ -425,8 +425,8 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
>  
>  		/* Delay the thread indefinitely.  */
>  		while (1)
> -		  lll_timedwait (&(int){0}, 0, 0 /* ignored */, NULL,
> -				 private);
> +		  __futex_abstimed_wait64 (&(unsigned int){0}, 0,
> +					   0 /* ignored */, NULL,
> private); }
>  
>  	    oldval = mutex->__data.__lock;
> diff --git a/sysdeps/nptl/lowlevellock.h b/sysdeps/nptl/lowlevellock.h
> index 68b3be8819..3de87d31a9 100644
> --- a/sysdeps/nptl/lowlevellock.h
> +++ b/sysdeps/nptl/lowlevellock.h
> @@ -122,38 +122,6 @@ extern void __lll_lock_wait (int *futex, int
> private) attribute_hidden; #define lll_cond_lock(futex, private)
> __lll_cond_lock (&(futex), private) 
>  
> -extern int __lll_clocklock_wait (int *futex, int val, clockid_t,
> -				 const struct timespec *,
> -				 int private) attribute_hidden;
> -
> -#define lll_timedwait(futex, val, clockid, abstime, private)
> 	\
> -  __lll_clocklock_wait (futex, val, clockid, abstime, private)
> -
> -/* As __lll_lock, but with an absolute timeout measured against the
> clock
> -   specified in CLOCKID.  If the timeout occurs then return
> ETIMEDOUT. If
> -   ABSTIME is invalid, return EINVAL.  */
> -#define __lll_clocklock(futex, clockid, abstime, private)       \
> -  ({                                                            \
> -    int *__futex = (futex);                                     \
> -    int __val = 0;                                              \
> -                                                                \
> -    if (__glibc_unlikely                                        \
> -        (atomic_compare_and_exchange_bool_acq (__futex, 1, 0))) \
> -      {
> 	\
> -	while (atomic_exchange_acq (futex, 2) != 0)		\
> -	  {							\
> -	    __val = __lll_clocklock_wait (__futex, 2, clockid,
> 	\
> -					  abstime, private);	\
> -	    if (__val == EINVAL || __val == ETIMEDOUT)
> 	\
> -	      break;						\
> -	  }							\
> -      }
> 	\
> -    __val;                                                      \
> -  })
> -#define lll_clocklock(futex, clockid, abstime, private)         \
> -  __lll_clocklock (&(futex), clockid, abstime, private)
> -
> -
>  /* This is an expression rather than a statement even though its
> value is void, so that it can be used in a comma expression or as an
> expression that's cast to void.  */

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


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 --]

Re: [PATCH 09/13] linux: nptl: Replace lll_futex_{timed_}wait by futex-internal.h

[Lukasz Majewski]

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

Hi Adhemerval,

> The idea is to make NPTL implementation to use on the functions
> provided by futex-internal.h.
> 
> Checked on x86_64-linux-gnu and i686-linux-gnu.
> ---
>  nptl/lowlevellock.c                 | 6 +++---
>  nptl/pthread_mutex_lock.c           | 9 +++++----
>  nptl/pthread_mutex_setprioceiling.c | 5 +++--
>  nptl/pthread_mutex_timedlock.c      | 6 +++---
>  4 files changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/nptl/lowlevellock.c b/nptl/lowlevellock.c
> index f69547a235..973df4d03a 100644
> --- a/nptl/lowlevellock.c
> +++ b/nptl/lowlevellock.c
> @@ -18,7 +18,7 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <sysdep.h>
> -#include <lowlevellock.h>
> +#include <futex-internal.h>
>  #include <atomic.h>
>  #include <stap-probe.h>
>  
> @@ -32,7 +32,7 @@ __lll_lock_wait_private (int *futex)
>      {
>      futex:
>        LIBC_PROBE (lll_lock_wait_private, 1, futex);
> -      lll_futex_wait (futex, 2, LLL_PRIVATE); /* Wait if *futex ==
> 2.  */
> +      futex_wait ((unsigned int *) futex, 2, LLL_PRIVATE); /* Wait
> if *futex == 2.  */ }
>  }
>  
> @@ -49,7 +49,7 @@ __lll_lock_wait (int *futex, int private)
>      {
>      futex:
>        LIBC_PROBE (lll_lock_wait, 1, futex);
> -      lll_futex_wait (futex, 2, private); /* Wait if *futex == 2.  */
> +      futex_wait ((unsigned int *) futex, 2, private); /* Wait if
> *futex == 2.  */ }
>  }
>  #endif
> diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c
> index 1f137f6201..965c5a3f4a 100644
> --- a/nptl/pthread_mutex_lock.c
> +++ b/nptl/pthread_mutex_lock.c
> @@ -307,8 +307,8 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
>  	  assume_other_futex_waiters |= FUTEX_WAITERS;
>  
>  	  /* Block using the futex and reload current lock value.  */
> -	  lll_futex_wait (&mutex->__data.__lock, oldval,
> -			  PTHREAD_ROBUST_MUTEX_PSHARED (mutex));
> +	  futex_wait ((unsigned int *) &mutex->__data.__lock, oldval,
> +		      PTHREAD_ROBUST_MUTEX_PSHARED (mutex));
>  	  oldval = mutex->__data.__lock;
>  	}
>  
> @@ -568,8 +568,9 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
>  		  break;
>  
>  		if (oldval != ceilval)
> -		  lll_futex_wait (&mutex->__data.__lock, ceilval | 2,
> -				  PTHREAD_MUTEX_PSHARED (mutex));
> +		  futex_wait ((unsigned int * )
> &mutex->__data.__lock,
> +			      ceilval | 2,
> +			      PTHREAD_MUTEX_PSHARED (mutex));
>  	      }
>  	    while (atomic_compare_and_exchange_val_acq
> (&mutex->__data.__lock, ceilval | 2, ceilval)
> diff --git a/nptl/pthread_mutex_setprioceiling.c
> b/nptl/pthread_mutex_setprioceiling.c index 521da72622..cbef202579
> 100644 --- a/nptl/pthread_mutex_setprioceiling.c
> +++ b/nptl/pthread_mutex_setprioceiling.c
> @@ -21,6 +21,7 @@
>  #include <errno.h>
>  #include <pthreadP.h>
>  #include <atomic.h>
> +#include <futex-internal.h>
>  
>  
>  int
> @@ -84,8 +85,8 @@ pthread_mutex_setprioceiling (pthread_mutex_t
> *mutex, int prioceiling, break;
>  
>  	    if (oldval != ceilval)
> -	      lll_futex_wait (&mutex->__data.__lock, ceilval | 2,
> -			      PTHREAD_MUTEX_PSHARED (mutex));
> +	      futex_wait ((unsigned int *) &mutex->__data.__lock,
> ceilval | 2,
> +			  PTHREAD_MUTEX_PSHARED (mutex));
>  	  }
>  	while (atomic_compare_and_exchange_val_acq
> (&mutex->__data.__lock, ceilval | 2, ceilval)
> diff --git a/nptl/pthread_mutex_timedlock.c
> b/nptl/pthread_mutex_timedlock.c index e643eab258..343acf6107 100644
> --- a/nptl/pthread_mutex_timedlock.c
> +++ b/nptl/pthread_mutex_timedlock.c
> @@ -561,9 +561,9 @@ __pthread_mutex_clocklock_common (pthread_mutex_t
> *mutex, goto failpp;
>  		      }
>  
> -		    lll_futex_timed_wait (&mutex->__data.__lock,
> -					  ceilval | 2, &rt,
> -					  PTHREAD_MUTEX_PSHARED
> (mutex));
> +		    __futex_abstimed_wait64 (
> +		      (unsigned int *) &mutex->__data.__lock,
> clockid,
> +		      ceilval | 2, &rt, PTHREAD_MUTEX_PSHARED
> (mutex)); }
>  	      }
>  	    while (atomic_compare_and_exchange_val_acq
> (&mutex->__data.__lock,

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


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 --]

Re: [PATCH 10/13] linux: nptl: Replace lll_futex_supported_clockid with futex-internal.h

[Lukasz Majewski]

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

On Mon, 23 Nov 2020 16:52:53 -0300
Adhemerval Zanella via Libc-alpha <libc-alpha@sourceware.org> wrote:

> The idea is to make NPTL implementation to use on the functions
> provided by futex-internal.h.
> 
> Checked on x86_64-linux-gnu and i686-linux-gnu.
> ---
>  nptl/pthread_mutex_timedlock.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/nptl/pthread_mutex_timedlock.c
> b/nptl/pthread_mutex_timedlock.c index 343acf6107..b42862193a 100644
> --- a/nptl/pthread_mutex_timedlock.c
> +++ b/nptl/pthread_mutex_timedlock.c
> @@ -600,7 +600,7 @@ __pthread_mutex_clocklock64 (pthread_mutex_t
> *mutex, clockid_t clockid,
>  			     const struct __timespec64 *abstime)
>  {
> -  if (__glibc_unlikely (!lll_futex_supported_clockid (clockid)))
> +  if (__glibc_unlikely (!futex_abstimed_supported_clockid (clockid)))
>      return EINVAL;
>  
>    LIBC_PROBE (mutex_clocklock_entry, 3, mutex, clockid, abstime);

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


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 --]

Re: [PATCH 11/13] nptl: Replace lll_futex_wake with futex-internal.h

[Lukasz Majewski]

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

Hi Adhemerval,

> The idea is to make NPTL implementation to use on the functions
> provided by futex-internal.h.
> 
> Checked on x86_64-linux-gnu and i686-linux-gnu.
> ---
>  nptl/pthread_mutex_setprioceiling.c | 4 ++--
>  nptl/pthread_mutex_unlock.c         | 6 +++---
>  nptl/sem_post.c                     | 9 ++-------
>  3 files changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/nptl/pthread_mutex_setprioceiling.c
> b/nptl/pthread_mutex_setprioceiling.c index cbef202579..8f1d6e1326
> 100644 --- a/nptl/pthread_mutex_setprioceiling.c
> +++ b/nptl/pthread_mutex_setprioceiling.c
> @@ -116,8 +116,8 @@ pthread_mutex_setprioceiling (pthread_mutex_t
> *mutex, int prioceiling, | (prioceiling <<
> PTHREAD_MUTEX_PRIO_CEILING_SHIFT); atomic_full_barrier ();
>  
> -  lll_futex_wake (&mutex->__data.__lock, INT_MAX,
> -		  PTHREAD_MUTEX_PSHARED (mutex));
> +  futex_wake ((unsigned int *)&mutex->__data.__lock, INT_MAX,
> +	      PTHREAD_MUTEX_PSHARED (mutex));
>  
>    return 0;
>  }
> diff --git a/nptl/pthread_mutex_unlock.c b/nptl/pthread_mutex_unlock.c
> index 2b4abb8ebe..56f1732e6d 100644
> --- a/nptl/pthread_mutex_unlock.c
> +++ b/nptl/pthread_mutex_unlock.c
> @@ -162,7 +162,7 @@ __pthread_mutex_unlock_full (pthread_mutex_t
> *mutex, int decr) private = PTHREAD_ROBUST_MUTEX_PSHARED (mutex);
>        if (__glibc_unlikely ((atomic_exchange_rel
> (&mutex->__data.__lock, 0) & FUTEX_WAITERS) != 0))
> -	lll_futex_wake (&mutex->__data.__lock, 1, private);
> +	futex_wake ((unsigned int *) &mutex->__data.__lock, 1,
> private); 
>        /* We must clear op_pending after we release the mutex.
>  	 FIXME However, this violates the mutex destruction
> requirements @@ -332,8 +332,8 @@ __pthread_mutex_unlock_full
> (pthread_mutex_t *mutex, int decr) &oldval, newval));
>  
>        if ((oldval & ~PTHREAD_MUTEX_PRIO_CEILING_MASK) > 1)
> -	lll_futex_wake (&mutex->__data.__lock, 1,
> -			PTHREAD_MUTEX_PSHARED (mutex));
> +	futex_wake ((unsigned int *)&mutex->__data.__lock, 1,
> +		    PTHREAD_MUTEX_PSHARED (mutex));
>  
>        int oldprio = newval >> PTHREAD_MUTEX_PRIO_CEILING_SHIFT;
>  
> diff --git a/nptl/sem_post.c b/nptl/sem_post.c
> index 88cfc24b30..5dbfb3a214 100644
> --- a/nptl/sem_post.c
> +++ b/nptl/sem_post.c
> @@ -84,19 +84,14 @@ int
>  attribute_compat_text_section
>  __old_sem_post (sem_t *sem)
>  {
> -  int *futex = (int *) sem;
> +  unsigned int *futex = (unsigned int *) sem;
>  
>    /* We must need to synchronize with consumers of this token, so
> the atomic increment must have release MO semantics.  */
>    atomic_write_barrier ();
>    (void) atomic_increment_val (futex);
>    /* We always have to assume it is a shared semaphore.  */
> -  int err = lll_futex_wake (futex, 1, LLL_SHARED);
> -  if (__builtin_expect (err, 0) < 0)
> -    {
> -      __set_errno (-err);
> -      return -1;
> -    }
> +  futex_wake (futex, 1, LLL_SHARED);
>    return 0;
>  }
>  compat_symbol (libpthread, __old_sem_post, sem_post, GLIBC_2_0);

Another little step to remove lll_* macros.

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


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 --]

Re: [PATCH v2 12/13] nptl: Return EINVAL for pthread_mutex_clocklock/PI with CLOCK_MONOTONIC [BZ #26801]

[Lukasz Majewski]

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

Hi Adhemerval,

> Changes from previous version:
> 
>   - Replace ENOTSUP by EINVAL
> 
> --
> 
> Linux futex FUTEX_LOCK_PI operation only supports CLOCK_REALTIME,
> so pthread_mutex_clocklock operation with priority aware mutexes
> may fail depending of the input timeout.
> 
> Also, it is not possible to convert a CLOCK_MONOTONIC to a
> CLOCK_REALTIME due the possible wall clock time change which might
> invalid the requested timeout.
> 
> Checked on x86_64-linux-gnu and i686-linux-gnu.
> ---
>  nptl/Makefile                  |  2 +-
>  nptl/pthread_mutex_timedlock.c |  7 ++++
>  nptl/tst-mutexpi10.c           | 68
> ++++++++++++++++++++++++++++++++++ sysdeps/pthread/tst-mutex5.c   |
> 2 + sysdeps/pthread/tst-mutex9.c   |  2 +
>  5 files changed, 80 insertions(+), 1 deletion(-)
>  create mode 100644 nptl/tst-mutexpi10.c
> 
> diff --git a/nptl/Makefile b/nptl/Makefile
> index 968768d33b..a48426a396 100644
> --- a/nptl/Makefile
> +++ b/nptl/Makefile
> @@ -265,7 +265,7 @@ tests = tst-attr2 tst-attr3 tst-default-attr \
>  	tst-mutex5a tst-mutex7a \
>  	tst-mutexpi1 tst-mutexpi2 tst-mutexpi3 tst-mutexpi4 \
>  	tst-mutexpi5 tst-mutexpi5a tst-mutexpi6 tst-mutexpi7
> tst-mutexpi7a \
> -	tst-mutexpi9 \
> +	tst-mutexpi9 tst-mutexpi10 \
>  	tst-cond22 tst-cond26 \
>  	tst-robustpi1 tst-robustpi2 tst-robustpi3 tst-robustpi4
> tst-robustpi5 \ tst-robustpi6 tst-robustpi7 tst-robustpi9 \
> diff --git a/nptl/pthread_mutex_timedlock.c
> b/nptl/pthread_mutex_timedlock.c index b42862193a..aaaafa21ce 100644
> --- a/nptl/pthread_mutex_timedlock.c
> +++ b/nptl/pthread_mutex_timedlock.c
> @@ -313,6 +313,13 @@ __pthread_mutex_clocklock_common
> (pthread_mutex_t *mutex, case PTHREAD_MUTEX_PI_ROBUST_NORMAL_NP:
>      case PTHREAD_MUTEX_PI_ROBUST_ADAPTIVE_NP:
>        {
> +	/* Currently futex FUTEX_LOCK_PI operation only provides
> support for
> +	   CLOCK_REALTIME and trying to emulate by converting a
> +	   CLOCK_MONOTONIC to CLOCK_REALTIME will take in account
> possible
> +	   changes to the wall clock.  */
> +	if (__glibc_unlikely (clockid != CLOCK_REALTIME))
> +	  return EINVAL;
> +
>  	int kind, robust;
>  	{
>  	  /* See concurrency notes regarding __kind in struct
> __pthread_mutex_s diff --git a/nptl/tst-mutexpi10.c
> b/nptl/tst-mutexpi10.c new file mode 100644
> index 0000000000..84ba1dfa97
> --- /dev/null
> +++ b/nptl/tst-mutexpi10.c
> @@ -0,0 +1,68 @@
> +/* Check if pthread_mutex_clocklock with PRIO_INHERIT fails with
> clock
> +   different than CLOCK_REALTIME.
> +   Copyright (C) 2015-2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be
> useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <pthread.h>
> +#include <errno.h>
> +#include <array_length.h>
> +
> +#include <support/check.h>
> +#include <support/xthread.h>
> +#include <support/timespec.h>
> +
> +static int
> +do_test (void)
> +{
> +  const int types[] = {
> +    PTHREAD_MUTEX_NORMAL,
> +    PTHREAD_MUTEX_ERRORCHECK,
> +    PTHREAD_MUTEX_RECURSIVE,
> +    PTHREAD_MUTEX_ADAPTIVE_NP
> +  };
> +  const int robust[] = {
> +    PTHREAD_MUTEX_STALLED,
> +    PTHREAD_MUTEX_ROBUST
> +  };
> +
> +
> +  for (int t = 0; t < array_length (types); t++)
> +    for (int r = 0; r < array_length (robust); r++)
> +      {
> +	pthread_mutexattr_t attr;
> +
> +	xpthread_mutexattr_init (&attr);
> +	xpthread_mutexattr_setprotocol (&attr, PTHREAD_PRIO_INHERIT);
> +	xpthread_mutexattr_settype (&attr, types[t]);
> +	xpthread_mutexattr_setrobust (&attr, robust[r]);
> +
> +	pthread_mutex_t mtx;
> +	xpthread_mutex_init (&mtx, &attr);
> +
> +	struct timespec tmo = timespec_add (xclock_now
> (CLOCK_MONOTONIC),
> +					    make_timespec (0,
> 100000000)); +
> +	TEST_COMPARE (pthread_mutex_clocklock (&mtx,
> CLOCK_MONOTONIC, &tmo),
> +		      EINVAL);
> +
> +	xpthread_mutex_destroy (&mtx);
> +      }
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/sysdeps/pthread/tst-mutex5.c
> b/sysdeps/pthread/tst-mutex5.c index 14490768c3..bfe1a79fa4 100644
> --- a/sysdeps/pthread/tst-mutex5.c
> +++ b/sysdeps/pthread/tst-mutex5.c
> @@ -112,7 +112,9 @@ static int do_test (void)
>  {
>    do_test_clock (CLOCK_USE_TIMEDLOCK, "timedlock");
>    do_test_clock (CLOCK_REALTIME, "clocklock(realtime)");
> +#ifndef ENABLE_PI
>    do_test_clock (CLOCK_MONOTONIC, "clocklock(monotonic)");
> +#endif
>    return 0;
>  }
>  
> diff --git a/sysdeps/pthread/tst-mutex9.c
> b/sysdeps/pthread/tst-mutex9.c index 2d7927b7c2..bfc01f8c75 100644
> --- a/sysdeps/pthread/tst-mutex9.c
> +++ b/sysdeps/pthread/tst-mutex9.c
> @@ -133,7 +133,9 @@ do_test (void)
>  {
>    do_test_clock (CLOCK_USE_TIMEDLOCK);
>    do_test_clock (CLOCK_REALTIME);
> +#ifndef ENABLE_PI
>    do_test_clock (CLOCK_MONOTONIC);
> +#endif
>    return 0;
>  }
>  

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


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 --]

Re: [PATCH 13/13] linux: Return EINVAL for invalid clock for pthread_clockjoin_np

[Lukasz Majewski]

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

Hi Adhemerval,

> The align the GNU extension with the others one that accept specify
> which clock to wait for (such as pthread_mutex_clocklock).
> 
> Check on x86_64-linux-gnu.
> ---
>  nptl/pthread_clockjoin.c     |  4 ++
>  sysdeps/pthread/Makefile     |  2 +-
>  sysdeps/pthread/tst-join15.c | 80
> ++++++++++++++++++++++++++++++++++++ 3 files changed, 85
> insertions(+), 1 deletion(-) create mode 100644
> sysdeps/pthread/tst-join15.c
> 
> diff --git a/nptl/pthread_clockjoin.c b/nptl/pthread_clockjoin.c
> index 0baba1e83d..3d54fe588f 100644
> --- a/nptl/pthread_clockjoin.c
> +++ b/nptl/pthread_clockjoin.c
> @@ -17,12 +17,16 @@
>     <http://www.gnu.org/licenses/>.  */
>  
>  #include <time.h>
> +#include <futex-internal.h>
>  #include "pthreadP.h"
>  
>  int
>  __pthread_clockjoin_np64 (pthread_t threadid, void **thread_return,
>                            clockid_t clockid, const struct
> __timespec64 *abstime) {
> +  if (!futex_abstimed_supported_clockid (clockid))
> +    return EINVAL;
> +
>    return __pthread_clockjoin_ex (threadid, thread_return,
>                                   clockid, abstime, true);
>  }
> diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile
> index 45a15b0b1a..8f335c13b5 100644
> --- a/sysdeps/pthread/Makefile
> +++ b/sysdeps/pthread/Makefile
> @@ -77,7 +77,7 @@ tests += tst-cnd-basic tst-mtx-trylock
> tst-cnd-broadcast \ tst-getpid3 \
>  	 tst-join1 tst-join2 tst-join3 tst-join4 tst-join5 tst-join6
> tst-join7 \ tst-join8 tst-join9 tst-join10 tst-join11 tst-join12
> tst-join13 \
> -	 tst-join14 \
> +	 tst-join14 tst-join15 \
>  	 tst-key1 tst-key2 tst-key3 tst-key4 \
>  	 tst-kill1 tst-kill2 tst-kill3 tst-kill4 tst-kill5 tst-kill6
> \ tst-locale1 tst-locale2 \
> diff --git a/sysdeps/pthread/tst-join15.c
> b/sysdeps/pthread/tst-join15.c new file mode 100644
> index 0000000000..4ed767e733
> --- /dev/null
> +++ b/sysdeps/pthread/tst-join15.c
> @@ -0,0 +1,80 @@
> +/* Check pthread_clockjoin_np clock support.
> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be
> useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <pthread.h>
> +#include <sys/time.h>
> +#include <unistd.h>
> +#include <errno.h>
> +
> +#include <array_length.h>
> +#include <support/check.h>
> +#include <support/timespec.h>
> +#include <support/xthread.h>
> +
> +static void *
> +tf (void *arg)
> +{
> +  pause ();
> +  return NULL;
> +}
> +
> +
> +static int
> +do_test (void)
> +{
> +  const clockid_t clocks[] = {
> +    CLOCK_REALTIME,
> +    CLOCK_MONOTONIC,
> +    CLOCK_PROCESS_CPUTIME_ID,
> +    CLOCK_THREAD_CPUTIME_ID,
> +    CLOCK_THREAD_CPUTIME_ID,
> +    CLOCK_MONOTONIC_RAW,
> +    CLOCK_REALTIME_COARSE,
> +    CLOCK_MONOTONIC_COARSE,
> +#ifdef CLOCK_BOOTTIME
> +    CLOCK_BOOTTIME,
> +#endif
> +#ifdef CLOCK_REALTIME_ALARM
> +    CLOCK_REALTIME_ALARM,
> +#endif
> +#ifdef CLOCK_BOOTTIME_ALARM
> +    CLOCK_BOOTTIME_ALARM,
> +#endif
> +#ifdef CLOCK_TAI
> +    CLOCK_TAI
> +#endif
> +  };
> +
> +  pthread_t thr = xpthread_create (NULL, tf, NULL);
> +
> +  for (int t = 0; t < array_length (clocks); t++)
> +    {
> +      /* A valid timeout so valid clock timeout.  */
> +      struct timespec tmo = timespec_add (xclock_now (clocks[t]),
> +					  make_timespec (0,
> 100000000)); +
> +      int ret = clocks[t] == CLOCK_REALTIME || clocks[t] ==
> CLOCK_MONOTONIC
> +		? ETIMEDOUT : EINVAL;
> +
> +      TEST_COMPARE (pthread_clockjoin_np (thr, NULL, clocks[t],
> &tmo), ret);
> +    }
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>

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


When can we expect that those patches will be pulled? It seems like we
mostly remove dead (i.e. non Y2038 supporting) code in this series.

As aio_suspend depends on this patch series I would be great if we
would pull those sooner than latter :-).

Adhemerval, thanks again for refactoring the futex code.

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 --]

Re: [PATCH v2 12/13] nptl: Return EINVAL for pthread_mutex_clocklock/PI with CLOCK_MONOTONIC [BZ #26801]

[Lukasz Majewski]

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

On Mon, 23 Nov 2020 16:52:55 -0300
Adhemerval Zanella via Libc-alpha <libc-alpha@sourceware.org> wrote:

> Changes from previous version:
> 
>   - Replace ENOTSUP by EINVAL
> 
> --
> 
> Linux futex FUTEX_LOCK_PI operation only supports CLOCK_REALTIME,
> so pthread_mutex_clocklock operation with priority aware mutexes
> may fail depending of the input timeout.
> 
> Also, it is not possible to convert a CLOCK_MONOTONIC to a
> CLOCK_REALTIME due the possible wall clock time change which might
> invalid the requested timeout.
> 
> Checked on x86_64-linux-gnu and i686-linux-gnu.
> ---
>  nptl/Makefile                  |  2 +-
>  nptl/pthread_mutex_timedlock.c |  7 ++++
>  nptl/tst-mutexpi10.c           | 68
> ++++++++++++++++++++++++++++++++++ sysdeps/pthread/tst-mutex5.c   |
> 2 + sysdeps/pthread/tst-mutex9.c   |  2 +
>  5 files changed, 80 insertions(+), 1 deletion(-)
>  create mode 100644 nptl/tst-mutexpi10.c
> 
> diff --git a/nptl/Makefile b/nptl/Makefile
> index 968768d33b..a48426a396 100644
> --- a/nptl/Makefile
> +++ b/nptl/Makefile
> @@ -265,7 +265,7 @@ tests = tst-attr2 tst-attr3 tst-default-attr \
>  	tst-mutex5a tst-mutex7a \
>  	tst-mutexpi1 tst-mutexpi2 tst-mutexpi3 tst-mutexpi4 \
>  	tst-mutexpi5 tst-mutexpi5a tst-mutexpi6 tst-mutexpi7
> tst-mutexpi7a \
> -	tst-mutexpi9 \
> +	tst-mutexpi9 tst-mutexpi10 \
>  	tst-cond22 tst-cond26 \
>  	tst-robustpi1 tst-robustpi2 tst-robustpi3 tst-robustpi4
> tst-robustpi5 \ tst-robustpi6 tst-robustpi7 tst-robustpi9 \
> diff --git a/nptl/pthread_mutex_timedlock.c
> b/nptl/pthread_mutex_timedlock.c index b42862193a..aaaafa21ce 100644
> --- a/nptl/pthread_mutex_timedlock.c
> +++ b/nptl/pthread_mutex_timedlock.c
> @@ -313,6 +313,13 @@ __pthread_mutex_clocklock_common
> (pthread_mutex_t *mutex, case PTHREAD_MUTEX_PI_ROBUST_NORMAL_NP:
>      case PTHREAD_MUTEX_PI_ROBUST_ADAPTIVE_NP:
>        {
> +	/* Currently futex FUTEX_LOCK_PI operation only provides
> support for
> +	   CLOCK_REALTIME and trying to emulate by converting a
> +	   CLOCK_MONOTONIC to CLOCK_REALTIME will take in account
> possible
> +	   changes to the wall clock.  */
> +	if (__glibc_unlikely (clockid != CLOCK_REALTIME))
> +	  return EINVAL;
> +
>  	int kind, robust;
>  	{
>  	  /* See concurrency notes regarding __kind in struct
> __pthread_mutex_s diff --git a/nptl/tst-mutexpi10.c
> b/nptl/tst-mutexpi10.c new file mode 100644
> index 0000000000..84ba1dfa97
> --- /dev/null
> +++ b/nptl/tst-mutexpi10.c
> @@ -0,0 +1,68 @@
> +/* Check if pthread_mutex_clocklock with PRIO_INHERIT fails with
> clock
> +   different than CLOCK_REALTIME.
> +   Copyright (C) 2015-2020 Free Software Foundation, Inc.

Minor -> 2020 only

> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be
> useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <pthread.h>
> +#include <errno.h>
> +#include <array_length.h>
> +
> +#include <support/check.h>
> +#include <support/xthread.h>
> +#include <support/timespec.h>
> +
> +static int
> +do_test (void)
> +{
> +  const int types[] = {
> +    PTHREAD_MUTEX_NORMAL,
> +    PTHREAD_MUTEX_ERRORCHECK,
> +    PTHREAD_MUTEX_RECURSIVE,
> +    PTHREAD_MUTEX_ADAPTIVE_NP
> +  };
> +  const int robust[] = {
> +    PTHREAD_MUTEX_STALLED,
> +    PTHREAD_MUTEX_ROBUST
> +  };
> +
> +
> +  for (int t = 0; t < array_length (types); t++)
> +    for (int r = 0; r < array_length (robust); r++)
> +      {
> +	pthread_mutexattr_t attr;
> +
> +	xpthread_mutexattr_init (&attr);
> +	xpthread_mutexattr_setprotocol (&attr, PTHREAD_PRIO_INHERIT);
> +	xpthread_mutexattr_settype (&attr, types[t]);
> +	xpthread_mutexattr_setrobust (&attr, robust[r]);
> +
> +	pthread_mutex_t mtx;
> +	xpthread_mutex_init (&mtx, &attr);
> +
> +	struct timespec tmo = timespec_add (xclock_now
> (CLOCK_MONOTONIC),
> +					    make_timespec (0,
> 100000000)); +
> +	TEST_COMPARE (pthread_mutex_clocklock (&mtx,
> CLOCK_MONOTONIC, &tmo),
> +		      EINVAL);
> +
> +	xpthread_mutex_destroy (&mtx);
> +      }
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/sysdeps/pthread/tst-mutex5.c
> b/sysdeps/pthread/tst-mutex5.c index 14490768c3..bfe1a79fa4 100644
> --- a/sysdeps/pthread/tst-mutex5.c
> +++ b/sysdeps/pthread/tst-mutex5.c
> @@ -112,7 +112,9 @@ static int do_test (void)
>  {
>    do_test_clock (CLOCK_USE_TIMEDLOCK, "timedlock");
>    do_test_clock (CLOCK_REALTIME, "clocklock(realtime)");
> +#ifndef ENABLE_PI
>    do_test_clock (CLOCK_MONOTONIC, "clocklock(monotonic)");
> +#endif
>    return 0;
>  }
>  
> diff --git a/sysdeps/pthread/tst-mutex9.c
> b/sysdeps/pthread/tst-mutex9.c index 2d7927b7c2..bfc01f8c75 100644
> --- a/sysdeps/pthread/tst-mutex9.c
> +++ b/sysdeps/pthread/tst-mutex9.c
> @@ -133,7 +133,9 @@ do_test (void)
>  {
>    do_test_clock (CLOCK_USE_TIMEDLOCK);
>    do_test_clock (CLOCK_REALTIME);
> +#ifndef ENABLE_PI
>    do_test_clock (CLOCK_MONOTONIC);
> +#endif
>    return 0;
>  }
>  

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


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 --]

Re: [PATCH 13/13] linux: Return EINVAL for invalid clock for pthread_clockjoin_np

[Lukasz Majewski]

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

On Tue, 24 Nov 2020 22:48:59 +0100
Lukasz Majewski <lukma@denx.de> wrote:

> Hi Adhemerval,
> 
> > The align the GNU extension with the others one that accept specify
> > which clock to wait for (such as pthread_mutex_clocklock).
> > 
> > Check on x86_64-linux-gnu.
> > ---
> >  nptl/pthread_clockjoin.c     |  4 ++
> >  sysdeps/pthread/Makefile     |  2 +-
> >  sysdeps/pthread/tst-join15.c | 80
> > ++++++++++++++++++++++++++++++++++++ 3 files changed, 85
> > insertions(+), 1 deletion(-) create mode 100644
> > sysdeps/pthread/tst-join15.c
> > 
> > diff --git a/nptl/pthread_clockjoin.c b/nptl/pthread_clockjoin.c
> > index 0baba1e83d..3d54fe588f 100644
> > --- a/nptl/pthread_clockjoin.c
> > +++ b/nptl/pthread_clockjoin.c
> > @@ -17,12 +17,16 @@
> >     <http://www.gnu.org/licenses/>.  */
> >  
> >  #include <time.h>
> > +#include <futex-internal.h>
> >  #include "pthreadP.h"
> >  
> >  int
> >  __pthread_clockjoin_np64 (pthread_t threadid, void **thread_return,
> >                            clockid_t clockid, const struct
> > __timespec64 *abstime) {
> > +  if (!futex_abstimed_supported_clockid (clockid))
> > +    return EINVAL;
> > +
> >    return __pthread_clockjoin_ex (threadid, thread_return,
> >                                   clockid, abstime, true);
> >  }
> > diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile
> > index 45a15b0b1a..8f335c13b5 100644
> > --- a/sysdeps/pthread/Makefile
> > +++ b/sysdeps/pthread/Makefile
> > @@ -77,7 +77,7 @@ tests += tst-cnd-basic tst-mtx-trylock
> > tst-cnd-broadcast \ tst-getpid3 \
> >  	 tst-join1 tst-join2 tst-join3 tst-join4 tst-join5
> > tst-join6 tst-join7 \ tst-join8 tst-join9 tst-join10 tst-join11
> > tst-join12 tst-join13 \
> > -	 tst-join14 \
> > +	 tst-join14 tst-join15 \
> >  	 tst-key1 tst-key2 tst-key3 tst-key4 \
> >  	 tst-kill1 tst-kill2 tst-kill3 tst-kill4 tst-kill5
> > tst-kill6 \ tst-locale1 tst-locale2 \
> > diff --git a/sysdeps/pthread/tst-join15.c
> > b/sysdeps/pthread/tst-join15.c new file mode 100644
> > index 0000000000..4ed767e733
> > --- /dev/null
> > +++ b/sysdeps/pthread/tst-join15.c
> > @@ -0,0 +1,80 @@
> > +/* Check pthread_clockjoin_np clock support.
> > +   Copyright (C) 2020 Free Software Foundation, Inc.
> > +   This file is part of the GNU C Library.
> > +
> > +   The GNU C Library is free software; you can redistribute it
> > and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later
> > version. +
> > +   The GNU C Library is distributed in the hope that it will be
> > useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with the GNU C Library; if not, see
> > +   <https://www.gnu.org/licenses/>.  */
> > +
> > +#include <pthread.h>
> > +#include <sys/time.h>
> > +#include <unistd.h>
> > +#include <errno.h>
> > +
> > +#include <array_length.h>
> > +#include <support/check.h>
> > +#include <support/timespec.h>
> > +#include <support/xthread.h>
> > +
> > +static void *
> > +tf (void *arg)
> > +{
> > +  pause ();
> > +  return NULL;
> > +}
> > +
> > +
> > +static int
> > +do_test (void)
> > +{
> > +  const clockid_t clocks[] = {
> > +    CLOCK_REALTIME,
> > +    CLOCK_MONOTONIC,
> > +    CLOCK_PROCESS_CPUTIME_ID,
> > +    CLOCK_THREAD_CPUTIME_ID,
> > +    CLOCK_THREAD_CPUTIME_ID,
> > +    CLOCK_MONOTONIC_RAW,
> > +    CLOCK_REALTIME_COARSE,
> > +    CLOCK_MONOTONIC_COARSE,
> > +#ifdef CLOCK_BOOTTIME
> > +    CLOCK_BOOTTIME,
> > +#endif
> > +#ifdef CLOCK_REALTIME_ALARM
> > +    CLOCK_REALTIME_ALARM,
> > +#endif
> > +#ifdef CLOCK_BOOTTIME_ALARM
> > +    CLOCK_BOOTTIME_ALARM,
> > +#endif
> > +#ifdef CLOCK_TAI
> > +    CLOCK_TAI
> > +#endif
> > +  };
> > +
> > +  pthread_t thr = xpthread_create (NULL, tf, NULL);
> > +
> > +  for (int t = 0; t < array_length (clocks); t++)
> > +    {
> > +      /* A valid timeout so valid clock timeout.  */
> > +      struct timespec tmo = timespec_add (xclock_now (clocks[t]),
> > +					  make_timespec (0,
> > 100000000)); +
> > +      int ret = clocks[t] == CLOCK_REALTIME || clocks[t] ==
> > CLOCK_MONOTONIC
> > +		? ETIMEDOUT : EINVAL;
> > +
> > +      TEST_COMPARE (pthread_clockjoin_np (thr, NULL, clocks[t],
> > &tmo), ret);
> > +    }
> > +
> > +  return 0;
> > +}
> > +
> > +#include <support/test-driver.c>  
> 
> Reviewed-by: Lukasz Majewski <lukma@denx.de>
> 
> 
> When can we expect that those patches will be pulled? It seems like we
> mostly remove dead (i.e. non Y2038 supporting) code in this series.
> 
> As aio_suspend depends on this patch series I would be great if we
> would pull those sooner than latter :-).

I've built tested following branch:
https://github.com/lmajewski/y2038_glibc/commits/y2038_edge-futex-rework

on x86-64-x32, x86, x86-64, arm 32 bits and pthread_* related tests
were OK, so

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


> 
> Adhemerval, thanks again for refactoring the futex code.
> 
> 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




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 --]

Re: [PATCH 09/13] linux: nptl: Replace lll_futex_{timed_}wait by futex-internal.h

[Mike Crowe via Libc-alpha]

On Monday 23 November 2020 at 16:52:52 -0300, Adhemerval Zanella wrote:
> The idea is to make NPTL implementation to use on the functions
> provided by futex-internal.h.
> 
> Checked on x86_64-linux-gnu and i686-linux-gnu.
> ---
>  nptl/lowlevellock.c                 | 6 +++---
>  nptl/pthread_mutex_lock.c           | 9 +++++----
>  nptl/pthread_mutex_setprioceiling.c | 5 +++--
>  nptl/pthread_mutex_timedlock.c      | 6 +++---
>  4 files changed, 14 insertions(+), 12 deletions(-)

[snip]

> diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
> index e643eab258..343acf6107 100644
> --- a/nptl/pthread_mutex_timedlock.c
> +++ b/nptl/pthread_mutex_timedlock.c
> @@ -561,9 +561,9 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
>  			goto failpp;
>  		      }
>  
> -		    lll_futex_timed_wait (&mutex->__data.__lock,
> -					  ceilval | 2, &rt,
> -					  PTHREAD_MUTEX_PSHARED (mutex));
> +		    __futex_abstimed_wait64 (
> +		      (unsigned int *) &mutex->__data.__lock, clockid,
> +		      ceilval | 2, &rt, PTHREAD_MUTEX_PSHARED (mutex));

I think you've replaced the lll_futex_timed_wait call that expects a
relative timeout with a __futex_abstimed_wait64 call that expects an
absolute timeout, yet you still appear to be passing the relative timeout.

However, it turns out that the implementation for the
PTHREAD_MUTEX_PP_RECURSIVE_NP and friends case appears to be have been
completely broken with clockid != CLOCK_REALTIME ever since I added it in
9d20e22e46d891b929a72b0f35586e079eb083fd anyway since the relative timeout
is calculated by calling __clock_gettime64(CLOCK_REALTIME) (although at the
time this was a less obvious __gettimeofday call.)

I'll work on writing some test cases for the those types of mutex in the
hope of catching both flaws before fixing them.

Mike.

Re: [PATCH 09/13] linux: nptl: Replace lll_futex_{timed_}wait by futex-internal.h

[Adhemerval Zanella via Libc-alpha]

On 25/11/2020 12:32, Mike Crowe wrote:
> On Monday 23 November 2020 at 16:52:52 -0300, Adhemerval Zanella wrote:
>> The idea is to make NPTL implementation to use on the functions
>> provided by futex-internal.h.
>>
>> Checked on x86_64-linux-gnu and i686-linux-gnu.
>> ---
>>  nptl/lowlevellock.c                 | 6 +++---
>>  nptl/pthread_mutex_lock.c           | 9 +++++----
>>  nptl/pthread_mutex_setprioceiling.c | 5 +++--
>>  nptl/pthread_mutex_timedlock.c      | 6 +++---
>>  4 files changed, 14 insertions(+), 12 deletions(-)
> 
> [snip]
> 
>> diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
>> index e643eab258..343acf6107 100644
>> --- a/nptl/pthread_mutex_timedlock.c
>> +++ b/nptl/pthread_mutex_timedlock.c
>> @@ -561,9 +561,9 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
>>  			goto failpp;
>>  		      }
>>  
>> -		    lll_futex_timed_wait (&mutex->__data.__lock,
>> -					  ceilval | 2, &rt,
>> -					  PTHREAD_MUTEX_PSHARED (mutex));
>> +		    __futex_abstimed_wait64 (
>> +		      (unsigned int *) &mutex->__data.__lock, clockid,
>> +		      ceilval | 2, &rt, PTHREAD_MUTEX_PSHARED (mutex));
> 
> I think you've replaced the lll_futex_timed_wait call that expects a
> relative timeout with a __futex_abstimed_wait64 call that expects an
> absolute timeout, yet you still appear to be passing the relative timeout.
> 
> However, it turns out that the implementation for the
> PTHREAD_MUTEX_PP_RECURSIVE_NP and friends case appears to be have been
> completely broken with clockid != CLOCK_REALTIME ever since I added it in
> 9d20e22e46d891b929a72b0f35586e079eb083fd anyway since the relative timeout
> is calculated by calling __clock_gettime64(CLOCK_REALTIME) (although at the
> time this was a less obvious __gettimeofday call.)
> 
> I'll work on writing some test cases for the those types of mutex in the
> hope of catching both flaws before fixing them.

Indeed, there is no need to calculate the relative timeout anymore. I think
the fix below should pass the absolute timeout directly.   I will check
a possible regression tests as well.


diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
index aaaafa21ce..86c5f4446e 100644
--- a/nptl/pthread_mutex_timedlock.c
+++ b/nptl/pthread_mutex_timedlock.c
@@ -508,7 +508,6 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
 	    if (__pthread_current_priority () > ceiling)
 	      {
 		result = EINVAL;
-	      failpp:
 		if (oldprio != -1)
 		  __pthread_tpp_change_priority (oldprio, -1);
 		return result;
@@ -540,37 +539,9 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
 
 		if (oldval != ceilval)
 		  {
-		    /* Reject invalid timeouts.  */
-		    if (! valid_nanoseconds (abstime->tv_nsec))
-		      {
-			result = EINVAL;
-			goto failpp;
-		      }
-
-		    struct __timespec64 rt;
-
-		    /* Get the current time.  */
-		    __clock_gettime64 (CLOCK_REALTIME, &rt);
-
-		    /* Compute relative timeout.  */
-		    rt.tv_sec = abstime->tv_sec - rt.tv_sec;
-		    rt.tv_nsec = abstime->tv_nsec - rt.tv_nsec;
-		    if (rt.tv_nsec < 0)
-		      {
-			rt.tv_nsec += 1000000000;
-			--rt.tv_sec;
-		      }
-
-		    /* Already timed out?  */
-		    if (rt.tv_sec < 0)
-		      {
-			result = ETIMEDOUT;
-			goto failpp;
-		      }
-
 		    __futex_abstimed_wait64 (
 		      (unsigned int *) &mutex->__data.__lock, clockid,
-		      ceilval | 2, &rt, PTHREAD_MUTEX_PSHARED (mutex));
+		      ceilval | 2, abstime, PTHREAD_MUTEX_PSHARED (mutex));
 		  }
 	      }
 	    while (atomic_compare_and_exchange_val_acq (&mutex->__data.__lock,

Re: [PATCH 09/13] linux: nptl: Replace lll_futex_{timed_}wait by futex-internal.h

[Mike Crowe via Libc-alpha]

On Wednesday 25 November 2020 at 12:40:46 -0300, Adhemerval Zanella wrote:
> 
> 
> On 25/11/2020 12:32, Mike Crowe wrote:
> > On Monday 23 November 2020 at 16:52:52 -0300, Adhemerval Zanella wrote:
> >> The idea is to make NPTL implementation to use on the functions
> >> provided by futex-internal.h.
> >>
> >> Checked on x86_64-linux-gnu and i686-linux-gnu.
> >> ---
> >>  nptl/lowlevellock.c                 | 6 +++---
> >>  nptl/pthread_mutex_lock.c           | 9 +++++----
> >>  nptl/pthread_mutex_setprioceiling.c | 5 +++--
> >>  nptl/pthread_mutex_timedlock.c      | 6 +++---
> >>  4 files changed, 14 insertions(+), 12 deletions(-)
> > 
> > [snip]
> > 
> >> diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
> >> index e643eab258..343acf6107 100644
> >> --- a/nptl/pthread_mutex_timedlock.c
> >> +++ b/nptl/pthread_mutex_timedlock.c
> >> @@ -561,9 +561,9 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
> >>  			goto failpp;
> >>  		      }
> >>  
> >> -		    lll_futex_timed_wait (&mutex->__data.__lock,
> >> -					  ceilval | 2, &rt,
> >> -					  PTHREAD_MUTEX_PSHARED (mutex));
> >> +		    __futex_abstimed_wait64 (
> >> +		      (unsigned int *) &mutex->__data.__lock, clockid,
> >> +		      ceilval | 2, &rt, PTHREAD_MUTEX_PSHARED (mutex));
> > 
> > I think you've replaced the lll_futex_timed_wait call that expects a
> > relative timeout with a __futex_abstimed_wait64 call that expects an
> > absolute timeout, yet you still appear to be passing the relative timeout.
> > 
> > However, it turns out that the implementation for the
> > PTHREAD_MUTEX_PP_RECURSIVE_NP and friends case appears to be have been
> > completely broken with clockid != CLOCK_REALTIME ever since I added it in
> > 9d20e22e46d891b929a72b0f35586e079eb083fd anyway since the relative timeout
> > is calculated by calling __clock_gettime64(CLOCK_REALTIME) (although at the
> > time this was a less obvious __gettimeofday call.)
> > 
> > I'll work on writing some test cases for the those types of mutex in the
> > hope of catching both flaws before fixing them.
> 
> Indeed, there is no need to calculate the relative timeout anymore. I think
> the fix below should pass the absolute timeout directly.   I will check
> a possible regression tests as well.

OK. I won't then. Thanks.

> diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
> index aaaafa21ce..86c5f4446e 100644
> --- a/nptl/pthread_mutex_timedlock.c
> +++ b/nptl/pthread_mutex_timedlock.c
> @@ -508,7 +508,6 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
>  	    if (__pthread_current_priority () > ceiling)
>  	      {
>  		result = EINVAL;
> -	      failpp:
>  		if (oldprio != -1)
>  		  __pthread_tpp_change_priority (oldprio, -1);
>  		return result;
> @@ -540,37 +539,9 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
>  
>  		if (oldval != ceilval)
>  		  {
> -		    /* Reject invalid timeouts.  */
> -		    if (! valid_nanoseconds (abstime->tv_nsec))
> -		      {
> -			result = EINVAL;
> -			goto failpp;
> -		      }

If this is removed then is there a risk of getting into a busy loop if
someone passes a bogus timespec? (Regardless of the answer, it makes sense
to ensure that is tested somehow.)

> -
> -		    struct __timespec64 rt;
> -
> -		    /* Get the current time.  */
> -		    __clock_gettime64 (CLOCK_REALTIME, &rt);
> -
> -		    /* Compute relative timeout.  */
> -		    rt.tv_sec = abstime->tv_sec - rt.tv_sec;
> -		    rt.tv_nsec = abstime->tv_nsec - rt.tv_nsec;
> -		    if (rt.tv_nsec < 0)
> -		      {
> -			rt.tv_nsec += 1000000000;
> -			--rt.tv_sec;
> -		      }
> -
> -		    /* Already timed out?  */
> -		    if (rt.tv_sec < 0)
> -		      {
> -			result = ETIMEDOUT;
> -			goto failpp;
> -		      }
> -
>  		    __futex_abstimed_wait64 (
>  		      (unsigned int *) &mutex->__data.__lock, clockid,
> -		      ceilval | 2, &rt, PTHREAD_MUTEX_PSHARED (mutex));
> +		      ceilval | 2, abstime, PTHREAD_MUTEX_PSHARED (mutex));
>  		  }
>  	      }
>  	    while (atomic_compare_and_exchange_val_acq (&mutex->__data.__lock,

LGTM.

Thanks.

Mike.

Re: [PATCH 09/13] linux: nptl: Replace lll_futex_{timed_}wait by futex-internal.h

[Adhemerval Zanella via Libc-alpha]

On 25/11/2020 12:46, Mike Crowe wrote:
> On Wednesday 25 November 2020 at 12:40:46 -0300, Adhemerval Zanella wrote:
>>
>>
>> On 25/11/2020 12:32, Mike Crowe wrote:
>>> On Monday 23 November 2020 at 16:52:52 -0300, Adhemerval Zanella wrote:
>>>> The idea is to make NPTL implementation to use on the functions
>>>> provided by futex-internal.h.
>>>>
>>>> Checked on x86_64-linux-gnu and i686-linux-gnu.
>>>> ---
>>>>  nptl/lowlevellock.c                 | 6 +++---
>>>>  nptl/pthread_mutex_lock.c           | 9 +++++----
>>>>  nptl/pthread_mutex_setprioceiling.c | 5 +++--
>>>>  nptl/pthread_mutex_timedlock.c      | 6 +++---
>>>>  4 files changed, 14 insertions(+), 12 deletions(-)
>>>
>>> [snip]
>>>
>>>> diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
>>>> index e643eab258..343acf6107 100644
>>>> --- a/nptl/pthread_mutex_timedlock.c
>>>> +++ b/nptl/pthread_mutex_timedlock.c
>>>> @@ -561,9 +561,9 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
>>>>  			goto failpp;
>>>>  		      }
>>>>  
>>>> -		    lll_futex_timed_wait (&mutex->__data.__lock,
>>>> -					  ceilval | 2, &rt,
>>>> -					  PTHREAD_MUTEX_PSHARED (mutex));
>>>> +		    __futex_abstimed_wait64 (
>>>> +		      (unsigned int *) &mutex->__data.__lock, clockid,
>>>> +		      ceilval | 2, &rt, PTHREAD_MUTEX_PSHARED (mutex));
>>>
>>> I think you've replaced the lll_futex_timed_wait call that expects a
>>> relative timeout with a __futex_abstimed_wait64 call that expects an
>>> absolute timeout, yet you still appear to be passing the relative timeout.
>>>
>>> However, it turns out that the implementation for the
>>> PTHREAD_MUTEX_PP_RECURSIVE_NP and friends case appears to be have been
>>> completely broken with clockid != CLOCK_REALTIME ever since I added it in
>>> 9d20e22e46d891b929a72b0f35586e079eb083fd anyway since the relative timeout
>>> is calculated by calling __clock_gettime64(CLOCK_REALTIME) (although at the
>>> time this was a less obvious __gettimeofday call.)
>>>
>>> I'll work on writing some test cases for the those types of mutex in the
>>> hope of catching both flaws before fixing them.
>>
>> Indeed, there is no need to calculate the relative timeout anymore. I think
>> the fix below should pass the absolute timeout directly.   I will check
>> a possible regression tests as well.
> 
> OK. I won't then. Thanks.
> 
>> diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
>> index aaaafa21ce..86c5f4446e 100644
>> --- a/nptl/pthread_mutex_timedlock.c
>> +++ b/nptl/pthread_mutex_timedlock.c
>> @@ -508,7 +508,6 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
>>  	    if (__pthread_current_priority () > ceiling)
>>  	      {
>>  		result = EINVAL;
>> -	      failpp:
>>  		if (oldprio != -1)
>>  		  __pthread_tpp_change_priority (oldprio, -1);
>>  		return result;
>> @@ -540,37 +539,9 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
>>  
>>  		if (oldval != ceilval)
>>  		  {
>> -		    /* Reject invalid timeouts.  */
>> -		    if (! valid_nanoseconds (abstime->tv_nsec))
>> -		      {
>> -			result = EINVAL;
>> -			goto failpp;
>> -		      }
> 
> If this is removed then is there a risk of getting into a busy loop if
> someone passes a bogus timespec? (Regardless of the answer, it makes sense
> to ensure that is tested somehow.)

The minimum supported kernel already does the same check on the futex call
(source for Linux 3.2):

2690 SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val,
2691                 struct timespec __user *, utime, u32 __user *, uaddr2,
2692                 u32, val3)
2693 {
2694         struct timespec ts;
2695         ktime_t t, *tp = NULL;
2696         u32 val2 = 0;
2697         int cmd = op & FUTEX_CMD_MASK;
2698 
2699         if (utime && (cmd == FUTEX_WAIT || cmd == FUTEX_LOCK_PI ||
2700                       cmd == FUTEX_WAIT_BITSET ||
2701                       cmd == FUTEX_WAIT_REQUEUE_PI)) {
2702                 if (copy_from_user(&ts, utime, sizeof(ts)) != 0)
2703                         return -EFAULT;
2704                 if (!timespec_valid(&ts))
2705                         return -EINVAL;
2706 
2707                 t = timespec_to_ktime(ts);
2708                 if (cmd == FUTEX_WAIT)
2709                         t = ktime_add_safe(ktime_get(), t);
2710                 tp = &t;
2711         }

113 #define timespec_valid(ts) \
114         (((ts)->tv_sec >= 0) && (((unsigned long) (ts)->tv_nsec) < NSEC_PER_SEC))

So it will return EINVAL for bogus timespec.

Re: [PATCH 09/13] linux: nptl: Replace lll_futex_{timed_}wait by futex-internal.h

[Mike Crowe via Libc-alpha]

On Wednesday 25 November 2020 at 14:19:37 -0300, Adhemerval Zanella wrote:
> On 25/11/2020 12:46, Mike Crowe wrote:
> > On Wednesday 25 November 2020 at 12:40:46 -0300, Adhemerval Zanella wrote:
> >>
> >>
> >> On 25/11/2020 12:32, Mike Crowe wrote:
> >>> On Monday 23 November 2020 at 16:52:52 -0300, Adhemerval Zanella wrote:
> >>>> The idea is to make NPTL implementation to use on the functions
> >>>> provided by futex-internal.h.
> >>>>
> >>>> Checked on x86_64-linux-gnu and i686-linux-gnu.
> >>>> ---
> >>>>  nptl/lowlevellock.c                 | 6 +++---
> >>>>  nptl/pthread_mutex_lock.c           | 9 +++++----
> >>>>  nptl/pthread_mutex_setprioceiling.c | 5 +++--
> >>>>  nptl/pthread_mutex_timedlock.c      | 6 +++---
> >>>>  4 files changed, 14 insertions(+), 12 deletions(-)
> >>>
> >>> [snip]
> >>>
> >>>> diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
> >>>> index e643eab258..343acf6107 100644
> >>>> --- a/nptl/pthread_mutex_timedlock.c
> >>>> +++ b/nptl/pthread_mutex_timedlock.c
> >>>> @@ -561,9 +561,9 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
> >>>>  			goto failpp;
> >>>>  		      }
> >>>>  
> >>>> -		    lll_futex_timed_wait (&mutex->__data.__lock,
> >>>> -					  ceilval | 2, &rt,
> >>>> -					  PTHREAD_MUTEX_PSHARED (mutex));
> >>>> +		    __futex_abstimed_wait64 (
> >>>> +		      (unsigned int *) &mutex->__data.__lock, clockid,
> >>>> +		      ceilval | 2, &rt, PTHREAD_MUTEX_PSHARED (mutex));
> >>>
> >>> I think you've replaced the lll_futex_timed_wait call that expects a
> >>> relative timeout with a __futex_abstimed_wait64 call that expects an
> >>> absolute timeout, yet you still appear to be passing the relative timeout.
> >>>
> >>> However, it turns out that the implementation for the
> >>> PTHREAD_MUTEX_PP_RECURSIVE_NP and friends case appears to be have been
> >>> completely broken with clockid != CLOCK_REALTIME ever since I added it in
> >>> 9d20e22e46d891b929a72b0f35586e079eb083fd anyway since the relative timeout
> >>> is calculated by calling __clock_gettime64(CLOCK_REALTIME) (although at the
> >>> time this was a less obvious __gettimeofday call.)
> >>>
> >>> I'll work on writing some test cases for the those types of mutex in the
> >>> hope of catching both flaws before fixing them.
> >>
> >> Indeed, there is no need to calculate the relative timeout anymore. I think
> >> the fix below should pass the absolute timeout directly.   I will check
> >> a possible regression tests as well.
> > 
> > OK. I won't then. Thanks.
> > 
> >> diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
> >> index aaaafa21ce..86c5f4446e 100644
> >> --- a/nptl/pthread_mutex_timedlock.c
> >> +++ b/nptl/pthread_mutex_timedlock.c
> >> @@ -508,7 +508,6 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
> >>  	    if (__pthread_current_priority () > ceiling)
> >>  	      {
> >>  		result = EINVAL;
> >> -	      failpp:
> >>  		if (oldprio != -1)
> >>  		  __pthread_tpp_change_priority (oldprio, -1);
> >>  		return result;
> >> @@ -540,37 +539,9 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
> >>  
> >>  		if (oldval != ceilval)
> >>  		  {
> >> -		    /* Reject invalid timeouts.  */
> >> -		    if (! valid_nanoseconds (abstime->tv_nsec))
> >> -		      {
> >> -			result = EINVAL;
> >> -			goto failpp;
> >> -		      }
> > 
> > If this is removed then is there a risk of getting into a busy loop if
> > someone passes a bogus timespec? (Regardless of the answer, it makes sense
> > to ensure that is tested somehow.)
> 
> The minimum supported kernel already does the same check on the futex call
> (source for Linux 3.2):
> 
> 2690 SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val,
> 2691                 struct timespec __user *, utime, u32 __user *, uaddr2,
> 2692                 u32, val3)
> 2693 {
> 2694         struct timespec ts;
> 2695         ktime_t t, *tp = NULL;
> 2696         u32 val2 = 0;
> 2697         int cmd = op & FUTEX_CMD_MASK;
> 2698 
> 2699         if (utime && (cmd == FUTEX_WAIT || cmd == FUTEX_LOCK_PI ||
> 2700                       cmd == FUTEX_WAIT_BITSET ||
> 2701                       cmd == FUTEX_WAIT_REQUEUE_PI)) {
> 2702                 if (copy_from_user(&ts, utime, sizeof(ts)) != 0)
> 2703                         return -EFAULT;
> 2704                 if (!timespec_valid(&ts))
> 2705                         return -EINVAL;
> 2706 
> 2707                 t = timespec_to_ktime(ts);
> 2708                 if (cmd == FUTEX_WAIT)
> 2709                         t = ktime_add_safe(ktime_get(), t);
> 2710                 tp = &t;
> 2711         }
> 
> 113 #define timespec_valid(ts) \
> 114         (((ts)->tv_sec >= 0) && (((unsigned long) (ts)->tv_nsec) < NSEC_PER_SEC))
> 
> So it will return EINVAL for bogus timespec.

Yes, but here:

>                     __futex_abstimed_wait64 (
>                       (unsigned int *) &mutex->__data.__lock, clockid,
> -                     ceilval | 2, &rt, PTHREAD_MUTEX_PSHARED (mutex));
> +                     ceilval | 2, abstime, PTHREAD_MUTEX_PSHARED (mutex));

the return value of __futex_abstimed_wait64 is not checked, so the loop
might just spin around busily until the timeout expires. Perhaps the return
value needs checking too?

Thanks.

Mike.

Re: [PATCH 09/13] linux: nptl: Replace lll_futex_{timed_}wait by futex-internal.h

[Adhemerval Zanella via Libc-alpha]

On 25/11/2020 14:37, Mike Crowe wrote:
> On Wednesday 25 November 2020 at 14:19:37 -0300, Adhemerval Zanella wrote:
>> On 25/11/2020 12:46, Mike Crowe wrote:
>>> On Wednesday 25 November 2020 at 12:40:46 -0300, Adhemerval Zanella wrote:
>>>>
>>>>
>>>> On 25/11/2020 12:32, Mike Crowe wrote:
>>>>> On Monday 23 November 2020 at 16:52:52 -0300, Adhemerval Zanella wrote:
>>>>>> The idea is to make NPTL implementation to use on the functions
>>>>>> provided by futex-internal.h.
>>>>>>
>>>>>> Checked on x86_64-linux-gnu and i686-linux-gnu.
>>>>>> ---
>>>>>>  nptl/lowlevellock.c                 | 6 +++---
>>>>>>  nptl/pthread_mutex_lock.c           | 9 +++++----
>>>>>>  nptl/pthread_mutex_setprioceiling.c | 5 +++--
>>>>>>  nptl/pthread_mutex_timedlock.c      | 6 +++---
>>>>>>  4 files changed, 14 insertions(+), 12 deletions(-)
>>>>>
>>>>> [snip]
>>>>>
>>>>>> diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
>>>>>> index e643eab258..343acf6107 100644
>>>>>> --- a/nptl/pthread_mutex_timedlock.c
>>>>>> +++ b/nptl/pthread_mutex_timedlock.c
>>>>>> @@ -561,9 +561,9 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
>>>>>>  			goto failpp;
>>>>>>  		      }
>>>>>>  
>>>>>> -		    lll_futex_timed_wait (&mutex->__data.__lock,
>>>>>> -					  ceilval | 2, &rt,
>>>>>> -					  PTHREAD_MUTEX_PSHARED (mutex));
>>>>>> +		    __futex_abstimed_wait64 (
>>>>>> +		      (unsigned int *) &mutex->__data.__lock, clockid,
>>>>>> +		      ceilval | 2, &rt, PTHREAD_MUTEX_PSHARED (mutex));
>>>>>
>>>>> I think you've replaced the lll_futex_timed_wait call that expects a
>>>>> relative timeout with a __futex_abstimed_wait64 call that expects an
>>>>> absolute timeout, yet you still appear to be passing the relative timeout.
>>>>>
>>>>> However, it turns out that the implementation for the
>>>>> PTHREAD_MUTEX_PP_RECURSIVE_NP and friends case appears to be have been
>>>>> completely broken with clockid != CLOCK_REALTIME ever since I added it in
>>>>> 9d20e22e46d891b929a72b0f35586e079eb083fd anyway since the relative timeout
>>>>> is calculated by calling __clock_gettime64(CLOCK_REALTIME) (although at the
>>>>> time this was a less obvious __gettimeofday call.)
>>>>>
>>>>> I'll work on writing some test cases for the those types of mutex in the
>>>>> hope of catching both flaws before fixing them.
>>>>
>>>> Indeed, there is no need to calculate the relative timeout anymore. I think
>>>> the fix below should pass the absolute timeout directly.   I will check
>>>> a possible regression tests as well.
>>>
>>> OK. I won't then. Thanks.
>>>
>>>> diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
>>>> index aaaafa21ce..86c5f4446e 100644
>>>> --- a/nptl/pthread_mutex_timedlock.c
>>>> +++ b/nptl/pthread_mutex_timedlock.c
>>>> @@ -508,7 +508,6 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
>>>>  	    if (__pthread_current_priority () > ceiling)
>>>>  	      {
>>>>  		result = EINVAL;
>>>> -	      failpp:
>>>>  		if (oldprio != -1)
>>>>  		  __pthread_tpp_change_priority (oldprio, -1);
>>>>  		return result;
>>>> @@ -540,37 +539,9 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
>>>>  
>>>>  		if (oldval != ceilval)
>>>>  		  {
>>>> -		    /* Reject invalid timeouts.  */
>>>> -		    if (! valid_nanoseconds (abstime->tv_nsec))
>>>> -		      {
>>>> -			result = EINVAL;
>>>> -			goto failpp;
>>>> -		      }
>>>
>>> If this is removed then is there a risk of getting into a busy loop if
>>> someone passes a bogus timespec? (Regardless of the answer, it makes sense
>>> to ensure that is tested somehow.)
>>
>> The minimum supported kernel already does the same check on the futex call
>> (source for Linux 3.2):
>>
>> 2690 SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val,
>> 2691                 struct timespec __user *, utime, u32 __user *, uaddr2,
>> 2692                 u32, val3)
>> 2693 {
>> 2694         struct timespec ts;
>> 2695         ktime_t t, *tp = NULL;
>> 2696         u32 val2 = 0;
>> 2697         int cmd = op & FUTEX_CMD_MASK;
>> 2698 
>> 2699         if (utime && (cmd == FUTEX_WAIT || cmd == FUTEX_LOCK_PI ||
>> 2700                       cmd == FUTEX_WAIT_BITSET ||
>> 2701                       cmd == FUTEX_WAIT_REQUEUE_PI)) {
>> 2702                 if (copy_from_user(&ts, utime, sizeof(ts)) != 0)
>> 2703                         return -EFAULT;
>> 2704                 if (!timespec_valid(&ts))
>> 2705                         return -EINVAL;
>> 2706 
>> 2707                 t = timespec_to_ktime(ts);
>> 2708                 if (cmd == FUTEX_WAIT)
>> 2709                         t = ktime_add_safe(ktime_get(), t);
>> 2710                 tp = &t;
>> 2711         }
>>
>> 113 #define timespec_valid(ts) \
>> 114         (((ts)->tv_sec >= 0) && (((unsigned long) (ts)->tv_nsec) < NSEC_PER_SEC))
>>
>> So it will return EINVAL for bogus timespec.
> 
> Yes, but here:
> 
>>                     __futex_abstimed_wait64 (
>>                       (unsigned int *) &mutex->__data.__lock, clockid,
>> -                     ceilval | 2, &rt, PTHREAD_MUTEX_PSHARED (mutex));
>> +                     ceilval | 2, abstime, PTHREAD_MUTEX_PSHARED (mutex));
> 
> the return value of __futex_abstimed_wait64 is not checked, so the loop
> might just spin around busily until the timeout expires. Perhaps the return
> value needs checking too?

Indeed, we need to check for ETIMEDOUT/EOVERFLOW.

Re: [PATCH v2 12/13] nptl: Return EINVAL for pthread_mutex_clocklock/PI with CLOCK_MONOTONIC [BZ #26801]

[Adhemerval Zanella via Libc-alpha]

[-- Attachment #1.1: Type: text/plain, Size: 428 bytes --]



On 24/11/2020 18:49, Lukasz Majewski wrote:
>> b/nptl/tst-mutexpi10.c new file mode 100644
>> index 0000000000..84ba1dfa97
>> --- /dev/null
>> +++ b/nptl/tst-mutexpi10.c
>> @@ -0,0 +1,68 @@
>> +/* Check if pthread_mutex_clocklock with PRIO_INHERIT fails with
>> clock
>> +   different than CLOCK_REALTIME.
>> +   Copyright (C) 2015-2020 Free Software Foundation, Inc.
> 
> Minor -> 2020 only

Thanks, fixed.


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

Re: [PATCH 03/13] nptl: Remove clockwait_tid

[Florian Weimer via Libc-alpha]

* Adhemerval Zanella via Libc-alpha:

> -static __always_inline int
> -futex_timed_wait_cancel64 (pid_t *tidp,  pid_t tid,
> -                           const struct __timespec64 *timeout, int private)
> -{
> -  int err = INTERNAL_SYSCALL_CANCEL (futex_time64, tidp,
> -                                     __lll_private_flag (FUTEX_WAIT, private),
> -                                     tid, timeout);

This uses FUTEX_WAIT.  But the replacement,
__futex_abstimed_wait_common64, uses FUTEX_WAIT_BITSET.  I do not think
this is correct because the kernel will use FUTEX_WAKE internally for
the pd->tid wakeup relied upon by pthread_join.

This seems to cause pthread_join regressions on some kernel versions.

We need to audit all callers of __futex_abstimed_wait64 if they are
actually compatible with FUTEX_WAIT_BITSET.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: [PATCH 03/13] nptl: Remove clockwait_tid

[Andreas Schwab]

On Dez 14 2020, Florian Weimer via Libc-alpha wrote:

> This uses FUTEX_WAIT.  But the replacement,
> __futex_abstimed_wait_common64, uses FUTEX_WAIT_BITSET.

FUTEX_WAIT is exactly equivalent to
FUTEX_WAIT_BITSET/FUTEX_BITSET_MATCH_ANY.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH 03/13] nptl: Remove clockwait_tid

[Adhemerval Zanella via Libc-alpha]

On 14/12/2020 09:16, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
> 
>> -static __always_inline int
>> -futex_timed_wait_cancel64 (pid_t *tidp,  pid_t tid,
>> -                           const struct __timespec64 *timeout, int private)
>> -{
>> -  int err = INTERNAL_SYSCALL_CANCEL (futex_time64, tidp,
>> -                                     __lll_private_flag (FUTEX_WAIT, private),
>> -                                     tid, timeout);
> 
> This uses FUTEX_WAIT.  But the replacement,
> __futex_abstimed_wait_common64, uses FUTEX_WAIT_BITSET.  I do not think
> this is correct because the kernel will use FUTEX_WAKE internally for
> the pd->tid wakeup relied upon by pthread_join.
> 
> This seems to cause pthread_join regressions on some kernel versions.
> 
> We need to audit all callers of __futex_abstimed_wait64 if they are
> actually compatible with FUTEX_WAIT_BITSET.

I don't think it should matter, as Andreas has put FUTEX_WAIT is exactly
as FUTEX_WAIT_BITSET plus FUTEX_BITSET_MATCH_ANY.  On a recent kernel:


  SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val,
                  struct __kernel_timespec __user *, utime, u32 __user *, uaddr2,
                  u32, val3)
  {
  [...]
    return do_futex(uaddr, op, val, tp, uaddr2, val2, val3);
  }

  long do_futex(u32 __user *uaddr, int op, u32 val, ktime_t *timeout,
                u32 __user *uaddr2, u32 val2, u32 val3)
  {
  [...]
          case FUTEX_WAIT:
                  val3 = FUTEX_BITSET_MATCH_ANY;
                  fallthrough;
          case FUTEX_WAIT_BITSET:
                  return futex_wait(uaddr, flags, val, timeout, val3);
  [...]

Re: [PATCH 03/13] nptl: Remove clockwait_tid

[Florian Weimer via Libc-alpha]

* Andreas Schwab:

> On Dez 14 2020, Florian Weimer via Libc-alpha wrote:
>
>> This uses FUTEX_WAIT.  But the replacement,
>> __futex_abstimed_wait_common64, uses FUTEX_WAIT_BITSET.
>
> FUTEX_WAIT is exactly equivalent to
> FUTEX_WAIT_BITSET/FUTEX_BITSET_MATCH_ANY.

Hmm.  I will continue debugging.  I encounter a missed wakeup in
pthread_join after making some changes that only should affect timing
(intl/tst-gettext6 is among the failures).

Reverting this patch here seems to help.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: [PATCH 03/13] nptl: Remove clockwait_tid

[Florian Weimer via Libc-alpha]

* Florian Weimer:

> * Andreas Schwab:
>
>> On Dez 14 2020, Florian Weimer via Libc-alpha wrote:
>>
>>> This uses FUTEX_WAIT.  But the replacement,
>>> __futex_abstimed_wait_common64, uses FUTEX_WAIT_BITSET.
>>
>> FUTEX_WAIT is exactly equivalent to
>> FUTEX_WAIT_BITSET/FUTEX_BITSET_MATCH_ANY.
>
> Hmm.  I will continue debugging.  I encounter a missed wakeup in
> pthread_join after making some changes that only should affect timing
> (intl/tst-gettext6 is among the failures).
>
> Reverting this patch here seems to help.

It's an accident, due to my changes and:

  /* In libc.so or ld.so all futexes are private.  */

in lowlevellock-futex.h.  So the patch posted here should be okay.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill