[PATCH 09/13] linux: nptl: Replace lll_futex_{timed_}wait by futex-internal.h
Adhemerval Zanella
adhemerval.zanella@linaro.org
Wed Nov 25 15:40:46 GMT 2020
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,
More information about the Libc-alpha
mailing list