This patch fixes PR52526, a lost wake-up in libitm (ie, one ore more
threads could hang and not get woken up anymore).
The problem was missing handling of one corner case in the futex-based
serial lock implementation (config/linux/rwlock.cc, read_lock()):
Multiple readers would set READERS to 1 and only call
futex_wait(&readers, 1) if there were any writers. Writers would set
READERS to 0 and then call futex_wake(&readers). That's fine, but
because there are multiple readers, it can happen that some would set
READERS to 1 after the writer's futex_wake() call, enabling the
futex_wait() in other readers (because READERS isn't 0 anymore). This
patch fixes this by having readers wake up all potentially waiting
readers when they set READERS to 1 without an existing writer (thus
taking over what the writer would do).
OK for trunk?
OK for 4.7 too? This is a showstopper if users hit it, so I'd prefer if
it could go into 4.7 as well.
commit 07d6d68b423797311bb04d8eb571f053d2078aa4
Author: Torvald Riegel <trie...@redhat.com>
Date: Sat Mar 10 17:44:37 2012 +0100
libitm: Fix lost wake-up in serial lock.
PR libitm/52526
* config/linux/rwlock.cc (GTM::gtm_rwlock::read_lock): Fix lost
wake-up.
diff --git a/libitm/config/linux/rwlock.cc b/libitm/config/linux/rwlock.cc
index ad1b042..cf1fdd5 100644
--- a/libitm/config/linux/rwlock.cc
+++ b/libitm/config/linux/rwlock.cc
@@ -74,6 +74,32 @@ gtm_rwlock::read_lock (gtm_thread *tx)
atomic_thread_fence (memory_order_seq_cst);
if (writers.load (memory_order_relaxed))
futex_wait(&readers, 1);
+ else
+ {
+ // There is no writer, actually. However, we can have enabled
+ // a futex_wait in other readers by previously setting readers
+ // to 1, so we have to wake them up because there is no writer
+ // that will do that. We don't know whether the wake-up is
+ // really necessary, but we can get lost wake-up situations
+ // otherwise.
+ // No additional barrier nor a nonrelaxed load is required due
+ // to coherency constraints. write_unlock() checks readers to
+ // see if any wake-up is necessary, but it is not possible that
+ // a reader's store prevents a required later writer wake-up;
+ // If the waking reader's store (value 0) is in modification
+ // order after the waiting readers store (value 1), then the
+ // latter will have to read 0 in the futex due to coherency
+ // constraints and the happens-before enforced by the futex
+ // (paragraph 6.10 in the standard, 6.19.4 in the Batty et al
+ // TR); second, the writer will be forced to read in
+ // modification order too due to Dekker-style synchronization
+ // with the waiting reader (see write_unlock()).
+ // ??? Can we avoid the wake-up if readers is zero (like in
+ // write_unlock())? Anyway, this might happen too infrequently
+ // to improve performance significantly.
+ readers.store (0, memory_order_relaxed);
+ futex_wake(&readers, INT_MAX);
+ }
}
// And we try again to acquire a read lock.
