|
 |
00db10 |
commit 823624bdc47f1f80109c9c52dee7939b9386d708
|
|
|
00db10 |
Author: Stefan Liebler <stli@linux.ibm.com>
|
|
|
00db10 |
Date: Thu Feb 7 15:18:36 2019 +0100
|
|
|
00db10 |
|
|
|
00db10 |
Add compiler barriers around modifications of the robust mutex list for pthread_mutex_trylock. [BZ #24180]
|
|
|
00db10 |
|
|
|
00db10 |
While debugging a kernel warning, Thomas Gleixner, Sebastian Sewior and
|
|
|
00db10 |
Heiko Carstens found a bug in pthread_mutex_trylock due to misordered
|
|
|
00db10 |
instructions:
|
|
|
00db10 |
140: a5 1b 00 01 oill %r1,1
|
|
|
00db10 |
144: e5 48 a0 f0 00 00 mvghi 240(%r10),0 <--- THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL);
|
|
|
00db10 |
14a: e3 10 a0 e0 00 24 stg %r1,224(%r10) <--- last THREAD_SETMEM of ENQUEUE_MUTEX_PI
|
|
|
00db10 |
|
|
|
00db10 |
vs (with compiler barriers):
|
|
|
00db10 |
140: a5 1b 00 01 oill %r1,1
|
|
|
00db10 |
144: e3 10 a0 e0 00 24 stg %r1,224(%r10)
|
|
|
00db10 |
14a: e5 48 a0 f0 00 00 mvghi 240(%r10),0
|
|
|
00db10 |
|
|
|
00db10 |
Please have a look at the discussion:
|
|
|
00db10 |
"Re: WARN_ON_ONCE(!new_owner) within wake_futex_pi() triggerede"
|
|
|
00db10 |
(https://lore.kernel.org/lkml/20190202112006.GB3381@osiris/)
|
|
|
00db10 |
|
|
|
00db10 |
This patch is introducing the same compiler barriers and comments
|
|
|
00db10 |
for pthread_mutex_trylock as introduced for pthread_mutex_lock and
|
|
|
00db10 |
pthread_mutex_timedlock by commit 8f9450a0b7a9e78267e8ae1ab1000ebca08e473e
|
|
|
00db10 |
"Add compiler barriers around modifications of the robust mutex list."
|
|
|
00db10 |
|
|
|
00db10 |
ChangeLog:
|
|
|
00db10 |
|
|
|
00db10 |
[BZ #24180]
|
|
|
00db10 |
* nptl/pthread_mutex_trylock.c (__pthread_mutex_trylock):
|
|
|
00db10 |
|
|
|
00db10 |
|
|
|
00db10 |
diff -Nrup a/nptl/pthread_mutex_trylock.c b/nptl/pthread_mutex_trylock.c
|
|
|
00db10 |
--- a/nptl/pthread_mutex_trylock.c 2019-07-26 16:43:11.028271897 -0400
|
|
|
00db10 |
+++ b/nptl/pthread_mutex_trylock.c 2019-07-26 17:06:48.708748979 -0400
|
|
|
00db10 |
@@ -95,6 +95,9 @@ __pthread_mutex_trylock (pthread_mutex_t
|
|
|
00db10 |
case PTHREAD_MUTEX_ROBUST_ADAPTIVE_NP:
|
|
|
00db10 |
THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending,
|
|
|
00db10 |
&mutex->__data.__list.__next);
|
|
|
00db10 |
+ /* We need to set op_pending before starting the operation. Also
|
|
|
00db10 |
+ see comments at ENQUEUE_MUTEX. */
|
|
|
00db10 |
+ __asm ("" ::: "memory");
|
|
|
00db10 |
|
|
|
00db10 |
oldval = mutex->__data.__lock;
|
|
|
00db10 |
do
|
|
|
00db10 |
@@ -120,7 +123,12 @@ __pthread_mutex_trylock (pthread_mutex_t
|
|
|
00db10 |
/* But it is inconsistent unless marked otherwise. */
|
|
|
00db10 |
mutex->__data.__owner = PTHREAD_MUTEX_INCONSISTENT;
|
|
|
00db10 |
|
|
|
00db10 |
+ /* We must not enqueue the mutex before we have acquired it.
|
|
|
00db10 |
+ Also see comments at ENQUEUE_MUTEX. */
|
|
|
00db10 |
+ __asm ("" ::: "memory");
|
|
|
00db10 |
ENQUEUE_MUTEX (mutex);
|
|
|
00db10 |
+ /* We need to clear op_pending after we enqueue the mutex. */
|
|
|
00db10 |
+ __asm ("" ::: "memory");
|
|
|
00db10 |
THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL);
|
|
|
00db10 |
|
|
|
00db10 |
/* Note that we deliberately exist here. If we fall
|
|
|
00db10 |
@@ -136,6 +144,8 @@ __pthread_mutex_trylock (pthread_mutex_t
|
|
|
00db10 |
int kind = PTHREAD_MUTEX_TYPE (mutex);
|
|
|
00db10 |
if (kind == PTHREAD_MUTEX_ROBUST_ERRORCHECK_NP)
|
|
|
00db10 |
{
|
|
|
00db10 |
+ /* We do not need to ensure ordering wrt another memory
|
|
|
00db10 |
+ access. Also see comments at ENQUEUE_MUTEX. */
|
|
|
00db10 |
THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending,
|
|
|
00db10 |
NULL);
|
|
|
00db10 |
return EDEADLK;
|
|
|
00db10 |
@@ -143,6 +153,8 @@ __pthread_mutex_trylock (pthread_mutex_t
|
|
|
00db10 |
|
|
|
00db10 |
if (kind == PTHREAD_MUTEX_ROBUST_RECURSIVE_NP)
|
|
|
00db10 |
{
|
|
|
00db10 |
+ /* We do not need to ensure ordering wrt another memory
|
|
|
00db10 |
+ access. */
|
|
|
00db10 |
THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending,
|
|
|
00db10 |
NULL);
|
|
|
00db10 |
|
|
|
00db10 |
@@ -160,6 +172,10 @@ __pthread_mutex_trylock (pthread_mutex_t
|
|
|
00db10 |
oldval = lll_robust_trylock (mutex->__data.__lock, id);
|
|
|
00db10 |
if (oldval != 0 && (oldval & FUTEX_OWNER_DIED) == 0)
|
|
|
00db10 |
{
|
|
|
00db10 |
+ /* We haven't acquired the lock as it is already acquired by
|
|
|
00db10 |
+ another owner. We do not need to ensure ordering wrt another
|
|
|
00db10 |
+ memory access. */
|
|
|
00db10 |
+
|
|
|
00db10 |
THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL);
|
|
|
00db10 |
|
|
|
00db10 |
return EBUSY;
|
|
|
00db10 |
@@ -173,13 +189,20 @@ __pthread_mutex_trylock (pthread_mutex_t
|
|
|
00db10 |
if (oldval == id)
|
|
|
00db10 |
lll_unlock (mutex->__data.__lock,
|
|
|
00db10 |
PTHREAD_ROBUST_MUTEX_PSHARED (mutex));
|
|
|
00db10 |
+ /* FIXME This violates the mutex destruction requirements. See
|
|
|
00db10 |
+ __pthread_mutex_unlock_full. */
|
|
|
00db10 |
THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL);
|
|
|
00db10 |
return ENOTRECOVERABLE;
|
|
|
00db10 |
}
|
|
|
00db10 |
}
|
|
|
00db10 |
while ((oldval & FUTEX_OWNER_DIED) != 0);
|
|
|
00db10 |
|
|
|
00db10 |
+ /* We must not enqueue the mutex before we have acquired it.
|
|
|
00db10 |
+ Also see comments at ENQUEUE_MUTEX. */
|
|
|
00db10 |
+ __asm ("" ::: "memory");
|
|
|
00db10 |
ENQUEUE_MUTEX (mutex);
|
|
|
00db10 |
+ /* We need to clear op_pending after we enqueue the mutex. */
|
|
|
00db10 |
+ __asm ("" ::: "memory");
|
|
|
00db10 |
THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL);
|
|
|
00db10 |
|
|
|
00db10 |
mutex->__data.__owner = id;
|
|
|
00db10 |
@@ -201,10 +224,15 @@ __pthread_mutex_trylock (pthread_mutex_t
|
|
|
00db10 |
int robust = mutex->__data.__kind & PTHREAD_MUTEX_ROBUST_NORMAL_NP;
|
|
|
00db10 |
|
|
|
00db10 |
if (robust)
|
|
|
00db10 |
- /* Note: robust PI futexes are signaled by setting bit 0. */
|
|
|
00db10 |
- THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending,
|
|
|
00db10 |
- (void *) (((uintptr_t) &mutex->__data.__list.__next)
|
|
|
00db10 |
- | 1));
|
|
|
00db10 |
+ {
|
|
|
00db10 |
+ /* Note: robust PI futexes are signaled by setting bit 0. */
|
|
|
00db10 |
+ THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending,
|
|
|
00db10 |
+ (void *) (((uintptr_t) &mutex->__data.__list.__next)
|
|
|
00db10 |
+ | 1));
|
|
|
00db10 |
+ /* We need to set op_pending before starting the operation. Also
|
|
|
00db10 |
+ see comments at ENQUEUE_MUTEX. */
|
|
|
00db10 |
+ __asm ("" ::: "memory");
|
|
|
00db10 |
+ }
|
|
|
00db10 |
|
|
|
00db10 |
oldval = mutex->__data.__lock;
|
|
|
00db10 |
|
|
|
00db10 |
@@ -213,12 +241,16 @@ __pthread_mutex_trylock (pthread_mutex_t
|
|
|
00db10 |
{
|
|
|
00db10 |
if (kind == PTHREAD_MUTEX_ERRORCHECK_NP)
|
|
|
00db10 |
{
|
|
|
00db10 |
+ /* We do not need to ensure ordering wrt another memory
|
|
|
00db10 |
+ access. */
|
|
|
00db10 |
THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL);
|
|
|
00db10 |
return EDEADLK;
|
|
|
00db10 |
}
|
|
|
00db10 |
|
|
|
00db10 |
if (kind == PTHREAD_MUTEX_RECURSIVE_NP)
|
|
|
00db10 |
{
|
|
|
00db10 |
+ /* We do not need to ensure ordering wrt another memory
|
|
|
00db10 |
+ access. */
|
|
|
00db10 |
THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL);
|
|
|
00db10 |
|
|
|
00db10 |
/* Just bump the counter. */
|
|
|
00db10 |
@@ -240,6 +272,9 @@ __pthread_mutex_trylock (pthread_mutex_t
|
|
|
00db10 |
{
|
|
|
00db10 |
if ((oldval & FUTEX_OWNER_DIED) == 0)
|
|
|
00db10 |
{
|
|
|
00db10 |
+ /* We haven't acquired the lock as it is already acquired by
|
|
|
00db10 |
+ another owner. We do not need to ensure ordering wrt another
|
|
|
00db10 |
+ memory access. */
|
|
|
00db10 |
THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL);
|
|
|
00db10 |
|
|
|
00db10 |
return EBUSY;
|
|
|
00db10 |
@@ -260,6 +295,9 @@ __pthread_mutex_trylock (pthread_mutex_t
|
|
|
00db10 |
if (INTERNAL_SYSCALL_ERROR_P (e, __err)
|
|
|
00db10 |
&& INTERNAL_SYSCALL_ERRNO (e, __err) == EWOULDBLOCK)
|
|
|
00db10 |
{
|
|
|
00db10 |
+ /* The kernel has not yet finished the mutex owner death.
|
|
|
00db10 |
+ We do not need to ensure ordering wrt another memory
|
|
|
00db10 |
+ access. */
|
|
|
00db10 |
THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL);
|
|
|
00db10 |
|
|
|
00db10 |
return EBUSY;
|
|
|
00db10 |
@@ -277,7 +315,12 @@ __pthread_mutex_trylock (pthread_mutex_t
|
|
|
00db10 |
/* But it is inconsistent unless marked otherwise. */
|
|
|
00db10 |
mutex->__data.__owner = PTHREAD_MUTEX_INCONSISTENT;
|
|
|
00db10 |
|
|
|
00db10 |
+ /* We must not enqueue the mutex before we have acquired it.
|
|
|
00db10 |
+ Also see comments at ENQUEUE_MUTEX. */
|
|
|
00db10 |
+ __asm ("" ::: "memory");
|
|
|
00db10 |
ENQUEUE_MUTEX (mutex);
|
|
|
00db10 |
+ /* We need to clear op_pending after we enqueue the mutex. */
|
|
|
00db10 |
+ __asm ("" ::: "memory");
|
|
|
00db10 |
THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL);
|
|
|
00db10 |
|
|
|
00db10 |
/* Note that we deliberately exit here. If we fall
|
|
|
00db10 |
@@ -300,13 +343,20 @@ __pthread_mutex_trylock (pthread_mutex_t
|
|
|
00db10 |
PTHREAD_ROBUST_MUTEX_PSHARED (mutex)),
|
|
|
00db10 |
0, 0);
|
|
|
00db10 |
|
|
|
00db10 |
+ /* To the kernel, this will be visible after the kernel has
|
|
|
00db10 |
+ acquired the mutex in the syscall. */
|
|
|
00db10 |
THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL);
|
|
|
00db10 |
return ENOTRECOVERABLE;
|
|
|
00db10 |
}
|
|
|
00db10 |
|
|
|
00db10 |
if (robust)
|
|
|
00db10 |
{
|
|
|
00db10 |
+ /* We must not enqueue the mutex before we have acquired it.
|
|
|
00db10 |
+ Also see comments at ENQUEUE_MUTEX. */
|
|
|
00db10 |
+ __asm ("" ::: "memory");
|
|
|
00db10 |
ENQUEUE_MUTEX_PI (mutex);
|
|
|
00db10 |
+ /* We need to clear op_pending after we enqueue the mutex. */
|
|
|
00db10 |
+ __asm ("" ::: "memory");
|
|
|
00db10 |
THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL);
|
|
|
00db10 |
}
|
|
|
00db10 |
|