[PATCH RFC 0/3] nptl: Introduce and use FUTEX_LOCK_PI2

[Adhemerval Zanella]

On 22/06/2021 04:26, Kurt Kanzenbach wrote:
>>   #else /* __ASSUME_TIME64_SYSCALLS  */
>>     bool need_time64 = abstime != NULL && !in_time_t_range (abstime->tv_sec)
>>     if (need_time64)
>>       {
>>         err = futex_lock_pi2_64 (futex_word, abstime, private);
>>       }
>>     else
>>       {
>>         struct timespec ts32;
>>         if (abstime != NULL)
>>           ts32 = valid_timespec64_to_timespec (*abstime);
>>
>>         err = INTERNAL_SYSCALL_CALL (futex, futex_word, __lll_private_flag
>>                                      (FUTEX_LOCK_PI, private), 0,
>>                                      abstime != NULL ? &ts32 : NULL);
>>       }
>>   #endif /* __ASSUME_TIME64_SYSCALLS  */
>>    [...]
>>   }
>>
>> It would make the changes on pthread_mutex code minimal, it would be only to
>> remove the extra check for clockid and adjust the comment.
> 
> Well, that's an interesting point. I think the current check has to
> stay, because there are two locking paths. Only the slow path calls
> futex_lock_pi_64() which may result in ENOSYS for clock monotonic. But,
> the fast path which doesn't call futex_lock_pi64() would succeed if the
> check is removed.
> 
> Maybe something like this:
> 
> sysdeps/nptl/futex-internal.h:
> |static __always_inline bool
> |futex_lockpi2_supported (void)
> |{
> |  return __ASSUME_FUTEX_LOCK_PI2;
> |}
> 
> nptl/pthread_mutex_timedlock.c:
> |	if (__glibc_unlikely (! futex_lockpi2_supported () &&
> |			      clockid != CLOCK_REALTIME))
> |	  return EINVAL;
> 
> Or did I get something wrong?

Besides what Florian has explained about __ASSUME_* macros, another issue we
have static initialization for pthread_mutex.  So the syscall probe only
works for dynamic initialization (where caller issue a pthread_mutex_init).

I view this as an inconsistent behavior and I don't have a straightforward
solution that won't result in a performance penalization for common cases
(it would require to probe for FUTEX_LOCK_PI2 *and* FUTEX_LOCK_PI).

Instead I think we should move the possible error on the slow path and let
the kernel advertise it any missing support.