Fix missing wake-ups in pthread_rwlock_rdlock.

This adds wake-ups that would be missing if assuming that for a
non-writer-preferring rwlock, if one thread has acquired a rdlock and
does not release it, another thread will eventually acquire a rdlock too
despite concurrent write lock acquisition attempts.  BZ 14958 is about
supporting this assumption.  Strictly speaking, this isn't a valid
test case, but nonetheless worth supporting (see comment 7 of BZ 14958).
This commit is contained in:
Torvald Riegel 2015-04-28 23:24:36 +02:00
parent 3c9c61febe
commit b634486d57
9 changed files with 297 additions and 18 deletions

View File

@ -1,3 +1,17 @@
2015-06-04 Torvald Riegel <triegel@redhat.com>
[BZ #14958]
* nptl/pthread_rwlock_rdlock.c (__pthread_rwlock_rdlock): Add missing
wake-up.
(__pthread_rwlock_rdlock_slow): Likewise.
* nptl/pthread_rwlock_timedrdlock.c (pthread_rwlock_timedrdlock):
Likewise.
* nptl/pthread_rwlock_tryrdlock.c (__pthread_rwlock_tryrdlock):
Likewise.
* nptl/pthread_rwlock_unlock.c (__pthread_rwlock_unlock): Add comments.
* nptl/tst-rwlock16.c: New file.
* nptl/Makefile (tests): Add new test.
2015-06-04 Torvald Riegel <triegel@redhat.com>
[BZ #18324]

24
NEWS
View File

@ -9,18 +9,18 @@ Version 2.22
* The following bugs are resolved with this release:
438, 4719, 6792, 13028, 13064, 14094, 14841, 14906, 15319, 15467, 15790,
15969, 16159, 16339, 16351, 16352, 16512, 16560, 16704, 16783, 16850,
17053, 17090, 17195, 17269, 17293, 17523, 17542, 17569, 17581, 17588,
17596, 17620, 17621, 17628, 17631, 17692, 17711, 17715, 17776, 17779,
17792, 17836, 17912, 17916, 17930, 17932, 17944, 17949, 17964, 17965,
17967, 17969, 17978, 17987, 17991, 17996, 17998, 17999, 18007, 18019,
18020, 18029, 18030, 18032, 18036, 18038, 18039, 18042, 18043, 18046,
18047, 18049, 18068, 18080, 18093, 18100, 18104, 18110, 18111, 18116,
18125, 18128, 18138, 18185, 18196, 18197, 18206, 18210, 18211, 18217,
18220, 18221, 18234, 18244, 18247, 18287, 18319, 18324, 18333, 18346,
18397, 18409, 18410, 18412, 18418, 18422, 18434, 18444, 18468, 18469,
18470.
438, 4719, 6792, 13028, 13064, 14094, 14841, 14906, 14958, 15319, 15467,
15790, 15969, 16159, 16339, 16351, 16352, 16512, 16560, 16704, 16783,
16850, 17053, 17090, 17195, 17269, 17293, 17523, 17542, 17569, 17581,
17588, 17596, 17620, 17621, 17628, 17631, 17692, 17711, 17715, 17776,
17779, 17792, 17836, 17912, 17916, 17930, 17932, 17944, 17949, 17964,
17965, 17967, 17969, 17978, 17987, 17991, 17996, 17998, 17999, 18007,
18019, 18020, 18029, 18030, 18032, 18036, 18038, 18039, 18042, 18043,
18046, 18047, 18049, 18068, 18080, 18093, 18100, 18104, 18110, 18111,
18116, 18125, 18128, 18138, 18185, 18196, 18197, 18206, 18210, 18211,
18217, 18220, 18221, 18234, 18244, 18247, 18287, 18319, 18324, 18333,
18346, 18397, 18409, 18410, 18412, 18418, 18422, 18434, 18444, 18468,
18469, 18470.
* Cache information can be queried via sysconf() function on s390 e.g. with
_SC_LEVEL1_ICACHE_SIZE as argument.

View File

@ -231,7 +231,7 @@ tests = tst-typesizes \
tst-rwlock1 tst-rwlock2 tst-rwlock2a tst-rwlock3 tst-rwlock4 \
tst-rwlock5 tst-rwlock6 tst-rwlock7 tst-rwlock8 tst-rwlock9 \
tst-rwlock10 tst-rwlock11 tst-rwlock12 tst-rwlock13 tst-rwlock14 \
tst-rwlock15 \
tst-rwlock15 tst-rwlock16 \
tst-once1 tst-once2 tst-once3 tst-once4 \
tst-key1 tst-key2 tst-key3 tst-key4 \
tst-sem1 tst-sem2 tst-sem3 tst-sem4 tst-sem5 tst-sem6 tst-sem7 \

View File

@ -23,6 +23,7 @@
#include <pthreadP.h>
#include <stap-probe.h>
#include <elide.h>
#include <stdbool.h>
/* Acquire read lock for RWLOCK. Slow path. */
@ -30,6 +31,7 @@ static int __attribute__((noinline))
__pthread_rwlock_rdlock_slow (pthread_rwlock_t *rwlock)
{
int result = 0;
bool wake = false;
/* Lock is taken in caller. */
@ -81,7 +83,17 @@ __pthread_rwlock_rdlock_slow (pthread_rwlock_t *rwlock)
result = EAGAIN;
}
else
LIBC_PROBE (rdlock_acquire_read, 1, rwlock);
{
LIBC_PROBE (rdlock_acquire_read, 1, rwlock);
/* See pthread_rwlock_rdlock. */
if (rwlock->__data.__nr_readers == 1
&& rwlock->__data.__nr_readers_queued > 0
&& rwlock->__data.__nr_writers_queued > 0)
{
++rwlock->__data.__readers_wakeup;
wake = true;
}
}
break;
}
@ -90,6 +102,10 @@ __pthread_rwlock_rdlock_slow (pthread_rwlock_t *rwlock)
/* We are done, free the lock. */
lll_unlock (rwlock->__data.__lock, rwlock->__data.__shared);
if (wake)
lll_futex_wake (&rwlock->__data.__readers_wakeup, INT_MAX,
rwlock->__data.__shared);
return result;
}
@ -100,6 +116,7 @@ int
__pthread_rwlock_rdlock (pthread_rwlock_t *rwlock)
{
int result = 0;
bool wake = false;
LIBC_PROBE (rdlock_entry, 1, rwlock);
@ -126,11 +143,30 @@ __pthread_rwlock_rdlock (pthread_rwlock_t *rwlock)
result = EAGAIN;
}
else
LIBC_PROBE (rdlock_acquire_read, 1, rwlock);
{
LIBC_PROBE (rdlock_acquire_read, 1, rwlock);
/* If we are the first reader, and there are blocked readers and
writers (which we don't prefer, see above), then it can be the
case that we stole the lock from a writer that was already woken
to acquire it. That means that we need to take over the writer's
responsibility to wake all readers (see pthread_rwlock_unlock).
Thus, wake all readers in this case. */
if (rwlock->__data.__nr_readers == 1
&& rwlock->__data.__nr_readers_queued > 0
&& rwlock->__data.__nr_writers_queued > 0)
{
++rwlock->__data.__readers_wakeup;
wake = true;
}
}
/* We are done, free the lock. */
lll_unlock (rwlock->__data.__lock, rwlock->__data.__shared);
if (wake)
lll_futex_wake (&rwlock->__data.__readers_wakeup, INT_MAX,
rwlock->__data.__shared);
return result;
}

View File

@ -23,6 +23,7 @@
#include <pthreadP.h>
#include <sys/time.h>
#include <kernel-features.h>
#include <stdbool.h>
/* Try to acquire read lock for RWLOCK or return after specfied time. */
@ -32,6 +33,7 @@ pthread_rwlock_timedrdlock (rwlock, abstime)
const struct timespec *abstime;
{
int result = 0;
bool wake = false;
/* Make sure we are alone. */
lll_lock(rwlock->__data.__lock, rwlock->__data.__shared);
@ -53,6 +55,17 @@ pthread_rwlock_timedrdlock (rwlock, abstime)
--rwlock->__data.__nr_readers;
result = EAGAIN;
}
else
{
/* See pthread_rwlock_rdlock. */
if (rwlock->__data.__nr_readers == 1
&& rwlock->__data.__nr_readers_queued > 0
&& rwlock->__data.__nr_writers_queued > 0)
{
++rwlock->__data.__readers_wakeup;
wake = true;
}
}
break;
}
@ -153,5 +166,9 @@ pthread_rwlock_timedrdlock (rwlock, abstime)
/* We are done, free the lock. */
lll_unlock (rwlock->__data.__lock, rwlock->__data.__shared);
if (wake)
lll_futex_wake (&rwlock->__data.__readers_wakeup, INT_MAX,
rwlock->__data.__shared);
return result;
}

View File

@ -141,8 +141,16 @@ pthread_rwlock_timedwrlock (rwlock, abstime)
/* If we prefer writers, it can have happened that readers blocked
for us to acquire the lock first. If we have timed out, we need
to wake such readers if there are any, and if there is no writer
currently (otherwise, the writer will take care of wake-up). */
if (!PTHREAD_RWLOCK_PREFER_READER_P (rwlock)
currently (otherwise, the writer will take care of wake-up).
Likewise, even if we prefer readers, we can be responsible for
wake-up (see pthread_rwlock_unlock) if no reader or writer has
acquired the lock. We have timed out and thus not consumed a
futex wake-up; therefore, if there is no other blocked writer
that would consume the wake-up and thus take over responsibility,
we need to wake blocked readers. */
if ((!PTHREAD_RWLOCK_PREFER_READER_P (rwlock)
|| ((rwlock->__data.__nr_readers == 0)
&& (rwlock->__data.__nr_writers_queued == 0)))
&& (rwlock->__data.__nr_readers_queued > 0)
&& (rwlock->__data.__writer == 0))
{

View File

@ -20,12 +20,14 @@
#include "pthreadP.h"
#include <lowlevellock.h>
#include <elide.h>
#include <stdbool.h>
int
__pthread_rwlock_tryrdlock (pthread_rwlock_t *rwlock)
{
int result = EBUSY;
bool wake = false;
if (ELIDE_TRYLOCK (rwlock->__data.__rwelision,
rwlock->__data.__lock == 0
@ -45,11 +47,25 @@ __pthread_rwlock_tryrdlock (pthread_rwlock_t *rwlock)
result = EAGAIN;
}
else
result = 0;
{
result = 0;
/* See pthread_rwlock_rdlock. */
if (rwlock->__data.__nr_readers == 1
&& rwlock->__data.__nr_readers_queued > 0
&& rwlock->__data.__nr_writers_queued > 0)
{
++rwlock->__data.__readers_wakeup;
wake = true;
}
}
}
lll_unlock (rwlock->__data.__lock, rwlock->__data.__shared);
if (wake)
lll_futex_wake (&rwlock->__data.__readers_wakeup, INT_MAX,
rwlock->__data.__shared);
return result;
}
strong_alias (__pthread_rwlock_tryrdlock, pthread_rwlock_tryrdlock)

View File

@ -40,8 +40,13 @@ __pthread_rwlock_unlock (pthread_rwlock_t *rwlock)
rwlock->__data.__writer = 0;
else
--rwlock->__data.__nr_readers;
/* If there are still readers present, we do not yet need to wake writers
nor are responsible to wake any readers. */
if (rwlock->__data.__nr_readers == 0)
{
/* Note that if there is a blocked writer, we effectively make it
responsible for waking any readers because we don't wake readers in
this case. */
if (rwlock->__data.__nr_writers_queued)
{
++rwlock->__data.__writer_wakeup;

183
nptl/tst-rwlock16.c Normal file
View File

@ -0,0 +1,183 @@
/* Copyright (C) 2015 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/>. */
/* This tests that with a reader-preferring rwlock, all readers are woken if
one reader "steals" lock ownership from a blocked writer. */
#include <errno.h>
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <semaphore.h>
#include <unistd.h>
/* If we strictly prefer writers over readers, a program must not expect
that, in the presence of concurrent writers, one reader will also acquire
the lock when another reader has already done so. Thus, use the
default rwlock type that does not strictly prefer writers. */
static pthread_rwlock_t r = PTHREAD_RWLOCK_INITIALIZER;
static pthread_mutex_t m = PTHREAD_MUTEX_INITIALIZER;
static pthread_cond_t cv = PTHREAD_COND_INITIALIZER;
/* Avoid using glibc-internal atomic operations. */
static sem_t stop;
static int consumer_stop = 0;
static void *
writer (void *arg)
{
int s;
do
{
if (pthread_rwlock_wrlock (&r) != 0)
{
puts ("wrlock failed");
exit (EXIT_FAILURE);
}
if (pthread_rwlock_unlock (&r) != 0)
{
puts ("unlock failed");
exit (EXIT_FAILURE);
}
sem_getvalue (&stop, &s);
}
while (s == 0);
return NULL;
}
static void *
reader_producer (void *arg)
{
int s;
do
{
if (pthread_rwlock_rdlock (&r) != 0)
{
puts ("rdlock reader failed");
exit (EXIT_FAILURE);
}
sem_getvalue (&stop, &s);
pthread_mutex_lock (&m);
if (s != 0)
consumer_stop = 1;
pthread_cond_signal (&cv);
pthread_mutex_unlock (&m);
if (pthread_rwlock_unlock (&r) != 0)
{
puts ("unlock reader failed");
exit (EXIT_FAILURE);
}
}
while (s == 0);
puts ("producer finished");
return NULL;
}
static void *
reader_consumer (void *arg)
{
int s;
do
{
if (pthread_rwlock_rdlock (&r) != 0)
{
puts ("rdlock reader failed");
exit (EXIT_FAILURE);
}
pthread_mutex_lock (&m);
s = consumer_stop;
if (s == 0)
pthread_cond_wait (&cv, &m);
pthread_mutex_unlock (&m);
if (pthread_rwlock_unlock (&r) != 0)
{
puts ("unlock reader failed");
exit (EXIT_FAILURE);
}
}
while (s == 0);
puts ("consumer finished");
return NULL;
}
static int
do_test (void)
{
pthread_t w1, w2, rp, rc;
if (pthread_create (&w1, NULL, writer, NULL) != 0)
{
puts ("create failed");
return 1;
}
if (pthread_create (&w2, NULL, writer, NULL) != 0)
{
puts ("create failed");
return 1;
}
if (pthread_create (&rc, NULL, reader_consumer, NULL) != 0)
{
puts ("create failed");
return 1;
}
if (pthread_create (&rp, NULL, reader_producer, NULL) != 0)
{
puts ("create failed");
return 1;
}
sleep (2);
sem_post (&stop);
if (pthread_join (w1, NULL) != 0)
{
puts ("w1 join failed");
return 1;
}
if (pthread_join (w2, NULL) != 0)
{
puts ("w2 join failed");
return 1;
}
if (pthread_join (rp, NULL) != 0)
{
puts ("reader_producer join failed");
return 1;
}
if (pthread_join (rc, NULL) != 0)
{
puts ("reader_consumer join failed");
return 1;
}
return 0;
}
#define TIMEOUT 3
#define TEST_FUNCTION do_test ()
#include "../test-skeleton.c"