[3/7] nptl: Add POSIX-proposed pthread_cond_clockwait
diff mbox series

Message ID 63007433d8e317b58abeac24674cb0b8ae31c33f.1551291557.git-series.mac@mcrowe.com
State New
Headers show
Series
  • Implement proposed POSIX _clockwait variants of existing _timedwait functions
Related show

Commit Message

Mike Crowe Feb. 27, 2019, 6:23 p.m. UTC
Add:

 int pthread_cond_clockwait (pthread_cond_t *cond,
                             pthread_mutex_t *mutex,
                             clockid_t clockid,
                             const struct timespec *abstime)

which behaves just like pthread_cond_timedwait except it always measures
abstime against the supplied clockid. Currently supports CLOCK_REALTIME and
CLOCK_MONOTONIC and returns EINVAL if any other clock is specified.

Includes feedback from many others. This function was originally
proposed[1] as pthread_cond_timedwaitonclock_np, but The Austin Group
preferred the new name.

* nptl/Makefile: Add tst-cond26 and tst-cond27

* nptl/Versions (GLIBC_2.30): Add pthread_cond_clockwait

* sysdeps/nptl/pthread.h: Likewise

* nptl/forward.c: Add __pthread_cond_clockwait (not sure if it should be
  __USE_GNU while it's still only proposed for POSIX.)

* nptl/forward.c: Likewise

* nptl/pthreadP.h: Likewise

* sysdeps/nptl/pthread-functions.h: Likewise.

* nptl/pthread_cond_wait.c (__pthread_cond_wait_common): Add clockid
  parameter and comment describing why we don't need to check its value.
  Use that value rather than reading the clock from the flags.
  (__pthread_cond_wait): Pass unused clockid parameter.
  (__pthread_cond_timedwait): Read clock from flags and pass it to
  __pthread_cond_wait_common. (__pthread_cond_clockwait): Add new function
  with weak alias from pthread_cond_clockwait.

* nptl/tst-cond11.c (run_test): Support testing pthread_cond_clockwait too
  by using a special magic CLOCK_USE_ATTR_CLOCK value to determine whether
  to call pthread_cond_timedwait or pthread_cond_clockwait. (do_test): Pass
  CLOCK_USE_ATTR_CLOCK for existing tests, and add new tests using all
  combinations of CLOCK_MONOTONIC and CLOCK_REALTIME.

* ntpl/tst-cond26.c: New test for passing unsupported and invalid clocks to
  pthread_cond_clockwait.

* nptl/tst-cond27.c: Add test similar to tst-cond5.c, but using struct
  timespec and pthread_cond_clockwait.

* sysdeps/unix/sysv/linux/arm/libpthread.abilist,
 sysdeps/unix/sysv/linux/i386/libpthread.abilist,
 sysdeps/unix/sysv/linux/x86_64/64/libpthread.abilist: Add
 pthread_cond_clockwait

* manual/threads.texi: Document pthread_cond_clockwait. The comment was
  provided by Carlos O'Donell.

[1] https://sourceware.org/ml/libc-alpha/2015-07/msg00193.html
---
 manual/threads.texi                                  |  20 ++-
 nptl/Makefile                                        |   1 +-
 nptl/Versions                                        |   2 +-
 nptl/forward.c                                       |   5 +-
 nptl/nptl-init.c                                     |   1 +-
 nptl/pthreadP.h                                      |   4 +-
 nptl/pthread_cond_wait.c                             |  43 ++++-
 nptl/tst-cond11.c                                    |  30 ++-
 nptl/tst-cond26.c                                    |  91 ++++++++++-
 nptl/tst-cond27.c                                    | 113 ++++++++++++-
 sysdeps/nptl/pthread-functions.h                     |   4 +-
 sysdeps/nptl/pthread.h                               |  13 +-
 sysdeps/unix/sysv/linux/x86_64/64/libpthread.abilist |   1 +-
 13 files changed, 314 insertions(+), 14 deletions(-)
 create mode 100644 nptl/tst-cond26.c
 create mode 100644 nptl/tst-cond27.c

Comments

Adhemerval Zanella March 5, 2019, 4:45 p.m. UTC | #1
On 27/02/2019 15:23, Mike Crowe wrote:
> Add:
> 
>  int pthread_cond_clockwait (pthread_cond_t *cond,
>                              pthread_mutex_t *mutex,
>                              clockid_t clockid,
>                              const struct timespec *abstime)
> 
> which behaves just like pthread_cond_timedwait except it always measures
> abstime against the supplied clockid. Currently supports CLOCK_REALTIME and
> CLOCK_MONOTONIC and returns EINVAL if any other clock is specified.
> 
> Includes feedback from many others. This function was originally
> proposed[1] as pthread_cond_timedwaitonclock_np, but The Austin Group
> preferred the new name.
> 
> * nptl/Makefile: Add tst-cond26 and tst-cond27
> 
> * nptl/Versions (GLIBC_2.30): Add pthread_cond_clockwait
> 
> * sysdeps/nptl/pthread.h: Likewise
> 
> * nptl/forward.c: Add __pthread_cond_clockwait (not sure if it should be
>   __USE_GNU while it's still only proposed for POSIX.)
> 
> * nptl/forward.c: Likewise
> 
> * nptl/pthreadP.h: Likewise
> 
> * sysdeps/nptl/pthread-functions.h: Likewise.
> 
> * nptl/pthread_cond_wait.c (__pthread_cond_wait_common): Add clockid
>   parameter and comment describing why we don't need to check its value.
>   Use that value rather than reading the clock from the flags.
>   (__pthread_cond_wait): Pass unused clockid parameter.
>   (__pthread_cond_timedwait): Read clock from flags and pass it to
>   __pthread_cond_wait_common. (__pthread_cond_clockwait): Add new function
>   with weak alias from pthread_cond_clockwait.
> 
> * nptl/tst-cond11.c (run_test): Support testing pthread_cond_clockwait too
>   by using a special magic CLOCK_USE_ATTR_CLOCK value to determine whether
>   to call pthread_cond_timedwait or pthread_cond_clockwait. (do_test): Pass
>   CLOCK_USE_ATTR_CLOCK for existing tests, and add new tests using all
>   combinations of CLOCK_MONOTONIC and CLOCK_REALTIME.
> 
> * ntpl/tst-cond26.c: New test for passing unsupported and invalid clocks to
>   pthread_cond_clockwait.
> 
> * nptl/tst-cond27.c: Add test similar to tst-cond5.c, but using struct
>   timespec and pthread_cond_clockwait.
> 
> * sysdeps/unix/sysv/linux/arm/libpthread.abilist,
>  sysdeps/unix/sysv/linux/i386/libpthread.abilist,
>  sysdeps/unix/sysv/linux/x86_64/64/libpthread.abilist: Add
>  pthread_cond_clockwait
> 
> * manual/threads.texi: Document pthread_cond_clockwait. The comment was
>   provided by Carlos O'Donell.
> 
> [1] https://sourceware.org/ml/libc-alpha/2015-07/msg00193.html
> ---
>  manual/threads.texi                                  |  20 ++-
>  nptl/Makefile                                        |   1 +-
>  nptl/Versions                                        |   2 +-
>  nptl/forward.c                                       |   5 +-
>  nptl/nptl-init.c                                     |   1 +-
>  nptl/pthreadP.h                                      |   4 +-
>  nptl/pthread_cond_wait.c                             |  43 ++++-
>  nptl/tst-cond11.c                                    |  30 ++-
>  nptl/tst-cond26.c                                    |  91 ++++++++++-
>  nptl/tst-cond27.c                                    | 113 ++++++++++++-
>  sysdeps/nptl/pthread-functions.h                     |   4 +-
>  sysdeps/nptl/pthread.h                               |  13 +-
>  sysdeps/unix/sysv/linux/x86_64/64/libpthread.abilist |   1 +-
>  13 files changed, 314 insertions(+), 14 deletions(-)
>  create mode 100644 nptl/tst-cond26.c
>  create mode 100644 nptl/tst-cond27.c
> 
> diff --git a/manual/threads.texi b/manual/threads.texi
> index 674267c..91462f5 100644
> --- a/manual/threads.texi
> +++ b/manual/threads.texi
> @@ -679,6 +679,26 @@ against the clock specified by @var{clockid} rather than
>  @code{CLOCK_MONOTONIC} or @code{CLOCK_REALTIME}.
>  @end deftypefun
>  
> +@comment pthread.h
> +@comment POSIX-proposed
> +@deftypefun int pthread_cond_clockwait (pthread_cond_t *@var{cond}, pthread_mutex_t *@var{mutex},
> +                                        clockid_t @var{clockid}, const struct timespec *@var{abstime})
> +@safety{@prelim{}@mtsafe{}@asunsafe{@asulock{}}@acunsafe{@aculock{}}}
> +@c If exactly the same function with arguments is called from a signal
> +@c handler that interrupts between the mutex unlock and sleep then it
> +@c will unlock the mutex twice resulting in undefined behaviour. Keep
> +@c in mind that the unlock and sleep are only atomic with respect to other
> +@c threads (really a happens-after relationship for pthread_cond_broadcast
> +@c and pthread_cond_signal).
> +@c In the AC case we would cancel the thread and the mutex would remain
> +@c locked and we can't recover from that.
> +Behaves like @code{pthread_cond_timedwait} except the time @var{abstime} is
> +measured against the clock specified by @var{clockid} rather than the clock
> +specified or defaulted when @code{pthread_cond_init} was called. Currently,
> +@var{clockid} must be either @code{CLOCK_MONOTONIC} or
> +@code{CLOCK_REALTIME}.
> +@end deftypefun
> +
>  @c FIXME these are undocumented:
>  @c pthread_atfork
>  @c pthread_attr_destroy
> diff --git a/nptl/Makefile b/nptl/Makefile
> index 4c9f5d3..7de4d40 100644
> --- a/nptl/Makefile
> +++ b/nptl/Makefile
> @@ -251,6 +251,7 @@ tests = tst-attr1 tst-attr2 tst-attr3 tst-default-attr \
>  	tst-cond8 tst-cond9 tst-cond10 tst-cond11 tst-cond12 tst-cond13 \
>  	tst-cond14 tst-cond15 tst-cond16 tst-cond17 tst-cond18 tst-cond19 \
>  	tst-cond20 tst-cond21 tst-cond22 tst-cond23 tst-cond24 tst-cond25 \
> +	tst-cond26 tst-cond27 \
>  	tst-cond-except \
>  	tst-robust1 tst-robust2 tst-robust3 tst-robust4 tst-robust5 \
>  	tst-robust6 tst-robust7 tst-robust8 tst-robust9 \
> diff --git a/nptl/Versions b/nptl/Versions
> index cd1806c..8c094d0 100644
> --- a/nptl/Versions
> +++ b/nptl/Versions
> @@ -278,7 +278,7 @@ libpthread {
>    }
>  
>    GLIBC_2.30 {
> -    sem_clockwait;
> +    sem_clockwait; pthread_cond_clockwait;
>    }
>  

This has the same issue brought by Joseph in previous patch.

>    GLIBC_PRIVATE {
> diff --git a/nptl/forward.c b/nptl/forward.c
> index ed1e7d0..50f358f 100644
> --- a/nptl/forward.c
> +++ b/nptl/forward.c
> @@ -164,6 +164,11 @@ FORWARD (__pthread_cond_timedwait,
>  	  const struct timespec *abstime), (cond, mutex, abstime), 0)
>  versioned_symbol (libc, __pthread_cond_timedwait, pthread_cond_timedwait,
>  		  GLIBC_2_3_2);
> +FORWARD (__pthread_cond_clockwait,
> +	 (pthread_cond_t *cond, pthread_mutex_t *mutex, clockid_t clockid,
> +	  const struct timespec *abstime), (cond, mutex, clockid, abstime),
> +	 0)
> +weak_alias (__pthread_cond_clockwait, pthread_cond_clockwait);
>  
>  
>  FORWARD (pthread_equal, (pthread_t thread1, pthread_t thread2),
> diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c
> index b5895fa..c872b9f 100644
> --- a/nptl/nptl-init.c
> +++ b/nptl/nptl-init.c
> @@ -104,6 +104,7 @@ static const struct pthread_functions pthread_functions =
>      .ptr___pthread_cond_signal = __pthread_cond_signal,
>      .ptr___pthread_cond_wait = __pthread_cond_wait,
>      .ptr___pthread_cond_timedwait = __pthread_cond_timedwait,
> +    .ptr___pthread_cond_clockwait = __pthread_cond_clockwait,
>  # if SHLIB_COMPAT(libpthread, GLIBC_2_0, GLIBC_2_3_2)
>      .ptr___pthread_cond_broadcast_2_0 = __pthread_cond_broadcast_2_0,
>      .ptr___pthread_cond_destroy_2_0 = __pthread_cond_destroy_2_0,
> diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
> index 626bd4b..28c0ee5 100644
> --- a/nptl/pthreadP.h
> +++ b/nptl/pthreadP.h
> @@ -487,6 +487,10 @@ extern int __pthread_cond_wait (pthread_cond_t *cond, pthread_mutex_t *mutex);
>  extern int __pthread_cond_timedwait (pthread_cond_t *cond,
>  				     pthread_mutex_t *mutex,
>  				     const struct timespec *abstime);
> +extern int __pthread_cond_clockwait (pthread_cond_t *cond,
> +				     pthread_mutex_t *mutex,
> +				     clockid_t clockid,
> +				     const struct timespec *abstime);
>  extern int __pthread_condattr_destroy (pthread_condattr_t *attr);
>  extern int __pthread_condattr_init (pthread_condattr_t *attr);
>  extern int __pthread_key_create (pthread_key_t *key, void (*destr) (void *));
> diff --git a/nptl/pthread_cond_wait.c b/nptl/pthread_cond_wait.c
> index daa4e25..5deb54b 100644
> --- a/nptl/pthread_cond_wait.c
> +++ b/nptl/pthread_cond_wait.c
> @@ -378,6 +378,7 @@ __condvar_cleanup_waiting (void *arg)
>  */
>  static __always_inline int
>  __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex,
> +    clockid_t clockid,
>      const struct timespec *abstime)
>  {
>    const int maxspin = 0;
> @@ -386,6 +387,11 @@ __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex,
>  
>    LIBC_PROBE (cond_wait, 2, cond, mutex);
>  
> +  /* clockid will already have been checked by
> +     __pthread_cond_clockwait or pthread_condattr_setclock, or we
> +     don't use it if abstime is NULL, so we don't need to check it
> +     here. */
> +

Ok.

>    /* Acquire a position (SEQ) in the waiter sequence (WSEQ).  We use an
>       atomic operation because signals and broadcasts may update the group
>       switch without acquiring the mutex.  We do not need release MO here
> @@ -510,7 +516,7 @@ __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex,
>  	      if (__glibc_unlikely (abstime->tv_sec < 0))
>  	        err = ETIMEDOUT;
>  
> -	      else if ((flags & __PTHREAD_COND_CLOCK_MONOTONIC_MASK) != 0)
> +	      else if (clockid == CLOCK_MONOTONIC)
>  		{
>  		  /* CLOCK_MONOTONIC is requested.  */
>  		  struct timespec rt;
> @@ -652,7 +658,8 @@ __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex,
>  int
>  __pthread_cond_wait (pthread_cond_t *cond, pthread_mutex_t *mutex)
>  {
> -  return __pthread_cond_wait_common (cond, mutex, NULL);
> +  /* clockid is unused when abstime is NULL. */
> +  return __pthread_cond_wait_common (cond, mutex, 0, NULL);
>  }
>  
>  /* See __pthread_cond_wait_common.  */

Ok.

> @@ -664,10 +671,40 @@ __pthread_cond_timedwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
>       it can assume that abstime is not NULL.  */
>    if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
>      return EINVAL;
> -  return __pthread_cond_wait_common (cond, mutex, abstime);
> +
> +  /* Relaxed MO is suffice because clock ID bit is only modified
> +     in condition creation.  */
> +  unsigned int flags = atomic_load_relaxed (&cond->__data.__wrefs);
> +  clockid_t clockid = (flags & __PTHREAD_COND_CLOCK_MONOTONIC_MASK)
> +                    ? CLOCK_MONOTONIC : CLOCK_REALTIME;
> +  return __pthread_cond_wait_common (cond, mutex, clockid, abstime);
> +}
> +

Ok.

> +/* See __pthread_cond_wait_common.  */
> +int
> +__pthread_cond_clockwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
> +			  clockid_t clockid,
> +			  const struct timespec *abstime)
> +{
> +  /* Check parameter validity.  This should also tell the compiler that
> +     it can assume that abstime is not NULL.  */
> +  if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
> +    return EINVAL;

The timespec check is used in different parts internally, I wonder
if it would better to consolidate it somewhere.

> +
> +  /* We only support CLOCK_REALTIME and CLOCK_MONOTONIC */
> +  if (clockid != CLOCK_REALTIME && clockid != CLOCK_MONOTONIC)
> +    return EINVAL;

Why not use lll_futex_supported_clockid here?

> +
> +  /* If we do not support waiting using CLOCK_MONOTONIC, return an error.  */
> +  if (clockid == CLOCK_MONOTONIC
> +      && !futex_supports_exact_relative_timeouts ())
> +    return EINVAL;

Why exactly do we need futex_supports_exact_relative_timeouts if Linux
always set it to true?

> +
> +  return __pthread_cond_wait_common (cond, mutex, clockid, abstime);
>  }
>  
>  versioned_symbol (libpthread, __pthread_cond_wait, pthread_cond_wait,
>  		  GLIBC_2_3_2);
>  versioned_symbol (libpthread, __pthread_cond_timedwait, pthread_cond_timedwait,
>  		  GLIBC_2_3_2);
> +weak_alias (__pthread_cond_clockwait, pthread_cond_clockwait);
> diff --git a/nptl/tst-cond11.c b/nptl/tst-cond11.c
> index 97a8bd0..4df6b15 100644
> --- a/nptl/tst-cond11.c
> +++ b/nptl/tst-cond11.c
> @@ -22,17 +22,20 @@
>  #include <time.h>
>  #include <unistd.h>
Not a requisite, but since you are touching the testcase it might a
good time to change it to use libsupport.

>  
> +/* A bogus clock value that tells run_test to use
> +   pthread_cond_timedwait rather than pthread_condclockwait. */
> +#define CLOCK_USE_ATTR_CLOCK (-1)
>  
>  #if defined _POSIX_CLOCK_SELECTION && _POSIX_CLOCK_SELECTION >= 0
>  static int
> -run_test (clockid_t cl)
> +run_test (clockid_t attr_clock, clockid_t wait_clock)
>  {
>    pthread_condattr_t condattr;
>    pthread_cond_t cond;
>    pthread_mutexattr_t mutattr;
>    pthread_mutex_t mut;
>  
> -  printf ("clock = %d\n", (int) cl);
> +  printf ("attr_clock = %d\n", (int) attr_clock);
>  
>    if (pthread_condattr_init (&condattr) != 0)
>      {
> @@ -40,7 +43,7 @@ run_test (clockid_t cl)
>        return 1;
>      }
>  
> -  if (pthread_condattr_setclock (&condattr, cl) != 0)
> +  if (pthread_condattr_setclock (&condattr, attr_clock) != 0)
>      {
>        puts ("condattr_setclock failed");
>        return 1;
> @@ -52,10 +55,10 @@ run_test (clockid_t cl)
>        puts ("condattr_getclock failed");
>        return 1;
>      }
> -  if (cl != cl2)
> +  if (attr_clock != cl2)
>      {
>        printf ("condattr_getclock returned wrong value: %d, expected %d\n",
> -	      (int) cl2, (int) cl);
> +	      (int) cl2, (int) attr_clock);
>        return 1;
>      }
>  
> @@ -108,7 +111,7 @@ run_test (clockid_t cl)
>      }
>  
>    struct timespec ts;
> -  if (clock_gettime (cl, &ts) != 0)
> +  if (clock_gettime ((wait_clock == CLOCK_USE_ATTR_CLOCK) ? attr_clock : wait_clock, &ts) != 0)

Line too long.

>      {
>        puts ("clock_gettime failed");
>        return 1;
> @@ -117,7 +120,9 @@ run_test (clockid_t cl)
>    /* Wait one second.  */
>    ++ts.tv_sec;
>  
> -  int e = pthread_cond_timedwait (&cond, &mut, &ts);
> +  int e = (wait_clock == CLOCK_USE_ATTR_CLOCK)
> +    ? pthread_cond_timedwait (&cond, &mut, &ts)
> +    : pthread_cond_clockwait (&cond, &mut, wait_clock, &ts);
>    if (e == 0)
>      {
>        puts ("cond_timedwait succeeded");
> @@ -130,7 +135,7 @@ run_test (clockid_t cl)
>      }
>  
>    struct timespec ts2;
> -  if (clock_gettime (cl, &ts2) != 0)
> +  if (clock_gettime ((wait_clock == CLOCK_USE_ATTR_CLOCK) ? attr_clock : wait_clock, &ts2) != 0)

Ditto.

>      {
>        puts ("second clock_gettime failed");
>        return 1;
> @@ -176,7 +181,7 @@ do_test (void)
>  
>  #else
>  
> -  int res = run_test (CLOCK_REALTIME);
> +  int res = run_test (CLOCK_REALTIME, CLOCK_USE_ATTR_CLOCK);
>  
>  # if defined _POSIX_MONOTONIC_CLOCK && _POSIX_MONOTONIC_CLOCK >= 0
>  #  if _POSIX_MONOTONIC_CLOCK == 0
> @@ -189,8 +194,13 @@ do_test (void)
>        res = 1;
>      }
>    else
> +    {
>  #  endif
> -    res |= run_test (CLOCK_MONOTONIC);
> +      res |= run_test (CLOCK_MONOTONIC, CLOCK_USE_ATTR_CLOCK);
> +      res |= run_test (CLOCK_REALTIME, CLOCK_MONOTONIC);
> +      res |= run_test (CLOCK_MONOTONIC, CLOCK_MONOTONIC);
> +      res |= run_test (CLOCK_MONOTONIC, CLOCK_REALTIME);
> +    }
>  # else
>    puts ("_POSIX_MONOTONIC_CLOCK not defined");
>  # endif
> diff --git a/nptl/tst-cond26.c b/nptl/tst-cond26.c
> new file mode 100644
> index 0000000..83b041d
> --- /dev/null
> +++ b/nptl/tst-cond26.c
> @@ -0,0 +1,91 @@
> +/* Test unsupported/bad clocks passed to pthread_cond_clockwait.
> +
> +   Copyright (C) 2019 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
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <errno.h>
> +#include <pthread.h>
> +#include <stdio.h>
> +#include <time.h>
> +#include <unistd.h>
> +
> +static pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
> +static pthread_mutex_t mut = PTHREAD_MUTEX_INITIALIZER;
> +
> +static int test_bad_clockid (clockid_t clockid)
> +{
> +  struct timespec ts = {0,0};
> +  int rc = pthread_cond_clockwait (&cond, &mut, clockid, &ts);

Use TEST_VERIFY (pthread_cond_clockwait (&cond, &mut, clockid, &ts) != 0).

> +
> +  if (rc == 0)
> +    {
> +      puts ("pthread_cond_clockwait on bad clock incorrectly succeeded\n");
> +      return 1;
> +    }

Use TEST_VERIFY (rc == 0);

> +  else if (rc != EINVAL)

Use TEST_COMPARE (rc, EINVAL).

> +    {
> +      printf ("pthread_cond_clockwait on bad clock incorrectly failed "
> +	      "with %d\n", rc);
> +      return 1;
> +    }
> +  else
> +    return 0;
> +}
> +
> +#define NOT_A_VALID_CLOCK 123456
> +
> +static int
> +do_test (void)
> +{
> +  if (pthread_mutex_lock (&mut) != 0)
> +    {
> +      puts("Failed to lock mutex\n");
> +      return 1;
> +    }

Use xpthread_mutex_lock.

> +
> +  int rc = 0;
> +
> +  /* These clocks are meaningless to pthread_cond_clockwait. */
> +#if defined(CLOCK_PROCESS_CPUTIME_ID)
> +  rc |= test_bad_clockid (CLOCK_PROCESS_CPUTIME_ID);
> +#endif
> +#if defined(CLOCK_THREAD_CPUTIME_ID)
> +  rc |= test_bad_clockid (CLOCK_PROCESS_CPUTIME_ID);
> +#endif
> +
> +  /* These clocks might be meaningful, but are currently unsupported
> +     by pthread_cond_clockwait. */
> +#if defined(CLOCK_REALTIME_COARSE)
> +  rc |= test_bad_clockid (CLOCK_REALTIME_COARSE);
> +#endif
> +#if defined(CLOCK_MONOTONIC_RAW)
> +  rc |= test_bad_clockid (CLOCK_MONOTONIC_RAW);
> +#endif
> +#if defined(CLOCK_MONOTONIC_COARSE)
> +  rc |= test_bad_clockid (CLOCK_MONOTONIC_COARSE);
> +#endif
> +#if defined(CLOCK_BOOTTIME)
> +  rc |= test_bad_clockid (CLOCK_BOOTTIME);
> +#endif
> +
> +  /* This is a completely invalid clock. */
> +  rc |= test_bad_clockid (NOT_A_VALID_CLOCK);
> +
> +  return rc;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/nptl/tst-cond27.c b/nptl/tst-cond27.c
> new file mode 100644
> index 0000000..efca7fd
> --- /dev/null
> +++ b/nptl/tst-cond27.c
> @@ -0,0 +1,113 @@
> +/* Test pthread_cond_clockwait, based on tst-cond5.c
> +
> +   Copyright (C) 2019 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
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <errno.h>
> +#include <pthread.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <time.h>
> +#include <sys/time.h>
> +
> +
> +static pthread_mutex_t mut;
> +static pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
> +
> +
> +static int
> +do_test_clock (clockid_t clockid)
> +{
> +  pthread_mutexattr_t ma;
> +  int err;
> +  struct timespec ts;
> +
> +  if (pthread_mutexattr_init (&ma) != 0)
> +    {
> +      puts ("mutexattr_init failed");
> +      exit (1);
> +    }

Use xpthread_mutexattr_init.

> +
> +  if (pthread_mutexattr_settype (&ma, PTHREAD_MUTEX_ERRORCHECK) != 0)
> +    {
> +      puts ("mutexattr_settype failed");
> +      exit (1);
> +    }
> +

Use xpthread_mutexattr_settype.

> +  if (pthread_mutex_init (&mut, &ma) != 0)
> +    {
> +      puts ("mutex_init failed");
> +      exit (1);
> +    }
> +

Use xpthread_mutex_init.

> +  /* Get the mutex.  */
> +  if (pthread_mutex_lock (&mut) != 0)
> +    {
> +      puts ("mutex_lock failed");
> +      exit (1);
> +    }

Use xpthread_mutex_lock.

> +
> +  /* Waiting for the condition will fail.  But we want the timeout here.  */
> +  if (clock_gettime (clockid, &ts) != 0)
> +    {
> +      puts ("clock_gettime failed");
> +      exit (1);
> +    }

Use TEST_COMPARE (clock_gettime (clockid, &ts), 0);

> +
> +  ts.tv_nsec += 500000000;
> +  if (ts.tv_nsec >= 1000000000)
> +    {
> +      ts.tv_nsec -= 1000000000;
> +      ++ts.tv_sec;
> +    }
> +  err = pthread_cond_clockwait (&cond, &mut, clockid, &ts);
> +  if (err == 0)

Use TEST_VERIFY_EXIT (pthread_cond_clockwait (&cond, &mut, clockid, &ts) != 0);

> +    {
> +      /* This could in theory happen but here without any signal and
> +	 additional waiter it should not.  */
> +      puts ("cond_timedwait succeeded");
> +      exit (1);
> +    }
> +  else if (err != ETIMEDOUT)

Use TEST_COMPARE (err, ETIMEDOUT).

> +    {
> +      printf ("cond_timedwait returned with %s\n", strerror (err));
> +      exit (1);
> +    }
> +
> +  err = pthread_mutex_unlock (&mut);

Use xpthread_mutex_unlock.

> +  if (err != 0)
> +    {
> +      printf ("mutex_unlock failed: %s\n", strerror (err));
> +      exit (1);
> +    }
> +
> +  return 0;
> +}
> +
> +static int
> +do_test (void)
> +{
> +  int rc;
> +  rc = do_test_clock (CLOCK_MONOTONIC);
> +  if (rc == 0)
> +    rc = do_test_clock (CLOCK_REALTIME);
> +
> +  return rc;
> +}

Libsupport will handle the correct return, so there is no need to keep
track of current status.

> +
> +#include <support/test-driver.c>
> diff --git a/sysdeps/nptl/pthread-functions.h b/sysdeps/nptl/pthread-functions.h
> index cd5e94d..cfa9660 100644
> --- a/sysdeps/nptl/pthread-functions.h
> +++ b/sysdeps/nptl/pthread-functions.h
> @@ -55,6 +55,10 @@ struct pthread_functions
>    int (*ptr___pthread_cond_wait) (pthread_cond_t *, pthread_mutex_t *);
>    int (*ptr___pthread_cond_timedwait) (pthread_cond_t *, pthread_mutex_t *,
>  				       const struct timespec *);
> +  int (*ptr___pthread_cond_clockwait) (pthread_cond_t *,
> +				       pthread_mutex_t *,
> +				       clockid_t,
> +				       const struct timespec *);
>    int (*ptr___pthread_cond_broadcast_2_0) (pthread_cond_2_0_t *);
>    int (*ptr___pthread_cond_destroy_2_0) (pthread_cond_2_0_t *);
>    int (*ptr___pthread_cond_init_2_0) (pthread_cond_2_0_t *,
> diff --git a/sysdeps/nptl/pthread.h b/sysdeps/nptl/pthread.h
> index 704a3c4..d4fe9d9 100644
> --- a/sysdeps/nptl/pthread.h
> +++ b/sysdeps/nptl/pthread.h
> @@ -1003,6 +1003,19 @@ extern int pthread_cond_timedwait (pthread_cond_t *__restrict __cond,
>  				   const struct timespec *__restrict __abstime)
>       __nonnull ((1, 2, 3));
>  
> +/* Wait for condition variable COND to be signaled or broadcast until
> +   ABSTIME measured by the specified clock. MUTEX is assumed to be
> +   locked before. CLOCK is the clock to use. ABSTIME is an absolute
> +   time specification against CLOCK's epoch.
> +
> +   This function is a cancellation point and therefore not marked with
> +   __THROW. */
> +extern int pthread_cond_clockwait (pthread_cond_t *__restrict __cond,
> +				   pthread_mutex_t *__restrict __mutex,
> +				   __clockid_t __clock_id,
> +				   const struct timespec *__restrict __abstime)
> +     __nonnull ((1, 2, 4));
> +
>  /* Functions for handling condition variable attributes.  */
>  
>  /* Initialize condition variable attribute ATTR.  */
> diff --git a/sysdeps/unix/sysv/linux/x86_64/64/libpthread.abilist b/sysdeps/unix/sysv/linux/x86_64/64/libpthread.abilist
> index 454d340..aaa1c3b 100644
> --- a/sysdeps/unix/sysv/linux/x86_64/64/libpthread.abilist
> +++ b/sysdeps/unix/sysv/linux/x86_64/64/libpthread.abilist
> @@ -245,6 +245,7 @@ GLIBC_2.3.4 pthread_attr_setaffinity_np F
>  GLIBC_2.3.4 pthread_getaffinity_np F
>  GLIBC_2.3.4 pthread_setaffinity_np F
>  GLIBC_2.3.4 pthread_setschedprio F
> +GLIBC_2.30 pthread_cond_clockwait F
>  GLIBC_2.30 sem_clockwait F
>  GLIBC_2.4 pthread_mutex_consistent_np F
>  GLIBC_2.4 pthread_mutex_getprioceiling F
>
Mike Crowe May 4, 2019, 8:22 p.m. UTC | #2
On Tuesday 05 March 2019 at 13:45:13 -0300, Adhemerval Zanella wrote:
> 
> 
> On 27/02/2019 15:23, Mike Crowe wrote:
> > Add:
> > 
> >  int pthread_cond_clockwait (pthread_cond_t *cond,
> >                              pthread_mutex_t *mutex,
> >                              clockid_t clockid,
> >                              const struct timespec *abstime)
> > 
> > which behaves just like pthread_cond_timedwait except it always measures
> > abstime against the supplied clockid. Currently supports CLOCK_REALTIME and
> > CLOCK_MONOTONIC and returns EINVAL if any other clock is specified.
> > 
> > Includes feedback from many others. This function was originally
> > proposed[1] as pthread_cond_timedwaitonclock_np, but The Austin Group
> > preferred the new name.
> > 
> > * nptl/Makefile: Add tst-cond26 and tst-cond27
> > 
> > * nptl/Versions (GLIBC_2.30): Add pthread_cond_clockwait
> > 
> > * sysdeps/nptl/pthread.h: Likewise
> > 
> > * nptl/forward.c: Add __pthread_cond_clockwait (not sure if it should be
> >   __USE_GNU while it's still only proposed for POSIX.)
> > 
> > * nptl/forward.c: Likewise
> > 
> > * nptl/pthreadP.h: Likewise
> > 
> > * sysdeps/nptl/pthread-functions.h: Likewise.
> > 
> > * nptl/pthread_cond_wait.c (__pthread_cond_wait_common): Add clockid
> >   parameter and comment describing why we don't need to check its value.
> >   Use that value rather than reading the clock from the flags.
> >   (__pthread_cond_wait): Pass unused clockid parameter.
> >   (__pthread_cond_timedwait): Read clock from flags and pass it to
> >   __pthread_cond_wait_common. (__pthread_cond_clockwait): Add new function
> >   with weak alias from pthread_cond_clockwait.
> > 
> > * nptl/tst-cond11.c (run_test): Support testing pthread_cond_clockwait too
> >   by using a special magic CLOCK_USE_ATTR_CLOCK value to determine whether
> >   to call pthread_cond_timedwait or pthread_cond_clockwait. (do_test): Pass
> >   CLOCK_USE_ATTR_CLOCK for existing tests, and add new tests using all
> >   combinations of CLOCK_MONOTONIC and CLOCK_REALTIME.
> > 
> > * ntpl/tst-cond26.c: New test for passing unsupported and invalid clocks to
> >   pthread_cond_clockwait.
> > 
> > * nptl/tst-cond27.c: Add test similar to tst-cond5.c, but using struct
> >   timespec and pthread_cond_clockwait.
> > 
> > * sysdeps/unix/sysv/linux/arm/libpthread.abilist,
> >  sysdeps/unix/sysv/linux/i386/libpthread.abilist,
> >  sysdeps/unix/sysv/linux/x86_64/64/libpthread.abilist: Add
> >  pthread_cond_clockwait
> > 
> > * manual/threads.texi: Document pthread_cond_clockwait. The comment was
> >   provided by Carlos O'Donell.
> > 
> > [1] https://sourceware.org/ml/libc-alpha/2015-07/msg00193.html
> > ---
> >  manual/threads.texi                                  |  20 ++-
> >  nptl/Makefile                                        |   1 +-
> >  nptl/Versions                                        |   2 +-
> >  nptl/forward.c                                       |   5 +-
> >  nptl/nptl-init.c                                     |   1 +-
> >  nptl/pthreadP.h                                      |   4 +-
> >  nptl/pthread_cond_wait.c                             |  43 ++++-
> >  nptl/tst-cond11.c                                    |  30 ++-
> >  nptl/tst-cond26.c                                    |  91 ++++++++++-
> >  nptl/tst-cond27.c                                    | 113 ++++++++++++-
> >  sysdeps/nptl/pthread-functions.h                     |   4 +-
> >  sysdeps/nptl/pthread.h                               |  13 +-
> >  sysdeps/unix/sysv/linux/x86_64/64/libpthread.abilist |   1 +-
> >  13 files changed, 314 insertions(+), 14 deletions(-)
> >  create mode 100644 nptl/tst-cond26.c
> >  create mode 100644 nptl/tst-cond27.c

[snip]

> > diff --git a/nptl/pthread_cond_wait.c b/nptl/pthread_cond_wait.c
> > index daa4e25..5deb54b 100644
> > --- a/nptl/pthread_cond_wait.c
> > +++ b/nptl/pthread_cond_wait.c

[snip]

> > @@ -664,10 +671,40 @@ __pthread_cond_timedwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
> >       it can assume that abstime is not NULL.  */
> >    if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
> >      return EINVAL;
> > -  return __pthread_cond_wait_common (cond, mutex, abstime);
> > +
> > +  /* Relaxed MO is suffice because clock ID bit is only modified
> > +     in condition creation.  */
> > +  unsigned int flags = atomic_load_relaxed (&cond->__data.__wrefs);
> > +  clockid_t clockid = (flags & __PTHREAD_COND_CLOCK_MONOTONIC_MASK)
> > +                    ? CLOCK_MONOTONIC : CLOCK_REALTIME;
> > +  return __pthread_cond_wait_common (cond, mutex, clockid, abstime);
> > +}
> > +
> 
> Ok.
> 
> > +/* See __pthread_cond_wait_common.  */
> > +int
> > +__pthread_cond_clockwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
> > +			  clockid_t clockid,
> > +			  const struct timespec *abstime)
> > +{
> > +  /* Check parameter validity.  This should also tell the compiler that
> > +     it can assume that abstime is not NULL.  */
> > +  if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
> > +    return EINVAL;
> 
> The timespec check is used in different parts internally, I wonder
> if it would better to consolidate it somewhere.

Yes, I think some sort of timespec_valid function would make sense. Where
would be a good place to put it?

> > +
> > +  /* We only support CLOCK_REALTIME and CLOCK_MONOTONIC */
> > +  if (clockid != CLOCK_REALTIME && clockid != CLOCK_MONOTONIC)
> > +    return EINVAL;
> 
> Why not use lll_futex_supported_clockid here?

In fact, it should be futex_abstimed_supported_clockid.

> > +
> > +  /* If we do not support waiting using CLOCK_MONOTONIC, return an error.  */
> > +  if (clockid == CLOCK_MONOTONIC
> > +      && !futex_supports_exact_relative_timeouts ())
> > +    return EINVAL;
> 
> Why exactly do we need futex_supports_exact_relative_timeouts if Linux
> always set it to true?

I copied the use of it from the implementation of
pthread_condattr_setclock. It's described in sysdeps/nptl/futex-internal.h,
but there's no definition of it for anything other than Linux. It seems
quite possible that it has no use at all.

I can try removing it from pthread_condattr_setclock, but that would
probably be better as a separate change.

Mike.

Patch
diff mbox series

diff --git a/manual/threads.texi b/manual/threads.texi
index 674267c..91462f5 100644
--- a/manual/threads.texi
+++ b/manual/threads.texi
@@ -679,6 +679,26 @@  against the clock specified by @var{clockid} rather than
 @code{CLOCK_MONOTONIC} or @code{CLOCK_REALTIME}.
 @end deftypefun
 
+@comment pthread.h
+@comment POSIX-proposed
+@deftypefun int pthread_cond_clockwait (pthread_cond_t *@var{cond}, pthread_mutex_t *@var{mutex},
+                                        clockid_t @var{clockid}, const struct timespec *@var{abstime})
+@safety{@prelim{}@mtsafe{}@asunsafe{@asulock{}}@acunsafe{@aculock{}}}
+@c If exactly the same function with arguments is called from a signal
+@c handler that interrupts between the mutex unlock and sleep then it
+@c will unlock the mutex twice resulting in undefined behaviour. Keep
+@c in mind that the unlock and sleep are only atomic with respect to other
+@c threads (really a happens-after relationship for pthread_cond_broadcast
+@c and pthread_cond_signal).
+@c In the AC case we would cancel the thread and the mutex would remain
+@c locked and we can't recover from that.
+Behaves like @code{pthread_cond_timedwait} except the time @var{abstime} is
+measured against the clock specified by @var{clockid} rather than the clock
+specified or defaulted when @code{pthread_cond_init} was called. Currently,
+@var{clockid} must be either @code{CLOCK_MONOTONIC} or
+@code{CLOCK_REALTIME}.
+@end deftypefun
+
 @c FIXME these are undocumented:
 @c pthread_atfork
 @c pthread_attr_destroy
diff --git a/nptl/Makefile b/nptl/Makefile
index 4c9f5d3..7de4d40 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -251,6 +251,7 @@  tests = tst-attr1 tst-attr2 tst-attr3 tst-default-attr \
 	tst-cond8 tst-cond9 tst-cond10 tst-cond11 tst-cond12 tst-cond13 \
 	tst-cond14 tst-cond15 tst-cond16 tst-cond17 tst-cond18 tst-cond19 \
 	tst-cond20 tst-cond21 tst-cond22 tst-cond23 tst-cond24 tst-cond25 \
+	tst-cond26 tst-cond27 \
 	tst-cond-except \
 	tst-robust1 tst-robust2 tst-robust3 tst-robust4 tst-robust5 \
 	tst-robust6 tst-robust7 tst-robust8 tst-robust9 \
diff --git a/nptl/Versions b/nptl/Versions
index cd1806c..8c094d0 100644
--- a/nptl/Versions
+++ b/nptl/Versions
@@ -278,7 +278,7 @@  libpthread {
   }
 
   GLIBC_2.30 {
-    sem_clockwait;
+    sem_clockwait; pthread_cond_clockwait;
   }
 
   GLIBC_PRIVATE {
diff --git a/nptl/forward.c b/nptl/forward.c
index ed1e7d0..50f358f 100644
--- a/nptl/forward.c
+++ b/nptl/forward.c
@@ -164,6 +164,11 @@  FORWARD (__pthread_cond_timedwait,
 	  const struct timespec *abstime), (cond, mutex, abstime), 0)
 versioned_symbol (libc, __pthread_cond_timedwait, pthread_cond_timedwait,
 		  GLIBC_2_3_2);
+FORWARD (__pthread_cond_clockwait,
+	 (pthread_cond_t *cond, pthread_mutex_t *mutex, clockid_t clockid,
+	  const struct timespec *abstime), (cond, mutex, clockid, abstime),
+	 0)
+weak_alias (__pthread_cond_clockwait, pthread_cond_clockwait);
 
 
 FORWARD (pthread_equal, (pthread_t thread1, pthread_t thread2),
diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c
index b5895fa..c872b9f 100644
--- a/nptl/nptl-init.c
+++ b/nptl/nptl-init.c
@@ -104,6 +104,7 @@  static const struct pthread_functions pthread_functions =
     .ptr___pthread_cond_signal = __pthread_cond_signal,
     .ptr___pthread_cond_wait = __pthread_cond_wait,
     .ptr___pthread_cond_timedwait = __pthread_cond_timedwait,
+    .ptr___pthread_cond_clockwait = __pthread_cond_clockwait,
 # if SHLIB_COMPAT(libpthread, GLIBC_2_0, GLIBC_2_3_2)
     .ptr___pthread_cond_broadcast_2_0 = __pthread_cond_broadcast_2_0,
     .ptr___pthread_cond_destroy_2_0 = __pthread_cond_destroy_2_0,
diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
index 626bd4b..28c0ee5 100644
--- a/nptl/pthreadP.h
+++ b/nptl/pthreadP.h
@@ -487,6 +487,10 @@  extern int __pthread_cond_wait (pthread_cond_t *cond, pthread_mutex_t *mutex);
 extern int __pthread_cond_timedwait (pthread_cond_t *cond,
 				     pthread_mutex_t *mutex,
 				     const struct timespec *abstime);
+extern int __pthread_cond_clockwait (pthread_cond_t *cond,
+				     pthread_mutex_t *mutex,
+				     clockid_t clockid,
+				     const struct timespec *abstime);
 extern int __pthread_condattr_destroy (pthread_condattr_t *attr);
 extern int __pthread_condattr_init (pthread_condattr_t *attr);
 extern int __pthread_key_create (pthread_key_t *key, void (*destr) (void *));
diff --git a/nptl/pthread_cond_wait.c b/nptl/pthread_cond_wait.c
index daa4e25..5deb54b 100644
--- a/nptl/pthread_cond_wait.c
+++ b/nptl/pthread_cond_wait.c
@@ -378,6 +378,7 @@  __condvar_cleanup_waiting (void *arg)
 */
 static __always_inline int
 __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex,
+    clockid_t clockid,
     const struct timespec *abstime)
 {
   const int maxspin = 0;
@@ -386,6 +387,11 @@  __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex,
 
   LIBC_PROBE (cond_wait, 2, cond, mutex);
 
+  /* clockid will already have been checked by
+     __pthread_cond_clockwait or pthread_condattr_setclock, or we
+     don't use it if abstime is NULL, so we don't need to check it
+     here. */
+
   /* Acquire a position (SEQ) in the waiter sequence (WSEQ).  We use an
      atomic operation because signals and broadcasts may update the group
      switch without acquiring the mutex.  We do not need release MO here
@@ -510,7 +516,7 @@  __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex,
 	      if (__glibc_unlikely (abstime->tv_sec < 0))
 	        err = ETIMEDOUT;
 
-	      else if ((flags & __PTHREAD_COND_CLOCK_MONOTONIC_MASK) != 0)
+	      else if (clockid == CLOCK_MONOTONIC)
 		{
 		  /* CLOCK_MONOTONIC is requested.  */
 		  struct timespec rt;
@@ -652,7 +658,8 @@  __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex,
 int
 __pthread_cond_wait (pthread_cond_t *cond, pthread_mutex_t *mutex)
 {
-  return __pthread_cond_wait_common (cond, mutex, NULL);
+  /* clockid is unused when abstime is NULL. */
+  return __pthread_cond_wait_common (cond, mutex, 0, NULL);
 }
 
 /* See __pthread_cond_wait_common.  */
@@ -664,10 +671,40 @@  __pthread_cond_timedwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
      it can assume that abstime is not NULL.  */
   if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
     return EINVAL;
-  return __pthread_cond_wait_common (cond, mutex, abstime);
+
+  /* Relaxed MO is suffice because clock ID bit is only modified
+     in condition creation.  */
+  unsigned int flags = atomic_load_relaxed (&cond->__data.__wrefs);
+  clockid_t clockid = (flags & __PTHREAD_COND_CLOCK_MONOTONIC_MASK)
+                    ? CLOCK_MONOTONIC : CLOCK_REALTIME;
+  return __pthread_cond_wait_common (cond, mutex, clockid, abstime);
+}
+
+/* See __pthread_cond_wait_common.  */
+int
+__pthread_cond_clockwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
+			  clockid_t clockid,
+			  const struct timespec *abstime)
+{
+  /* Check parameter validity.  This should also tell the compiler that
+     it can assume that abstime is not NULL.  */
+  if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
+    return EINVAL;
+
+  /* We only support CLOCK_REALTIME and CLOCK_MONOTONIC */
+  if (clockid != CLOCK_REALTIME && clockid != CLOCK_MONOTONIC)
+    return EINVAL;
+
+  /* If we do not support waiting using CLOCK_MONOTONIC, return an error.  */
+  if (clockid == CLOCK_MONOTONIC
+      && !futex_supports_exact_relative_timeouts ())
+    return EINVAL;
+
+  return __pthread_cond_wait_common (cond, mutex, clockid, abstime);
 }
 
 versioned_symbol (libpthread, __pthread_cond_wait, pthread_cond_wait,
 		  GLIBC_2_3_2);
 versioned_symbol (libpthread, __pthread_cond_timedwait, pthread_cond_timedwait,
 		  GLIBC_2_3_2);
+weak_alias (__pthread_cond_clockwait, pthread_cond_clockwait);
diff --git a/nptl/tst-cond11.c b/nptl/tst-cond11.c
index 97a8bd0..4df6b15 100644
--- a/nptl/tst-cond11.c
+++ b/nptl/tst-cond11.c
@@ -22,17 +22,20 @@ 
 #include <time.h>
 #include <unistd.h>
 
+/* A bogus clock value that tells run_test to use
+   pthread_cond_timedwait rather than pthread_condclockwait. */
+#define CLOCK_USE_ATTR_CLOCK (-1)
 
 #if defined _POSIX_CLOCK_SELECTION && _POSIX_CLOCK_SELECTION >= 0
 static int
-run_test (clockid_t cl)
+run_test (clockid_t attr_clock, clockid_t wait_clock)
 {
   pthread_condattr_t condattr;
   pthread_cond_t cond;
   pthread_mutexattr_t mutattr;
   pthread_mutex_t mut;
 
-  printf ("clock = %d\n", (int) cl);
+  printf ("attr_clock = %d\n", (int) attr_clock);
 
   if (pthread_condattr_init (&condattr) != 0)
     {
@@ -40,7 +43,7 @@  run_test (clockid_t cl)
       return 1;
     }
 
-  if (pthread_condattr_setclock (&condattr, cl) != 0)
+  if (pthread_condattr_setclock (&condattr, attr_clock) != 0)
     {
       puts ("condattr_setclock failed");
       return 1;
@@ -52,10 +55,10 @@  run_test (clockid_t cl)
       puts ("condattr_getclock failed");
       return 1;
     }
-  if (cl != cl2)
+  if (attr_clock != cl2)
     {
       printf ("condattr_getclock returned wrong value: %d, expected %d\n",
-	      (int) cl2, (int) cl);
+	      (int) cl2, (int) attr_clock);
       return 1;
     }
 
@@ -108,7 +111,7 @@  run_test (clockid_t cl)
     }
 
   struct timespec ts;
-  if (clock_gettime (cl, &ts) != 0)
+  if (clock_gettime ((wait_clock == CLOCK_USE_ATTR_CLOCK) ? attr_clock : wait_clock, &ts) != 0)
     {
       puts ("clock_gettime failed");
       return 1;
@@ -117,7 +120,9 @@  run_test (clockid_t cl)
   /* Wait one second.  */
   ++ts.tv_sec;
 
-  int e = pthread_cond_timedwait (&cond, &mut, &ts);
+  int e = (wait_clock == CLOCK_USE_ATTR_CLOCK)
+    ? pthread_cond_timedwait (&cond, &mut, &ts)
+    : pthread_cond_clockwait (&cond, &mut, wait_clock, &ts);
   if (e == 0)
     {
       puts ("cond_timedwait succeeded");
@@ -130,7 +135,7 @@  run_test (clockid_t cl)
     }
 
   struct timespec ts2;
-  if (clock_gettime (cl, &ts2) != 0)
+  if (clock_gettime ((wait_clock == CLOCK_USE_ATTR_CLOCK) ? attr_clock : wait_clock, &ts2) != 0)
     {
       puts ("second clock_gettime failed");
       return 1;
@@ -176,7 +181,7 @@  do_test (void)
 
 #else
 
-  int res = run_test (CLOCK_REALTIME);
+  int res = run_test (CLOCK_REALTIME, CLOCK_USE_ATTR_CLOCK);
 
 # if defined _POSIX_MONOTONIC_CLOCK && _POSIX_MONOTONIC_CLOCK >= 0
 #  if _POSIX_MONOTONIC_CLOCK == 0
@@ -189,8 +194,13 @@  do_test (void)
       res = 1;
     }
   else
+    {
 #  endif
-    res |= run_test (CLOCK_MONOTONIC);
+      res |= run_test (CLOCK_MONOTONIC, CLOCK_USE_ATTR_CLOCK);
+      res |= run_test (CLOCK_REALTIME, CLOCK_MONOTONIC);
+      res |= run_test (CLOCK_MONOTONIC, CLOCK_MONOTONIC);
+      res |= run_test (CLOCK_MONOTONIC, CLOCK_REALTIME);
+    }
 # else
   puts ("_POSIX_MONOTONIC_CLOCK not defined");
 # endif
diff --git a/nptl/tst-cond26.c b/nptl/tst-cond26.c
new file mode 100644
index 0000000..83b041d
--- /dev/null
+++ b/nptl/tst-cond26.c
@@ -0,0 +1,91 @@ 
+/* Test unsupported/bad clocks passed to pthread_cond_clockwait.
+
+   Copyright (C) 2019 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <errno.h>
+#include <pthread.h>
+#include <stdio.h>
+#include <time.h>
+#include <unistd.h>
+
+static pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
+static pthread_mutex_t mut = PTHREAD_MUTEX_INITIALIZER;
+
+static int test_bad_clockid (clockid_t clockid)
+{
+  struct timespec ts = {0,0};
+  int rc = pthread_cond_clockwait (&cond, &mut, clockid, &ts);
+
+  if (rc == 0)
+    {
+      puts ("pthread_cond_clockwait on bad clock incorrectly succeeded\n");
+      return 1;
+    }
+  else if (rc != EINVAL)
+    {
+      printf ("pthread_cond_clockwait on bad clock incorrectly failed "
+	      "with %d\n", rc);
+      return 1;
+    }
+  else
+    return 0;
+}
+
+#define NOT_A_VALID_CLOCK 123456
+
+static int
+do_test (void)
+{
+  if (pthread_mutex_lock (&mut) != 0)
+    {
+      puts("Failed to lock mutex\n");
+      return 1;
+    }
+
+  int rc = 0;
+
+  /* These clocks are meaningless to pthread_cond_clockwait. */
+#if defined(CLOCK_PROCESS_CPUTIME_ID)
+  rc |= test_bad_clockid (CLOCK_PROCESS_CPUTIME_ID);
+#endif
+#if defined(CLOCK_THREAD_CPUTIME_ID)
+  rc |= test_bad_clockid (CLOCK_PROCESS_CPUTIME_ID);
+#endif
+
+  /* These clocks might be meaningful, but are currently unsupported
+     by pthread_cond_clockwait. */
+#if defined(CLOCK_REALTIME_COARSE)
+  rc |= test_bad_clockid (CLOCK_REALTIME_COARSE);
+#endif
+#if defined(CLOCK_MONOTONIC_RAW)
+  rc |= test_bad_clockid (CLOCK_MONOTONIC_RAW);
+#endif
+#if defined(CLOCK_MONOTONIC_COARSE)
+  rc |= test_bad_clockid (CLOCK_MONOTONIC_COARSE);
+#endif
+#if defined(CLOCK_BOOTTIME)
+  rc |= test_bad_clockid (CLOCK_BOOTTIME);
+#endif
+
+  /* This is a completely invalid clock. */
+  rc |= test_bad_clockid (NOT_A_VALID_CLOCK);
+
+  return rc;
+}
+
+#include <support/test-driver.c>
diff --git a/nptl/tst-cond27.c b/nptl/tst-cond27.c
new file mode 100644
index 0000000..efca7fd
--- /dev/null
+++ b/nptl/tst-cond27.c
@@ -0,0 +1,113 @@ 
+/* Test pthread_cond_clockwait, based on tst-cond5.c
+
+   Copyright (C) 2019 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <errno.h>
+#include <pthread.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <time.h>
+#include <sys/time.h>
+
+
+static pthread_mutex_t mut;
+static pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
+
+
+static int
+do_test_clock (clockid_t clockid)
+{
+  pthread_mutexattr_t ma;
+  int err;
+  struct timespec ts;
+
+  if (pthread_mutexattr_init (&ma) != 0)
+    {
+      puts ("mutexattr_init failed");
+      exit (1);
+    }
+
+  if (pthread_mutexattr_settype (&ma, PTHREAD_MUTEX_ERRORCHECK) != 0)
+    {
+      puts ("mutexattr_settype failed");
+      exit (1);
+    }
+
+  if (pthread_mutex_init (&mut, &ma) != 0)
+    {
+      puts ("mutex_init failed");
+      exit (1);
+    }
+
+  /* Get the mutex.  */
+  if (pthread_mutex_lock (&mut) != 0)
+    {
+      puts ("mutex_lock failed");
+      exit (1);
+    }
+
+  /* Waiting for the condition will fail.  But we want the timeout here.  */
+  if (clock_gettime (clockid, &ts) != 0)
+    {
+      puts ("clock_gettime failed");
+      exit (1);
+    }
+
+  ts.tv_nsec += 500000000;
+  if (ts.tv_nsec >= 1000000000)
+    {
+      ts.tv_nsec -= 1000000000;
+      ++ts.tv_sec;
+    }
+  err = pthread_cond_clockwait (&cond, &mut, clockid, &ts);
+  if (err == 0)
+    {
+      /* This could in theory happen but here without any signal and
+	 additional waiter it should not.  */
+      puts ("cond_timedwait succeeded");
+      exit (1);
+    }
+  else if (err != ETIMEDOUT)
+    {
+      printf ("cond_timedwait returned with %s\n", strerror (err));
+      exit (1);
+    }
+
+  err = pthread_mutex_unlock (&mut);
+  if (err != 0)
+    {
+      printf ("mutex_unlock failed: %s\n", strerror (err));
+      exit (1);
+    }
+
+  return 0;
+}
+
+static int
+do_test (void)
+{
+  int rc;
+  rc = do_test_clock (CLOCK_MONOTONIC);
+  if (rc == 0)
+    rc = do_test_clock (CLOCK_REALTIME);
+
+  return rc;
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/nptl/pthread-functions.h b/sysdeps/nptl/pthread-functions.h
index cd5e94d..cfa9660 100644
--- a/sysdeps/nptl/pthread-functions.h
+++ b/sysdeps/nptl/pthread-functions.h
@@ -55,6 +55,10 @@  struct pthread_functions
   int (*ptr___pthread_cond_wait) (pthread_cond_t *, pthread_mutex_t *);
   int (*ptr___pthread_cond_timedwait) (pthread_cond_t *, pthread_mutex_t *,
 				       const struct timespec *);
+  int (*ptr___pthread_cond_clockwait) (pthread_cond_t *,
+				       pthread_mutex_t *,
+				       clockid_t,
+				       const struct timespec *);
   int (*ptr___pthread_cond_broadcast_2_0) (pthread_cond_2_0_t *);
   int (*ptr___pthread_cond_destroy_2_0) (pthread_cond_2_0_t *);
   int (*ptr___pthread_cond_init_2_0) (pthread_cond_2_0_t *,
diff --git a/sysdeps/nptl/pthread.h b/sysdeps/nptl/pthread.h
index 704a3c4..d4fe9d9 100644
--- a/sysdeps/nptl/pthread.h
+++ b/sysdeps/nptl/pthread.h
@@ -1003,6 +1003,19 @@  extern int pthread_cond_timedwait (pthread_cond_t *__restrict __cond,
 				   const struct timespec *__restrict __abstime)
      __nonnull ((1, 2, 3));
 
+/* Wait for condition variable COND to be signaled or broadcast until
+   ABSTIME measured by the specified clock. MUTEX is assumed to be
+   locked before. CLOCK is the clock to use. ABSTIME is an absolute
+   time specification against CLOCK's epoch.
+
+   This function is a cancellation point and therefore not marked with
+   __THROW. */
+extern int pthread_cond_clockwait (pthread_cond_t *__restrict __cond,
+				   pthread_mutex_t *__restrict __mutex,
+				   __clockid_t __clock_id,
+				   const struct timespec *__restrict __abstime)
+     __nonnull ((1, 2, 4));
+
 /* Functions for handling condition variable attributes.  */
 
 /* Initialize condition variable attribute ATTR.  */
diff --git a/sysdeps/unix/sysv/linux/x86_64/64/libpthread.abilist b/sysdeps/unix/sysv/linux/x86_64/64/libpthread.abilist
index 454d340..aaa1c3b 100644
--- a/sysdeps/unix/sysv/linux/x86_64/64/libpthread.abilist
+++ b/sysdeps/unix/sysv/linux/x86_64/64/libpthread.abilist
@@ -245,6 +245,7 @@  GLIBC_2.3.4 pthread_attr_setaffinity_np F
 GLIBC_2.3.4 pthread_getaffinity_np F
 GLIBC_2.3.4 pthread_setaffinity_np F
 GLIBC_2.3.4 pthread_setschedprio F
+GLIBC_2.30 pthread_cond_clockwait F
 GLIBC_2.30 sem_clockwait F
 GLIBC_2.4 pthread_mutex_consistent_np F
 GLIBC_2.4 pthread_mutex_getprioceiling F