Description: Add full memory barrier to ensure proper StoreLoad order on Power The fixes for MDEV-6450 and MDEV-6483 introduced a regression on x86, so the solution was reworked with full barrier support added for Power in architectures specific ifdefs. This patch brought back a data corruption issue on AArch64: https://mariadb.atlassian.net/browse/MDEV-7658 This is fixed up in a patch later in the series. Origin: https://github.com/MariaDB/server/commit/6ea41f1e84eb6b864cac17ad0b862bde9820dc33 Bug-Ubuntu: https://bugs.launchpad.net/bugs/1427406 Forwarded: no Author: Kristian Nielsen Last-Update: 2015-03-30 Index: mysql-5.6/storage/innobase/include/os0sync.h =================================================================== --- mysql-5.6.orig/storage/innobase/include/os0sync.h +++ mysql-5.6/storage/innobase/include/os0sync.h @@ -238,10 +238,20 @@ os_fast_mutex_trylock( os_fast_mutex_t* fast_mutex); /*!< in: mutex to acquire */ /********************************************************************** +Acquires ownership of a fast mutex. Implies a full memory barrier even on +platforms such as PowerPC where this is not normally required. +@return 0 if success, != 0 if was reserved by another thread */ +UNIV_INLINE +ulint +os_fast_mutex_trylock_full_barrier( +/*==================*/ + os_fast_mutex_t* fast_mutex); /*!< in: mutex to acquire */ +/**********************************************************//** Following os_fast_ mutex APIs would be performance schema instrumented: os_fast_mutex_init os_fast_mutex_lock +os_fast_mutex_unlock_full_barrier os_fast_mutex_unlock os_fast_mutex_free @@ -258,6 +268,8 @@ corresponding function. */ # define os_fast_mutex_lock(M) \ pfs_os_fast_mutex_lock(M, __FILE__, __LINE__) +# define os_fast_mutex_unlock_full_barrier(M) pfs_os_fast_mutex_unlock_full_barrier(M) + # define os_fast_mutex_unlock(M) pfs_os_fast_mutex_unlock(M) # define os_fast_mutex_free(M) pfs_os_fast_mutex_free(M) @@ -305,6 +317,15 @@ void pfs_os_fast_mutex_unlock( /*=====================*/ os_fast_mutex_t* fast_mutex); /*!< in/out: mutex to release */ +/**********************************************************//** +NOTE! Please use the corresponding macro os_fast_mutex_unlock, not directly +this function! +Wrapper function of os_fast_mutex_unlock. Releases ownership of a fast mutex. */ +UNIV_INLINE +void +pfs_os_fast_mutex_unlock_full_barrier( +/*=====================*/ + os_fast_mutex_t* fast_mutex); /*!< in/out: mutex to release */ #else /* UNIV_PFS_MUTEX */ @@ -314,6 +335,9 @@ pfs_os_fast_mutex_unlock( # define os_fast_mutex_lock(M) \ os_fast_mutex_lock_func(&((os_fast_mutex_t*)(M))->mutex) +# define os_fast_mutex_unlock_full_barrier(M) \ + os_fast_mutex_unlock_full_barrier_func(&((os_fast_mutex_t*)(M))->mutex) + # define os_fast_mutex_unlock(M) \ os_fast_mutex_unlock_func(&((os_fast_mutex_t*)(M))->mutex) @@ -328,6 +352,14 @@ void os_fast_mutex_unlock_func( /*======================*/ fast_mutex_t* fast_mutex); /*!< in: mutex to release */ +/**********************************************************//** +Releases ownership of a fast mutex. Implies a full memory barrier even on +platforms such as PowerPC where this is not normally required. */ +UNIV_INTERN +void +os_fast_mutex_unlock_full_barrier_func( +/*=================*/ + fast_mutex_t* fast_mutex); /*!< in: mutex to release */ /*********************************************************//** Initializes an operating system fast mutex semaphore. */ UNIV_INTERN @@ -432,15 +464,32 @@ amount to decrement. */ /**********************************************************//** Returns the old value of *ptr, atomically sets *ptr to new_val */ -# define os_atomic_test_and_set_byte(ptr, new_val) \ +#ifdef __powerpc__ +/* + os_atomic_test_and_set_byte_release() should imply a release barrier before + setting, and a full barrier after. But __sync_lock_test_and_set() is only + documented as an aquire barrier. So on PowerPC we need to add the full + barrier explicitly. */ +# define os_atomic_test_and_set_byte_release(ptr, new_val) \ + do { __sync_lock_release(ptr); \ + __sync_synchronize(); } while (0) +#else +/* + On x86, __sync_lock_test_and_set() happens to be full barrier, due to + LOCK prefix. +*/ +# define os_atomic_test_and_set_byte_release(ptr, new_val) \ + __sync_lock_test_and_set(ptr, (byte) new_val) +#endif +/* + os_atomic_test_and_set_byte_acquire() is a full memory barrier on x86. But + in general, just an aquire barrier should be sufficient. */ +# define os_atomic_test_and_set_byte_acquire(ptr, new_val) \ __sync_lock_test_and_set(ptr, (byte) new_val) # define os_atomic_test_and_set_ulint(ptr, new_val) \ __sync_lock_test_and_set(ptr, new_val) -# define os_atomic_lock_release_byte(ptr) \ - __sync_lock_release(ptr) - #elif defined(HAVE_IB_SOLARIS_ATOMICS) # define HAVE_ATOMIC_BUILTINS @@ -517,15 +566,14 @@ amount to decrement. */ /**********************************************************//** Returns the old value of *ptr, atomically sets *ptr to new_val */ -# define os_atomic_test_and_set_byte(ptr, new_val) \ +# define os_atomic_test_and_set_byte_acquire(ptr, new_val) \ + atomic_swap_uchar(ptr, new_val) +# define os_atomic_test_and_set_byte_release(ptr, new_val) \ atomic_swap_uchar(ptr, new_val) # define os_atomic_test_and_set_ulint(ptr, new_val) \ atomic_swap_ulong(ptr, new_val) -# define os_atomic_lock_release_byte(ptr) \ - (void) atomic_swap_uchar(ptr, 0) - #elif defined(HAVE_WINDOWS_ATOMICS) # define HAVE_ATOMIC_BUILTINS @@ -644,15 +692,14 @@ Returns the old value of *ptr, atomicall InterlockedExchange() operates on LONG, and the LONG will be clobbered */ -# define os_atomic_test_and_set_byte(ptr, new_val) \ +# define os_atomic_test_and_set_byte_acquire(ptr, new_val) \ + ((byte) InterlockedExchange(ptr, new_val)) +# define os_atomic_test_and_set_byte_release(ptr, new_val) \ ((byte) InterlockedExchange(ptr, new_val)) # define os_atomic_test_and_set_ulong(ptr, new_val) \ InterlockedExchange(ptr, new_val) -# define os_atomic_lock_release_byte(ptr) \ - (void) InterlockedExchange(ptr, 0) - #else # define IB_ATOMICS_STARTUP_MSG \ "Mutexes and rw_locks use InnoDB's own implementation" @@ -712,6 +759,7 @@ architecture. Disable memory barrier for # define HAVE_MEMORY_BARRIER # define os_rmb __atomic_thread_fence(__ATOMIC_ACQUIRE) # define os_wmb __atomic_thread_fence(__ATOMIC_RELEASE) +# define os_mb __atomic_thread_fence(__ATOMIC_SEQ_CST) # define IB_MEMORY_BARRIER_STARTUP_MSG \ "GCC builtin __atomic_thread_fence() is used for memory barrier" @@ -727,6 +775,7 @@ architecture. Disable memory barrier for # include # define os_rmb __machine_r_barrier() # define os_wmb __machine_w_barrier() +# define os_mb __machine_rw_barrier() # define IB_MEMORY_BARRIER_STARTUP_MSG \ "Solaris memory ordering functions are used for memory barrier" @@ -735,12 +784,14 @@ architecture. Disable memory barrier for # include # define os_rmb _mm_lfence() # define os_wmb _mm_sfence() +# define os_mb _mm_mfence() # define IB_MEMORY_BARRIER_STARTUP_MSG \ "_mm_lfence() and _mm_sfence() are used for memory barrier" #else # define os_rmb # define os_wmb +# define os_mb # define IB_MEMORY_BARRIER_STARTUP_MSG \ "Memory barrier is not used" #endif Index: mysql-5.6/storage/innobase/include/os0sync.ic =================================================================== --- mysql-5.6.orig/storage/innobase/include/os0sync.ic +++ mysql-5.6/storage/innobase/include/os0sync.ic @@ -51,6 +51,35 @@ os_fast_mutex_trylock( #endif } + +/**********************************************************//** +Acquires ownership of a fast mutex. Implies a full memory barrier even on +platforms such as PowerPC where this is not normally required. +@return 0 if success, != 0 if was reserved by another thread */ +UNIV_INLINE +ulint +os_fast_mutex_trylock_full_barrier( +/*==================*/ + os_fast_mutex_t* fast_mutex) /*!< in: mutex to acquire */ +{ + fast_mutex_t* mutex = &fast_mutex->mutex; + +#ifdef __WIN__ + return(!TryEnterCriticalSection(mutex)); +#else + /* NOTE that the MySQL my_pthread.h redefines pthread_mutex_trylock + so that it returns 0 on success. In the operating system + libraries, HP-UX-10.20 follows the old Posix 1003.4a Draft 4 and + returns 1 on success (but MySQL remaps that to 0), while Linux, + FreeBSD, Solaris, AIX, Tru64 Unix, HP-UX-11.0 return 0 on success. */ + +#ifdef __powerpc__ + os_mb; +#endif + return((ulint) pthread_mutex_trylock(mutex)); +#endif +} + #ifdef UNIV_PFS_MUTEX /*********************************************************//** NOTE! Please use the corresponding macro os_fast_mutex_init(), not directly @@ -148,6 +177,25 @@ pfs_os_fast_mutex_unlock( os_fast_mutex_unlock_func(&fast_mutex->mutex); } +/**********************************************************//** +NOTE! Please use the corresponding macro os_fast_mutex_unlock_full_barrier, not directly +this function! +Wrapper function of os_fast_mutex_unlock_full_barrier_func. Releases ownership of a +fast mutex. Implies a full memory barrier even on +platforms such as PowerPC where this is not normally required.*/ +UNIV_INLINE +void +pfs_os_fast_mutex_unlock_full_barrier( +/*=====================*/ + os_fast_mutex_t* fast_mutex) /*!< in/out: mutex to release */ +{ +#ifdef HAVE_PSI_MUTEX_INTERFACE + if (fast_mutex->pfs_psi != NULL) + PSI_MUTEX_CALL(unlock_mutex)(fast_mutex->pfs_psi); +#endif + + os_fast_mutex_unlock_full_barrier_func(&fast_mutex->mutex); +} #endif /* UNIV_PFS_MUTEX */ #ifdef HAVE_WINDOWS_ATOMICS Index: mysql-5.6/storage/innobase/include/sync0sync.ic =================================================================== --- mysql-5.6.orig/storage/innobase/include/sync0sync.ic +++ mysql-5.6/storage/innobase/include/sync0sync.ic @@ -81,14 +81,14 @@ ib_mutex_test_and_set( { #if defined(HAVE_ATOMIC_BUILTINS) # if defined(HAVE_ATOMIC_BUILTINS_BYTE) - return(os_atomic_test_and_set_byte(&mutex->lock_word, 1)); + return(os_atomic_test_and_set_byte_acquire(&mutex->lock_word, 1)); # else return(os_atomic_test_and_set_ulint(&mutex->lock_word, 1)); # endif #else ibool ret; - ret = os_fast_mutex_trylock(&(mutex->os_fast_mutex)); + ret = os_fast_mutex_trylock_full_barrier(&(mutex->os_fast_mutex)); if (ret == 0) { /* We check that os_fast_mutex_trylock does not leak @@ -96,7 +96,6 @@ ib_mutex_test_and_set( ut_a(mutex->lock_word == 0); mutex->lock_word = 1; - os_wmb; } return((byte) ret); @@ -114,14 +113,17 @@ mutex_reset_lock_word( { #if defined(HAVE_ATOMIC_BUILTINS) # if defined(HAVE_ATOMIC_BUILTINS_BYTE) - os_atomic_lock_release_byte(&mutex->lock_word); + /* In theory __sync_lock_release should be used to release the lock. + Unfortunately, it does not work properly alone. The workaround is + that more conservative __sync_lock_test_and_set is used instead. */ + os_atomic_test_and_set_byte_release(&mutex->lock_word, 0); # else os_atomic_test_and_set_ulint(&mutex->lock_word, 0); # endif #else mutex->lock_word = 0; - os_fast_mutex_unlock(&(mutex->os_fast_mutex)); + os_fast_mutex_unlock_full_barrier(&(mutex->os_fast_mutex)); #endif } Index: mysql-5.6/storage/innobase/os/os0sync.cc =================================================================== --- mysql-5.6.orig/storage/innobase/os/os0sync.cc +++ mysql-5.6/storage/innobase/os/os0sync.cc @@ -890,6 +890,25 @@ os_fast_mutex_unlock_func( } /**********************************************************//** +Releases ownership of a fast mutex. Implies a full memory barrier even on +platforms such as PowerPC where this is not normally required. */ +UNIV_INTERN +void +os_fast_mutex_unlock_full_barrier_func( +/*=================*/ + fast_mutex_t* fast_mutex) /*!< in: mutex to release */ +{ +#ifdef __WIN__ + LeaveCriticalSection(fast_mutex); +#else + pthread_mutex_unlock(fast_mutex); +#ifdef __powerpc__ + os_mb; +#endif +#endif +} + +/**********************************************************//** Frees a mutex object. */ UNIV_INTERN void Index: mysql-5.6/storage/innobase/sync/sync0sync.cc =================================================================== --- mysql-5.6.orig/storage/innobase/sync/sync0sync.cc +++ mysql-5.6/storage/innobase/sync/sync0sync.cc @@ -458,7 +458,6 @@ mutex_set_waiters( *ptr = n; /* Here we assume that the write of a single word in memory is atomic */ - os_wmb; } /******************************************************************//**