Re: [PATCH v1 3/6] nptl: Use futex_lock_pi2()

[Adhemerval Zanella via Libc-alpha <libc-alpha@sourceware.org>]

On 25/06/2021 05:11, Kurt Kanzenbach wrote:
> Currently it is not possible to specify CLOCK_MONOTONIC for timeouts for PI
> mutexes. The FUTEX_LOCK_PI2 operation can be used to implement that.
> 
> Therefore, use it by default. In case FUTEX_LOCK_PI2 is not available on the
> running kernel, then EINVAL is returned as of now.
> 
> Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
> ---
>  nptl/pthread_mutex_timedlock.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
> index 5afd6222d61e..95892a32a6af 100644
> --- a/nptl/pthread_mutex_timedlock.c
> +++ b/nptl/pthread_mutex_timedlock.c
> @@ -300,13 +300,6 @@ __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
> @@ -370,7 +363,8 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
>  	    int private = (robust
>  			   ? PTHREAD_ROBUST_MUTEX_PSHARED (mutex)
>  			   : PTHREAD_MUTEX_PSHARED (mutex));
> -	    int e = futex_lock_pi64 (&mutex->__data.__lock, abstime, private);
> +	    int e = futex_lock_pi2_64 (&mutex->__data.__lock, clockid, abstime,
> +                                       private);
>  	    if (e == ETIMEDOUT)
>  	      return ETIMEDOUT;
>  	    else if (e == ESRCH || e == EDEADLK)
> 

I think we should change futex_lock_pi64() to use FUTEX_LOCK_PI2 and only
exports it instead of both futex_lock_pi64() and futex_lock_pi2_64.  It
can simplify the implementation and provide a complete internal futex
functionality (since by using two different PI-futex operation caller
should be aware that only futex_lock_pi2_64 supports FUTEX_LOCK_PI2).