BUG#21102971 data corruption on arm64

The root cause is that x86 has a stronger memory model than the ARM
processors. And the GCC builtins didn't issue the correct fences when
setting/unsetting the lock word. In particular during the mutex release.

The solution is rewriting atomic TAS operations: replace '__sync_' by
'__atomic_' if possible.

Reviewed-by: Sunny Bains      <sunny.bains@oracle.com>
Reviewed-by: Bin Su           <bin.x.su@oracle.com>
Reviewed-by: Debarun Banerjee <debarun.banerjee@oracle.com>
Reviewed-by: Krunal Bauskar   <krunal.bauskar@oracle.com>
RB: 9782
RB: 9665
RB: 9783

storage/innobase/CMakeLists.txt

@@ -1,4 +1,4 @@
-# Copyright (c) 2006, 2011, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2006, 2015, Oracle and/or its affiliates. All rights reserved.
 # 
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -85,12 +85,28 @@ IF(NOT CMAKE_CROSSCOMPILING)
   }"
   HAVE_IB_GCC_ATOMIC_BUILTINS
   )
+  CHECK_C_SOURCE_RUNS(
+  "#include<stdint.h>
+  int main()
+  {
+    unsigned char	c;
+
+    __atomic_test_and_set(&c, __ATOMIC_ACQUIRE);
+    __atomic_clear(&c, __ATOMIC_RELEASE);
+    return(0);
+  }"
+  HAVE_IB_GCC_ATOMIC_TEST_AND_SET
+  )
 ENDIF()
 
 IF(HAVE_IB_GCC_ATOMIC_BUILTINS)
  ADD_DEFINITIONS(-DHAVE_IB_GCC_ATOMIC_BUILTINS=1)
 ENDIF()
 
+IF(HAVE_IB_GCC_ATOMIC_TEST_AND_SET)
+ ADD_DEFINITIONS(-DHAVE_IB_GCC_ATOMIC_TEST_AND_SET=1)
+ENDIF()
+
  # either define HAVE_IB_ATOMIC_PTHREAD_T_GCC or not
 IF(NOT CMAKE_CROSSCOMPILING)
   CHECK_C_SOURCE_RUNS(

storage/innobase/include/os0sync.h

@@ -1,6 +1,6 @@
 /*****************************************************************************
 
-Copyright (c) 1995, 2009, Innobase Oy. All Rights Reserved.
+Copyright (c) 1995, 2015, Oracle and/or its affiliates. All Rights Reserved.
 Copyright (c) 2008, Google Inc.
 
 Portions of this file contain modifications contributed and copyrighted by
@@ -37,6 +37,20 @@ Created 9/6/1995 Heikki Tuuri
 #include "univ.i"
 #include "ut0lst.h"
 
+#if defined __i386__ || defined __x86_64__ || defined _M_IX86 \
+    || defined _M_X64 || defined __WIN__
+
+#define IB_STRONG_MEMORY_MODEL
+
+#endif /* __i386__ || __x86_64__ || _M_IX86 || M_X64 || __WIN__ */
+
+#ifdef HAVE_WINDOWS_ATOMICS
+typedef LONG lock_word_t;	/*!< On Windows, InterlockedExchange operates
+				on LONG variable */
+#else
+typedef byte lock_word_t;
+#endif
+
 #ifdef __WIN__
 /** Native event (slow)*/
 typedef HANDLE			os_native_event_t;
@@ -304,11 +318,61 @@ amount of increment. */
 # define os_atomic_increment_ulint(ptr, amount) \
 	os_atomic_increment(ptr, amount)
 
-/**********************************************************//**
-Returns the old value of *ptr, atomically sets *ptr to new_val */
-
-# define os_atomic_test_and_set_byte(ptr, new_val) \
-	__sync_lock_test_and_set(ptr, (byte) new_val)
+# if defined(HAVE_IB_GCC_ATOMIC_TEST_AND_SET)
+
+/** Do an atomic test-and-set.
+@param[in,out]	ptr		Memory location to set to non-zero
+@return the previous value */
+static inline
+lock_word_t
+os_atomic_test_and_set(volatile lock_word_t* ptr)
+{
+       return(__atomic_test_and_set(ptr, __ATOMIC_ACQUIRE));
+}
+
+/** Do an atomic clear.
+@param[in,out]	ptr		Memory location to set to zero */
+static inline
+void
+os_atomic_clear(volatile lock_word_t* ptr)
+{
+	__atomic_clear(ptr, __ATOMIC_RELEASE);
+}
+
+# elif defined(IB_STRONG_MEMORY_MODEL)
+
+/** Do an atomic test and set.
+@param[in,out]	ptr		Memory location to set to non-zero
+@return the previous value */
+static inline
+lock_word_t
+os_atomic_test_and_set(volatile lock_word_t* ptr)
+{
+	return(__sync_lock_test_and_set(ptr, 1));
+}
+
+/** Do an atomic release.
+
+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.
+
+Performance regression was observed at some conditions for Intel
+architecture. Disable release barrier on Intel architecture for now.
+@param[in,out]	ptr		Memory location to write to
+@return the previous value */
+static inline
+lock_word_t
+os_atomic_clear(volatile lock_word_t* ptr)
+{
+	return(__sync_lock_test_and_set(ptr, 0));
+}
+
+# else
+
+#  error "Unsupported platform"
+
+# endif /* HAVE_IB_GCC_ATOMIC_TEST_AND_SET */
 
 #elif defined(HAVE_IB_SOLARIS_ATOMICS)
 
@@ -357,11 +421,25 @@ amount of increment. */
 # define os_atomic_increment_ulint(ptr, amount) \
 	atomic_add_long_nv(ptr, amount)
 
-/**********************************************************//**
-Returns the old value of *ptr, atomically sets *ptr to new_val */
-
-# define os_atomic_test_and_set_byte(ptr, new_val) \
-	atomic_swap_uchar(ptr, new_val)
+/** Do an atomic xchg and set to non-zero.
+@param[in,out]	ptr		Memory location to set to non-zero
+@return the previous value */
+static inline
+lock_word_t
+os_atomic_test_and_set(volatile lock_word_t* ptr)
+{
+	return(atomic_swap_uchar(ptr, 1));
+}
+
+/** Do an atomic xchg and set to zero.
+@param[in,out]	ptr		Memory location to set to zero
+@return the previous value */
+static inline
+lock_word_t
+os_atomic_clear(volatile lock_word_t* ptr)
+{
+	return(atomic_swap_uchar(ptr, 0));
+}
 
 #elif defined(HAVE_WINDOWS_ATOMICS)
 
@@ -403,13 +481,27 @@ amount of increment. */
 # define os_atomic_increment_ulint(ptr, amount) \
 	((ulint) (win_xchg_and_add(ptr, amount) + amount))
 
-/**********************************************************//**
-Returns the old value of *ptr, atomically sets *ptr to new_val.
-InterlockedExchange() operates on LONG, and the LONG will be
-clobbered */
-
-# define os_atomic_test_and_set_byte(ptr, new_val) \
-	((byte) InterlockedExchange(ptr, new_val))
+/** Do an atomic test and set.
+InterlockedExchange() operates on LONG, and the LONG will be clobbered
+@param[in,out]	ptr		Memory location to set to non-zero
+@return the previous value */
+static inline
+lock_word_t
+os_atomic_test_and_set(volatile lock_word_t* ptr)
+{
+	return(InterlockedExchange(ptr, 1));
+}
+
+/** Do an atomic release.
+InterlockedExchange() operates on LONG, and the LONG will be clobbered
+@param[in,out]	ptr		Memory location to set to zero
+@return the previous value */
+static inline
+lock_word_t
+os_atomic_clear(volatile lock_word_t* ptr)
+{
+	return(InterlockedExchange(ptr, 0));
+}
 
 #else
 # define IB_ATOMICS_STARTUP_MSG \

storage/innobase/include/sync0sync.h

@@ -1,6 +1,6 @@
 /*****************************************************************************
 
-Copyright (c) 1995, 2012, Oracle and/or its affiliates. All Rights Reserved.
+Copyright (c) 1995, 2015, Oracle and/or its affiliates. All Rights Reserved.
 Copyright (c) 2008, Google Inc.
 
 Portions of this file contain modifications contributed and copyrighted by
@@ -45,13 +45,6 @@ Created 9/5/1995 Heikki Tuuri
 extern my_bool	timed_mutexes;
 #endif /* UNIV_DEBUG && !UNIV_HOTBACKUP */
 
-#ifdef HAVE_WINDOWS_ATOMICS
-typedef LONG lock_word_t;	/*!< On Windows, InterlockedExchange operates
-				on LONG variable */
-#else
-typedef byte lock_word_t;
-#endif
-
 #if defined UNIV_PFS_MUTEX || defined UNIV_PFS_RWLOCK
 /* There are mutexes/rwlocks that we want to exclude from
 instrumentation even if their corresponding performance schema

storage/innobase/include/sync0sync.ic

@@ -1,6 +1,6 @@
 /*****************************************************************************
 
-Copyright (c) 1995, 2009, Innobase Oy. All Rights Reserved.
+Copyright (c) 1995, 2015, Oracle and/or its affiliates. All Rights Reserved.
 Copyright (c) 2008, Google Inc.
 
 Portions of this file contain modifications contributed and copyrighted by
@@ -80,7 +80,7 @@ mutex_test_and_set(
 	mutex_t*	mutex)	/*!< in: mutex */
 {
 #if defined(HAVE_ATOMIC_BUILTINS)
-	return(os_atomic_test_and_set_byte(&mutex->lock_word, 1));
+	return(os_atomic_test_and_set(&mutex->lock_word));
 #else
 	ibool	ret;
 
@@ -108,10 +108,7 @@ mutex_reset_lock_word(
 	mutex_t*	mutex)	/*!< in: mutex */
 {
 #if defined(HAVE_ATOMIC_BUILTINS)
-	/* 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(&mutex->lock_word, 0);
+	os_atomic_clear(&mutex->lock_word);
 #else
 	mutex->lock_word = 0;