MDEV-11296 - InnoDB stalls under OLTP RW on P8

Threads may fall asleep forever while acquiring InnoDB rw-lock on Power8. This
regression was introduced along with InnoDB atomic operations fixes.

The problem was that proper memory order wasn't enforced between "writers"
store and "lock_word" load:

  my_atomic_store32((int32*) &lock->waiters, 1);
  ...
  local_lock_word = lock->lock_word;

Locking protocol is such that store to "writers" must be completed before load
of "lock_word". my_atomic_store32() was expected to enforce memory order because
it issues strongest MY_MEMORY_ORDER_SEQ_CST memory barrier.

According to C11:
- any operation with this memory order is both an acquire operation and a
  release operation
- for atomic store order must be one of memory_order_relaxed
  memory_order_release or memory_order_seq_cst. Otherwise the behavior is
  undefined.

That is it doesn't say explicitly that this expectation is wrong, but there are
indications that acquire (which is actually supposed to guarantee memory order
in this case) may not be issued along with MY_MEMORY_ORDER_SEQ_CST.

A good fix for this is to encode waiters into lock_word, but it is a bit too
intrusive. Instead we change atomic store to atomic fetch-and-store, which
does memory load and is guaranteed to issue acquire.

1 files changed, 3 insertions, 3 deletions

diff --git a/storage/innobase/sync/sync0rw.cc b/storage/innobase/sync/sync0rw.cc
index 77a9102112f..5f617ae22f5 100644
--- a/storage/innobase/sync/sync0rw.cc
+++ b/storage/innobase/sync/sync0rw.cc
@@ -358,7 +358,7 @@ lock_loop:
 
 		/* Set waiters before checking lock_word to ensure wake-up
 		signal is sent. This may lead to some unnecessary signals. */
-		my_atomic_store32((int32*) &lock->waiters, 1);
+		my_atomic_fas32((int32*) &lock->waiters, 1);
 
 		if (rw_lock_s_lock_low(lock, pass, file_name, line)) {
 
@@ -750,7 +750,7 @@ lock_loop:
 
 	/* Waiters must be set before checking lock_word, to ensure signal
 	is sent. This could lead to a few unnecessary wake-up signals. */
-	my_atomic_store32((int32*) &lock->waiters, 1);
+	my_atomic_fas32((int32*) &lock->waiters, 1);
 
 	if (rw_lock_x_lock_low(lock, pass, file_name, line)) {
 		sync_array_free_cell(sync_arr, cell);
@@ -855,7 +855,7 @@ lock_loop:
 
 	/* Waiters must be set before checking lock_word, to ensure signal
 	is sent. This could lead to a few unnecessary wake-up signals. */
-	my_atomic_store32((int32*) &lock->waiters, 1);
+	my_atomic_fas32((int32*) &lock->waiters, 1);
 
 	if (rw_lock_sx_lock_low(lock, pass, file_name, line)) {