MDEV-14529 - InnoDB rw-locks: optimize memory barriers

Remove volatile modifier from waiters: it's not supposed for inter-thread communication, use appropriate atomic operations instead. Changed waiters to int32_t, my_atomic friendly type.
MariaDB · Dec 8, 2017 · 5b624f0 · 5b624f0
1 parent 57d20f1
commit 5b624f0
Show file tree

Hide file tree

Showing 5 changed files with 13 additions and 10 deletions.
diff --git a/storage/innobase/include/sync0rw.h b/storage/innobase/include/sync0rw.h
@@ -574,7 +574,7 @@ struct rw_lock_t
 	int32_t	lock_word;
 
 	/** 1: there are waiters */
-	volatile uint32_t	waiters;
+	int32_t	waiters;
 
 	/** number of granted SX locks. */
 	volatile ulint	sx_recursive;

diff --git a/storage/innobase/include/sync0rw.ic b/storage/innobase/include/sync0rw.ic
@@ -418,7 +418,7 @@ rw_lock_x_unlock_func(
 		We need to signal read/write waiters.
 		We do not need to signal wait_ex waiters, since they cannot
 		exist when there is a writer. */
-		if (my_atomic_load32_explicit((int32*) &lock->waiters,
+		if (my_atomic_load32_explicit(&lock->waiters,
 					      MY_MEMORY_ORDER_RELAXED)) {
 			my_atomic_store32((int32*) &lock->waiters, 0);
 			os_event_set(lock->event);
@@ -472,7 +472,8 @@ rw_lock_sx_unlock_func(
 			waiters. We do not need to signal wait_ex waiters,
 			since they cannot exist when there is an sx-lock
 			holder. */
-			if (lock->waiters) {
+			if (my_atomic_load32_explicit(&lock->waiters,
+						      MY_MEMORY_ORDER_RELAXED)) {
 				my_atomic_store32((int32*) &lock->waiters, 0);
 				os_event_set(lock->event);
 				sync_array_object_signalled();

diff --git a/storage/innobase/row/row0merge.cc b/storage/innobase/row/row0merge.cc
@@ -1984,7 +1984,8 @@ row_merge_read_clustered_index(
 			}
 
 			if (dbug_run_purge
-			    || dict_index_get_lock(clust_index)->waiters) {
+			    || my_atomic_load32_explicit(&clust_index->lock.waiters,
+							 MY_MEMORY_ORDER_RELAXED)) {
 				/* There are waiters on the clustered
 				index tree lock, likely the purge
 				thread. Store and restore the cursor

diff --git a/storage/innobase/sync/sync0arr.cc b/storage/innobase/sync/sync0arr.cc
@@ -588,7 +588,7 @@ sync_array_cell_print(
 
 			fprintf(file,
 				"number of readers " ULINTPF
-				", waiters flag %u, "
+				", waiters flag %d, "
 				"lock_word: %x\n"
 				"Last time read locked in file %s line %u\n"
 				"Last time write locked in file %s line %u"
@@ -598,7 +598,7 @@ sync_array_cell_print(
 #endif
 				"\n",
 				rw_lock_get_reader_count(rwlock),
-				rwlock->waiters,
+				my_atomic_load32_explicit(&rwlock->waiters, MY_MEMORY_ORDER_RELAXED),
 				my_atomic_load32_explicit(&rwlock->lock_word, MY_MEMORY_ORDER_RELAXED),
 				innobase_basename(rwlock->last_s_file_name),
 				rwlock->last_s_line,
@@ -1397,7 +1397,8 @@ sync_arr_fill_sys_semphore_waits_table(
 						//OK(fields[SYS_SEMAPHORE_WAITS_HOLDER_LINE]->store(rwlock->line, true));
 						//fields[SYS_SEMAPHORE_WAITS_HOLDER_LINE]->set_notnull();
 						OK(field_store_ulint(fields[SYS_SEMAPHORE_WAITS_READERS], rw_lock_get_reader_count(rwlock)));
-						OK(field_store_ulint(fields[SYS_SEMAPHORE_WAITS_WAITERS_FLAG], (longlong)rwlock->waiters));
+						OK(field_store_ulint(fields[SYS_SEMAPHORE_WAITS_WAITERS_FLAG],
+						   my_atomic_load32_explicit(&rwlock->waiters, MY_MEMORY_ORDER_RELAXED)));
 						OK(field_store_ulint(fields[SYS_SEMAPHORE_WAITS_LOCK_WORD],
 						   my_atomic_load32_explicit(&rwlock->lock_word, MY_MEMORY_ORDER_RELAXED)));
 						OK(field_store_string(fields[SYS_SEMAPHORE_WAITS_LAST_READER_FILE], innobase_basename(rwlock->last_s_file_name)));

diff --git a/storage/innobase/sync/sync0rw.cc b/storage/innobase/sync/sync0rw.cc
@@ -897,7 +897,7 @@ rw_lock_validate(
 					      MY_MEMORY_ORDER_RELAXED);
 
 	ut_ad(lock->magic_n == RW_LOCK_MAGIC_N);
-	ut_ad(my_atomic_load32_explicit((int32*) &lock->waiters,
+	ut_ad(my_atomic_load32_explicit(const_cast<int32_t*>(&lock->waiters),
 					MY_MEMORY_ORDER_RELAXED) < 2);
 	ut_ad(lock_word > -(2 * X_LOCK_DECR));
 	ut_ad(lock_word <= X_LOCK_DECR);
@@ -1166,8 +1166,8 @@ rw_lock_list_print_info(
 
 			fprintf(file, "RW-LOCK: %p ", (void*) lock);
 
-			if (lock->waiters) {
-				fputs(" Waiters for the lock exist\n", file);
+			if (int32_t waiters= my_atomic_load32_explicit(const_cast<int32_t*>(&lock->waiters), MY_MEMORY_ORDER_RELAXED)) {
+				fprintf(file, " (%d waiters)\n", waiters);
 			} else {
 				putc('\n', file);
 			}