[Commits] Rev 4346: MDEV-7026 - Occasional hang during startup on Power8 in lp:maria/5.5

Sergey Vojtovich svoj at mariadb.org
Tue Nov 18 10:11:13 EET 2014


At lp:maria/5.5

------------------------------------------------------------
revno: 4346
revision-id: svoj at mariadb.org-20141118080742-sdf93kgsgxulenvb
parent: knielsen at knielsen-hq.org-20141112101013-t63ayylsk08ncgs3
committer: Sergey Vojtovich <svoj at mariadb.org>
branch nick: 5.5
timestamp: Tue 2014-11-18 12:07:42 +0400
message:
  MDEV-7026 - Occasional hang during startup on Power8
  
  In MariaDB 5.5.40 and 10.0.13, the InnoDB/XtraDB low-level mutex
  implementation was inadvertently broken, so that a waiter may miss the wakeup
  when another thread releases the mutex. This affects at least x86 and amd64
  architectures. This could result in threads occasionally stalling for about 1
  second, or in some cases even hanging the whole server infinitely.
  
  To avoid this hang we need to ensure proper StoreLoad order between lock_word
  and waiters flags. They must appear in the following order:
  
  mutex_enter()
  {
    ...
    waiters= 1;
    if (lock_word == 1)
    ...
  }
  
  mutex_exit()
  {
    ...
    lock_word= 0;
    if (waiters)
    ...
  }
  
  There was an attempt to ensure this order on PPC64 by "isync":::"memory", but
  it is just instruction synchronization + compiler memory barrier. It is not HW
  memory barrier.
  
  Since isync is not a memory barrier and lwsync doesn't guarantee StoreLoad
  order, the only option we have is full memory barrier.
=== modified file 'storage/innobase/include/os0sync.h'
--- a/storage/innobase/include/os0sync.h	2014-09-08 15:10:48 +0000
+++ b/storage/innobase/include/os0sync.h	2014-11-18 08:07:42 +0000
@@ -310,8 +310,16 @@ Returns the old value of *ptr, atomicall
 # define os_atomic_test_and_set_byte(ptr, new_val) \
 	__sync_lock_test_and_set(ptr, (byte) new_val)
 
+#ifdef __powerpc__
 # define os_atomic_lock_release_byte(ptr) \
 	__sync_lock_release(ptr)
+#else
+/* 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. */
+# define os_atomic_lock_release_byte(ptr) \
+	__sync_lock_test_and_set(ptr, 0)
+#endif
 
 #elif defined(HAVE_IB_SOLARIS_ATOMICS)
 
@@ -428,7 +436,7 @@ clobbered */
 # define os_rmb	__atomic_thread_fence(__ATOMIC_ACQUIRE)
 # define os_wmb	__atomic_thread_fence(__ATOMIC_RELEASE)
 #ifdef __powerpc__
-# define os_isync  __asm __volatile ("isync":::"memory")
+# define os_isync __atomic_thread_fence(__ATOMIC_SEQ_CST)
 #else
 #define os_isync do { } while(0)
 #endif

=== modified file 'storage/innobase/sync/sync0sync.c'
--- a/storage/innobase/sync/sync0sync.c	2014-11-03 13:43:44 +0000
+++ b/storage/innobase/sync/sync0sync.c	2014-11-18 08:07:42 +0000
@@ -474,10 +474,9 @@ mutex_set_waiters(
 
 	ptr = &(mutex->waiters);
 
-        os_wmb;
-
 	*ptr = n;		/* Here we assume that the write of a single
 				word in memory is atomic */
+        os_isync;
 }
 
 /******************************************************************//**

=== modified file 'storage/xtradb/include/os0sync.h'
--- a/storage/xtradb/include/os0sync.h	2014-09-08 15:10:48 +0000
+++ b/storage/xtradb/include/os0sync.h	2014-11-18 08:07:42 +0000
@@ -317,8 +317,16 @@ Returns the old value of *ptr, atomicall
 # define os_atomic_test_and_set_byte(ptr, new_val) \
 	__sync_lock_test_and_set(ptr, (byte) new_val)
 
+#ifdef __powerpc__
 # define os_atomic_lock_release_byte(ptr) \
 	__sync_lock_release(ptr)
+#else
+/* 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. */
+# define os_atomic_lock_release_byte(ptr) \
+	__sync_lock_test_and_set(ptr, 0)
+#endif
 
 #elif defined(HAVE_IB_SOLARIS_ATOMICS)
 
@@ -448,7 +456,7 @@ clobbered */
 # define os_rmb	__atomic_thread_fence(__ATOMIC_ACQUIRE)
 # define os_wmb	__atomic_thread_fence(__ATOMIC_RELEASE)
 #ifdef __powerpc__
-# define os_isync  __asm __volatile ("isync":::"memory")
+# define os_isync __atomic_thread_fence(__ATOMIC_SEQ_CST)
 #else
 #define os_isync do { } while(0)
 #endif

=== modified file 'storage/xtradb/sync/sync0sync.c'
--- a/storage/xtradb/sync/sync0sync.c	2014-11-03 13:43:44 +0000
+++ b/storage/xtradb/sync/sync0sync.c	2014-11-18 08:07:42 +0000
@@ -482,10 +482,9 @@ mutex_set_waiters(
 
 	ptr = &(mutex->waiters);
 
-        os_wmb;
-
 	*ptr = n;		/* Here we assume that the write of a single
 				word in memory is atomic */
+	os_isync;
 #endif
 }
 



More information about the commits mailing list