From 5b624f00fc0e6fa0a5a676d3ec445f62c0fb75ec Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Fri, 1 Dec 2017 13:37:07 +0400 Subject: [PATCH] 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. --- storage/innobase/include/sync0rw.h | 2 +- storage/innobase/include/sync0rw.ic | 5 +++-- storage/innobase/row/row0merge.cc | 3 ++- storage/innobase/sync/sync0arr.cc | 7 ++++--- storage/innobase/sync/sync0rw.cc | 6 +++--- 5 files changed, 13 insertions(+), 10 deletions(-) diff --git a/storage/innobase/include/sync0rw.h b/storage/innobase/include/sync0rw.h index b8d5a8e832625..ae5f410e81075 100644 --- 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 index 774d7993e199d..9e3d2996ad393 100644 --- 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 index 6f0d7ccdb6148..53e51bdd93009 100644 --- 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 index c5fd42a88e93a..858b8c02e73dd 100644 --- 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 index f44385677ca1d..93898756a2737 100644 --- 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(&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(&lock->waiters), MY_MEMORY_ORDER_RELAXED)) { + fprintf(file, " (%d waiters)\n", waiters); } else { putc('\n', file); }