Remove swpb-based spinlock implementation for ARMv5 and earlier.

Per recent analysis by Andres Freund, this implementation is in fact
unsafe, because ARMv5 has weak memory ordering, which means tha the
CPU could move loads or stores across the volatile store performed by
the default S_UNLOCK.  We could try to fix this, but have no ARMv5
hardware to test on, so removing support seems better.  We can still
support ARMv5 systems on GCC versions new enough to have built-in
atomics support for this platform, and can also re-add support for
the old way if someone has hardware that can be used to test a fix.
However, since the requirement to use a relatively-new GCC hasn't
been an issue for ARMv6 or ARMv7, which lack the swpb instruction
altogether, perhaps it won't be an issue for ARMv5 either.

diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h
index 895abe672b836bc2ef15d5ca9bac1820ebeb7f4b..849651ed02c75540796dc44f14d3201c45135ef0 100644 (file)
--- a/src/include/storage/s_lock.h
+++ b/src/include/storage/s_lock.h
@@ -300,55 +300,13 @@ tas(volatile slock_t *lock)
 #endif /* __INTEL_COMPILER */
 #endif  /* __ia64__ || __ia64 */
 
-
 /*
- * On ARM, we use __sync_lock_test_and_set(int *, int) if available, and if
- * not fall back on the SWPB instruction.  SWPB does not work on ARMv6 or
- * later, so the compiler builtin is preferred if available.  Note also that
- * the int-width variant of the builtin works on more chips than other widths.
- */
-#if defined(__arm__) || defined(__arm)
-#define HAS_TEST_AND_SET
-
-#define TAS(lock) tas(lock)
-
-#ifdef HAVE_GCC_INT_ATOMICS
-
-typedef int slock_t;
-
-static __inline__ int
-tas(volatile slock_t *lock)
-{
-       return __sync_lock_test_and_set(lock, 1);
-}
-
-#define S_UNLOCK(lock) __sync_lock_release(lock)
-
-#else /* !HAVE_GCC_INT_ATOMICS */
-
-typedef unsigned char slock_t;
-
-static __inline__ int
-tas(volatile slock_t *lock)
-{
-       register slock_t _res = 1;
-
-       __asm__ __volatile__(
-               "       swpb    %0, %0, [%2]    \n"
-:              "+r"(_res), "+m"(*lock)
-:              "r"(lock)
-:              "memory");
-       return (int) _res;
-}
-
-#endif  /* HAVE_GCC_INT_ATOMICS */
-#endif  /* __arm__ */
-
-
-/*
- * On ARM64, we use __sync_lock_test_and_set(int *, int) if available.
+ * On ARM and ARM64, we use __sync_lock_test_and_set(int *, int) if available.
+ *
+ * We use the int-width variant of the builtin because it works on more chips
+ * than other widths.
  */
-#if defined(__aarch64__) || defined(__aarch64)
+#if defined(__arm__) || defined(__arm) || defined(__aarch64__) || defined(__aarch64)
 #ifdef HAVE_GCC_INT_ATOMICS
 #define HAS_TEST_AND_SET
 
@@ -365,7 +323,7 @@ tas(volatile slock_t *lock)
 #define S_UNLOCK(lock) __sync_lock_release(lock)
 
 #endif  /* HAVE_GCC_INT_ATOMICS */
-#endif  /* __aarch64__ */
+#endif  /* __arm__ || __arm || __aarch64__ || __aarch64 */
 
 
 /* S/390 and S/390x Linux (32- and 64-bit zSeries) */