rwlock: Fix explicit hand-over (bug 21298)

Without this fix, the rwlock can fail to execute the explicit hand-over
in certain cases (e.g., empty critical sections that switch quickly between
read and write phases).  This can then lead to errors in how __wrphase_futex
is accessed, which in turn can lead to deadlocks.
This commit is contained in:
Carlos O'Donell 2017-07-28 00:22:44 -04:00
parent 2557ae38f3
commit faf8c066df
12 changed files with 557 additions and 241 deletions

View File

@ -1,3 +1,27 @@
2017-07-28 Torvald Riegel <triegel@redhat.com>
Carlos O'Donell <carlos@redhat.com>
[BZ #21298]
* nptl/Makefile (tests-internal): Add tst-rwlock20.
* nptl/pthread_rwlock_common.c (__pthread_rwlock_rdlock_full): Fix
explicit hand-over.
(__pthread_rwlock_wrlock_full): Likewise.
* nptl/tst-rwlock20.c: New file.
* support/Makefile (libsupport-routines): Add xpthread_rwlock_init,
xpthread_rwlock_rdlock, xpthread_rwlock_unlock,
xpthread_rwlock_wrlock, xpthread_rwlockattr_init, and
xpthread_rwlockattr_setkind_np.
* support/xpthread_rwlock_init.c: New file.
* support/xpthread_rwlock_rdlock.c: New file.
* support/xpthread_rwlock_unlock.c: New file.
* support/xpthread_rwlock_wrlock.c: New file.
* support/xpthread_rwlockattr_init.c: New file.
* support/xpthread_rwlockattr_setkind_np.c: New file.
* support/xthread.h: Add xpthread_rwlock_init, xpthread_rwlock_rdlock,
xpthread_rwlock_unlock, xpthread_rwlock_wrlock,
xpthread_rwlockattr_init, and xpthread_rwlockattr_setkind_np
prototypes.
2017-07-27 Adhemerval Zanella <adhemerval.zanella@linaro.org>
* sysdeps/alpha/fpu/libm-test-ulps: Update.

View File

@ -304,7 +304,9 @@ tests = tst-attr1 tst-attr2 tst-attr3 tst-default-attr \
tst-thread_local1 tst-mutex-errorcheck tst-robust10 \
tst-robust-fork tst-create-detached tst-memstream
tests-internal := tst-typesizes tst-rwlock19 tst-sem11 tst-sem12 tst-sem13 \
tests-internal := tst-typesizes \
tst-rwlock19 tst-rwlock20 \
tst-sem11 tst-sem12 tst-sem13 \
tst-barrier5 tst-signal7 tst-mutex8 tst-mutex8-static \
tst-mutexpi8 tst-mutexpi8-static

View File

@ -55,7 +55,6 @@
lock acquisition attempts, so that new incoming readers do not prolong a
phase in which readers have acquired the lock.
The main components of the rwlock are a writer-only lock that allows only
one of the concurrent writers to be the primary writer, and a
single-writer-multiple-readers lock that decides between read phases, in
@ -70,15 +69,16 @@
---------------------------
#1 0 0 0 0 Lock is idle (and in a read phase).
#2 0 0 >0 0 Readers have acquired the lock.
#3 0 1 0 0 Lock is not acquired; a writer is waiting for a write
phase to start or will try to start one.
#3 0 1 0 0 Lock is not acquired; a writer will try to start a
write phase.
#4 0 1 >0 0 Readers have acquired the lock; a writer is waiting
and explicit hand-over to the writer is required.
#4a 0 1 >0 1 Same as #4 except that there are further readers
waiting because the writer is to be preferred.
#5 1 0 0 0 Lock is idle (and in a write phase).
#6 1 0 >0 0 Write phase; readers are waiting for a read phase to
start or will try to start one.
#6 1 0 >0 0 Write phase; readers will try to start a read phase
(requires explicit hand-over to all readers that
do not start the read phase).
#7 1 1 0 0 Lock is acquired by a writer.
#8 1 1 >0 0 Lock acquired by a writer and readers are waiting;
explicit hand-over to the readers is required.
@ -375,9 +375,9 @@ __pthread_rwlock_rdlock_full (pthread_rwlock_t *rwlock,
complexity. */
if (__glibc_likely ((r & PTHREAD_RWLOCK_WRPHASE) == 0))
return 0;
/* If there is no primary writer but we are in a write phase, we can try
to install a read phase ourself. */
/* Otherwise, if we were in a write phase (states #6 or #8), we must wait
for explicit hand-over of the read phase; the only exception is if we
can start a read phase if there is no primary writer currently. */
while (((r & PTHREAD_RWLOCK_WRPHASE) != 0)
&& ((r & PTHREAD_RWLOCK_WRLOCKED) == 0))
{
@ -390,15 +390,18 @@ __pthread_rwlock_rdlock_full (pthread_rwlock_t *rwlock,
{
/* We started the read phase, so we are also responsible for
updating the write-phase futex. Relaxed MO is sufficient.
Note that there can be no other reader that we have to wake
because all other readers will see the read phase started by us
(or they will try to start it themselves); if a writer started
the read phase, we cannot have started it. Furthermore, we
cannot discard a PTHREAD_RWLOCK_FUTEX_USED flag because we will
overwrite the value set by the most recent writer (or the readers
before it in case of explicit hand-over) and we know that there
are no waiting readers. */
atomic_store_relaxed (&rwlock->__data.__wrphase_futex, 0);
We have to do the same steps as a writer would when handing
over the read phase to us because other readers cannot
distinguish between us and the writer; this includes
explicit hand-over and potentially having to wake other readers
(but we can pretend to do the setting and unsetting of WRLOCKED
atomically, and thus can skip this step). */
if ((atomic_exchange_relaxed (&rwlock->__data.__wrphase_futex, 0)
& PTHREAD_RWLOCK_FUTEX_USED) != 0)
{
int private = __pthread_rwlock_get_private (rwlock);
futex_wake (&rwlock->__data.__wrphase_futex, INT_MAX, private);
}
return 0;
}
else
@ -407,15 +410,12 @@ __pthread_rwlock_rdlock_full (pthread_rwlock_t *rwlock,
}
}
if ((r & PTHREAD_RWLOCK_WRPHASE) != 0)
{
/* We are in a write phase, and there must be a primary writer because
of the previous loop. Block until the primary writer gives up the
write phase. This case requires explicit hand-over using
__wrphase_futex.
/* We were in a write phase but did not install the read phase. We cannot
distinguish between a writer and another reader starting the read phase,
so we must wait for explicit hand-over via __wrphase_futex.
However, __wrphase_futex might not have been set to 1 yet (either
because explicit hand-over to the writer is still ongoing, or because
the writer has started the write phase but does not yet have updated
the writer has started the write phase but has not yet updated
__wrphase_futex). The least recent value of __wrphase_futex we can
read from here is the modification of the last read phase (because
we synchronize with the last reader in this read phase through
@ -503,7 +503,6 @@ __pthread_rwlock_rdlock_full (pthread_rwlock_t *rwlock,
above one more time. */
ready = true;
}
}
return 0;
}
@ -741,10 +740,23 @@ __pthread_rwlock_wrlock_full (pthread_rwlock_t *rwlock,
r = atomic_load_relaxed (&rwlock->__data.__readers);
}
/* Our snapshot of __readers is up-to-date at this point because we
either set WRLOCKED using a CAS or were handed over WRLOCKED from
either set WRLOCKED using a CAS (and update r accordingly below,
which was used as expected value for the CAS) or got WRLOCKED from
another writer whose snapshot of __readers we inherit. */
r |= PTHREAD_RWLOCK_WRLOCKED;
}
/* We are the primary writer; enable blocking on __writers_futex. Relaxed
MO is sufficient for futex words; acquire MO on the previous
modifications of __readers ensures that this store happens after the
store of value 0 by the previous primary writer. */
atomic_store_relaxed (&rwlock->__data.__writers_futex,
1 | (may_share_futex_used_flag ? PTHREAD_RWLOCK_FUTEX_USED : 0));
/* If we are in a write phase, we have acquired the lock. */
if ((r & PTHREAD_RWLOCK_WRPHASE) != 0)
goto done;
/* If we are in a read phase and there are no readers, try to start a write
phase. */
while (((r & PTHREAD_RWLOCK_WRPHASE) == 0)
@ -758,27 +770,19 @@ __pthread_rwlock_wrlock_full (pthread_rwlock_t *rwlock,
&r, r | PTHREAD_RWLOCK_WRPHASE))
{
/* We have started a write phase, so need to enable readers to wait.
See the similar case in__pthread_rwlock_rdlock_full. */
See the similar case in __pthread_rwlock_rdlock_full. Unlike in
that similar case, we are the (only) primary writer and so do
not need to wake another writer. */
atomic_store_relaxed (&rwlock->__data.__wrphase_futex, 1);
/* Make sure we fall through to the end of the function. */
r |= PTHREAD_RWLOCK_WRPHASE;
break;
goto done;
}
/* TODO Back-off. */
}
/* We are the primary writer; enable blocking on __writers_futex. Relaxed
MO is sufficient for futex words; acquire MO on the previous
modifications of __readers ensures that this store happens after the
store of value 0 by the previous primary writer. */
atomic_store_relaxed (&rwlock->__data.__writers_futex,
1 | (may_share_futex_used_flag ? PTHREAD_RWLOCK_FUTEX_USED : 0));
if (__glibc_unlikely ((r & PTHREAD_RWLOCK_WRPHASE) == 0))
{
/* We are not in a read phase and there are readers (because of the
previous loop). Thus, we have to wait for explicit hand-over from
one of these readers.
/* We became the primary writer in a read phase and there were readers when
we did (because of the previous loop). Thus, we have to wait for
explicit hand-over from one of these readers.
We basically do the same steps as for the similar case in
__pthread_rwlock_rdlock_full, except that we additionally might try
to directly hand over to another writer and need to wake up
@ -829,9 +833,8 @@ __pthread_rwlock_wrlock_full (pthread_rwlock_t *rwlock,
{
/* Wake other writers. */
if ((wf & PTHREAD_RWLOCK_FUTEX_USED) != 0)
futex_wake
(&rwlock->__data.__writers_futex, 1,
private);
futex_wake (&rwlock->__data.__writers_futex,
1, private);
return ETIMEDOUT;
}
/* TODO Back-off. */
@ -886,8 +889,7 @@ __pthread_rwlock_wrlock_full (pthread_rwlock_t *rwlock,
write phase concurrently, so enable waiting again.
Make sure we don't loose the flag that signals
whether there are threads waiting on this futex. */
atomic_store_relaxed (&rwlock->__data.__writers_futex,
wf);
atomic_store_relaxed (&rwlock->__data.__writers_futex, wf);
}
/* Use the acquire MO fence to mirror the steps taken in the
non-timeout case. Note that the read can happen both
@ -916,8 +918,8 @@ __pthread_rwlock_wrlock_full (pthread_rwlock_t *rwlock,
& PTHREAD_RWLOCK_WRPHASE) != 0)
ready = true;
}
}
done:
atomic_store_relaxed (&rwlock->__data.__cur_writer,
THREAD_GETMEM (THREAD_SELF, tid));
return 0;

116
nptl/tst-rwlock20.c Normal file
View File

@ -0,0 +1,116 @@
/* Test program for a read-phase / write-phase explicit hand-over.
Copyright (C) 2017 Free Software Foundation, Inc.
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; see the file COPYING.LIB. If
not, see <http://www.gnu.org/licenses/>. */
#include <errno.h>
#include <error.h>
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <stdint.h>
#include <time.h>
#include <atomic.h>
#include <support/xthread.h>
/* We realy want to set threads to 2 to reproduce this issue. The goal
is to have one primary writer and a single reader, and to hit the
bug that happens in the interleaving of those two phase transitions.
However, on most hardware, adding a second writer seems to help the
interleaving happen slightly more often, say 20% of the time. On a
16 core ppc64 machine this fails 100% of the time with an unpatched
glibc. On a 8 core x86_64 machine this fails ~93% of the time, but
it doesn't fail at all on a 4 core system, so having available
unloaded cores makes a big difference in reproducibility. On an 8
core qemu/kvm guest the reproducer reliability drops to ~10%. */
#define THREADS 3
#define KIND PTHREAD_RWLOCK_PREFER_READER_NP
static pthread_rwlock_t lock;
static int done = 0;
static void*
tf (void* arg)
{
while (atomic_load_relaxed (&done) == 0)
{
int rcnt = 0;
int wcnt = 100;
if ((uintptr_t) arg == 0)
{
rcnt = 1;
wcnt = 1;
}
do
{
if (wcnt)
{
xpthread_rwlock_wrlock (&lock);
xpthread_rwlock_unlock (&lock);
wcnt--;
}
if (rcnt)
{
xpthread_rwlock_rdlock (&lock);
xpthread_rwlock_unlock (&lock);
rcnt--;
}
}
while ((atomic_load_relaxed (&done) == 0) && (rcnt + wcnt > 0));
}
return NULL;
}
static int
do_test (void)
{
pthread_t thr[THREADS];
int n;
pthread_rwlockattr_t attr;
xpthread_rwlockattr_init (&attr);
xpthread_rwlockattr_setkind_np (&attr, KIND);
xpthread_rwlock_init (&lock, &attr);
/* Make standard error the same as standard output. */
dup2 (1, 2);
/* Make sure we see all message, even those on stdout. */
setvbuf (stdout, NULL, _IONBF, 0);
for (n = 0; n < THREADS; ++n)
thr[n] = xpthread_create (NULL, tf, (void *) (uintptr_t) n);
struct timespec delay;
delay.tv_sec = 10;
delay.tv_nsec = 0;
nanosleep (&delay, NULL);
atomic_store_relaxed (&done, 1);
/* Wait for all the threads. */
for (n = 0; n < THREADS; ++n)
xpthread_join (thr[n]);
return 0;
}
#include <support/test-driver.c>

View File

@ -106,6 +106,12 @@ libsupport-routines = \
xpthread_mutexattr_setrobust \
xpthread_mutexattr_settype \
xpthread_once \
xpthread_rwlock_init \
xpthread_rwlock_rdlock \
xpthread_rwlock_wrlock \
xpthread_rwlock_unlock \
xpthread_rwlockattr_init \
xpthread_rwlockattr_setkind_np \
xpthread_sigmask \
xpthread_spin_lock \
xpthread_spin_unlock \

View File

@ -0,0 +1,27 @@
/* pthread_rwlock_init with error checking.
Copyright (C) 2017 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 <support/xthread.h>
void
xpthread_rwlock_init (pthread_rwlock_t *rwlock,
const pthread_rwlockattr_t *attr)
{
xpthread_check_return ("pthread_rwlock_init",
pthread_rwlock_init (rwlock, attr));
}

View File

@ -0,0 +1,26 @@
/* pthread_rwlock_rdlock with error checking.
Copyright (C) 2017 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 <support/xthread.h>
void
xpthread_rwlock_rdlock (pthread_rwlock_t *rwlock)
{
xpthread_check_return ("pthread_rwlock_rdlock",
pthread_rwlock_rdlock (rwlock));
}

View File

@ -0,0 +1,26 @@
/* pthread_rwlock_unlock with error checking.
Copyright (C) 2017 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 <support/xthread.h>
void
xpthread_rwlock_unlock (pthread_rwlock_t *rwlock)
{
xpthread_check_return ("pthread_rwlock_unlock",
pthread_rwlock_unlock (rwlock));
}

View File

@ -0,0 +1,26 @@
/* pthread_rwlock_wrlock with error checking.
Copyright (C) 2017 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 <support/xthread.h>
void
xpthread_rwlock_wrlock (pthread_rwlock_t *rwlock)
{
xpthread_check_return ("pthread_rwlock_wrlock",
pthread_rwlock_wrlock (rwlock));
}

View File

@ -0,0 +1,26 @@
/* pthread_rwlockattr_init with error checking.
Copyright (C) 2017 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 <support/xthread.h>
void
xpthread_rwlockattr_init (pthread_rwlockattr_t *attr)
{
xpthread_check_return ("pthread_rwlockattr_init",
pthread_rwlockattr_init (attr));
}

View File

@ -0,0 +1,27 @@
/* pthread_rwlockattr_setkind_np with error checking.
Copyright (C) 2017 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 <support/xthread.h>
void
xpthread_rwlockattr_setkind_np (pthread_rwlockattr_t *attr,
int pref)
{
xpthread_check_return ("pthread_rwlockattr_setkind_np",
pthread_rwlockattr_setkind_np (attr, pref));
}

View File

@ -74,6 +74,14 @@ void xpthread_attr_setguardsize (pthread_attr_t *attr,
PTHREAD_BARRIER_SERIAL_THREAD. */
int xpthread_barrier_wait (pthread_barrier_t *barrier);
void xpthread_rwlock_init (pthread_rwlock_t *rwlock,
const pthread_rwlockattr_t *attr);
void xpthread_rwlockattr_init (pthread_rwlockattr_t *attr);
void xpthread_rwlockattr_setkind_np (pthread_rwlockattr_t *attr, int pref);
void xpthread_rwlock_wrlock (pthread_rwlock_t *rwlock);
void xpthread_rwlock_rdlock (pthread_rwlock_t *rwlock);
void xpthread_rwlock_unlock (pthread_rwlock_t *rwlock);
__END_DECLS
#endif /* SUPPORT_THREAD_H */