You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by ad...@apache.org on 2017/04/18 21:30:19 UTC
kudu git commit: Revert "gutil: stop supporting AMD Opteron K8"
Repository: kudu
Updated Branches:
refs/heads/master 5132624f2 -> b5592dc3d
Revert "gutil: stop supporting AMD Opteron K8"
This reverts commit b5004e1886eb96839ec120da582f0a06ce869db2.
This change broke the release build on el6:
../../../lib/libgutil.a(atomicops-internals-x86.cc.o): In function `AtomicOps_x86CPUFeaturesInit()':
<src/kudu/gutil/atomicops-internals-x86.cc>:122: multiple definition of `AtomicOps_Internalx86CPUFeatures'
../../../../../thirdparty/installed/uninstrumented/lib64/libtcmalloc.a(atomicops-internals-x86.o):(.bss+0x0): first defined here
/opt/rh/devtoolset-3/root/usr/libexec/gcc/x86_64-redhat-linux/4.9.2/ld: Warning: size of symbol `AtomicOps_Internalx86CPUFeatures' changed from 3 in ../../../../../thirdparty/installed/uninstrumented/lib64/libtcmalloc.a(atomicops-internals-x86.o) to 2 in ../../../lib/libgutil.a(atomicops-internals-x86.cc.o)
Change-Id: I898fe74cf80b0f91f1c88700572b07e131227901
Reviewed-on: http://gerrit.cloudera.org:8080/6676
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/b5592dc3
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/b5592dc3
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/b5592dc3
Branch: refs/heads/master
Commit: b5592dc3df630f8ece857ea5444943c9e1beec33
Parents: 5132624
Author: Todd Lipcon <to...@apache.org>
Authored: Tue Apr 18 13:34:59 2017 -0700
Committer: Adar Dembo <ad...@cloudera.com>
Committed: Tue Apr 18 21:29:42 2017 +0000
----------------------------------------------------------------------
src/kudu/gutil/atomicops-internals-tsan.h | 2 ++
src/kudu/gutil/atomicops-internals-x86.cc | 14 +++++++-------
src/kudu/gutil/atomicops-internals-x86.h | 26 ++++++++++++++++++++++++++
src/kudu/util/init.cc | 4 ----
4 files changed, 35 insertions(+), 11 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/b5592dc3/src/kudu/gutil/atomicops-internals-tsan.h
----------------------------------------------------------------------
diff --git a/src/kudu/gutil/atomicops-internals-tsan.h b/src/kudu/gutil/atomicops-internals-tsan.h
index be9c144..aecaefc 100644
--- a/src/kudu/gutil/atomicops-internals-tsan.h
+++ b/src/kudu/gutil/atomicops-internals-tsan.h
@@ -19,6 +19,8 @@
// Features of this x86. Values may not be correct before main() is run,
// but are set conservatively.
struct AtomicOps_x86CPUFeatureStruct {
+ bool has_amd_lock_mb_bug; // Processor has AMD memory-barrier bug; do lfence
+ // after acquire compare-and-swap.
bool has_sse2; // Processor has SSE2.
};
BASE_EXPORT extern struct AtomicOps_x86CPUFeatureStruct
http://git-wip-us.apache.org/repos/asf/kudu/blob/b5592dc3/src/kudu/gutil/atomicops-internals-x86.cc
----------------------------------------------------------------------
diff --git a/src/kudu/gutil/atomicops-internals-x86.cc b/src/kudu/gutil/atomicops-internals-x86.cc
index 9650847..5d4529e 100644
--- a/src/kudu/gutil/atomicops-internals-x86.cc
+++ b/src/kudu/gutil/atomicops-internals-x86.cc
@@ -60,6 +60,7 @@
// Set the flags so that code will run correctly and conservatively
// until InitGoogle() is called.
struct AtomicOps_x86CPUFeatureStruct AtomicOps_Internalx86CPUFeatures = {
+ false, // bug can't exist before process spawns multiple threads
false, // no SSE2
false, // no cmpxchg16b
};
@@ -94,16 +95,12 @@ static void AtomicOps_Internalx86CPUFeaturesInit() {
// non-locked read-modify-write instruction. Rev F has this bug in
// pre-release versions, but not in versions released to customers,
// so we test only for Rev E, which is family 15, model 32..63 inclusive.
- //
- // Opteron Rev E is the first-generation Opteron (models 1xx, 2xx, 8xx)
- // also known as "K8", released in 2003. These processors are old enough
- // that we can just drop support for them instead of trying to work around
- // the above bug.
if (strcmp(vendor, "AuthenticAMD") == 0 && // AMD
family == 15 &&
32 <= model && model <= 63) {
- LOG(FATAL) << "AMD Family 0fh model 32 through 63 not supported due to "
- << "buggy atomic operations.";
+ AtomicOps_Internalx86CPUFeatures.has_amd_lock_mb_bug = true;
+ } else {
+ AtomicOps_Internalx86CPUFeatures.has_amd_lock_mb_bug = false;
}
// edx bit 26 is SSE2 which we use to tell use whether we can use mfence
@@ -111,9 +108,12 @@ static void AtomicOps_Internalx86CPUFeaturesInit() {
// ecx bit 13 indicates whether the cmpxchg16b instruction is supported
AtomicOps_Internalx86CPUFeatures.has_cmpxchg16b = ((ecx >> 13) & 1);
+
VLOG(1) << "vendor " << vendor <<
" family " << family <<
" model " << model <<
+ " amd_lock_mb_bug " <<
+ AtomicOps_Internalx86CPUFeatures.has_amd_lock_mb_bug <<
" sse2 " << AtomicOps_Internalx86CPUFeatures.has_sse2 <<
" cmpxchg16b " << AtomicOps_Internalx86CPUFeatures.has_cmpxchg16b;
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/b5592dc3/src/kudu/gutil/atomicops-internals-x86.h
----------------------------------------------------------------------
diff --git a/src/kudu/gutil/atomicops-internals-x86.h b/src/kudu/gutil/atomicops-internals-x86.h
index c24907e..acbd2e3 100644
--- a/src/kudu/gutil/atomicops-internals-x86.h
+++ b/src/kudu/gutil/atomicops-internals-x86.h
@@ -43,6 +43,8 @@
// Features of this x86. Values may not be correct before InitGoogle() is run,
// but are set conservatively.
struct AtomicOps_x86CPUFeatureStruct {
+ bool has_amd_lock_mb_bug; // Processor has AMD memory-barrier bug; do lfence
+ // after acquire compare-and-swap.
bool has_sse2; // Processor has SSE2.
bool has_cmpxchg16b; // Processor supports cmpxchg16b instruction.
};
@@ -100,6 +102,9 @@ inline Atomic32 Acquire_AtomicExchange(volatile Atomic32* ptr,
Atomic32 new_value) {
CheckNaturalAlignment(ptr);
Atomic32 old_val = NoBarrier_AtomicExchange(ptr, new_value);
+ if (AtomicOps_Internalx86CPUFeatures.has_amd_lock_mb_bug) {
+ __asm__ __volatile__("lfence" : : : "memory");
+ }
return old_val;
}
@@ -127,6 +132,9 @@ inline Atomic32 Barrier_AtomicIncrement(volatile Atomic32* ptr,
: "+r" (temp), "+m" (*ptr)
: : "memory");
// temp now holds the old value of *ptr
+ if (AtomicOps_Internalx86CPUFeatures.has_amd_lock_mb_bug) {
+ __asm__ __volatile__("lfence" : : : "memory");
+ }
return temp + increment;
}
@@ -134,6 +142,9 @@ inline Atomic32 Acquire_CompareAndSwap(volatile Atomic32* ptr,
Atomic32 old_value,
Atomic32 new_value) {
Atomic32 x = NoBarrier_CompareAndSwap(ptr, old_value, new_value);
+ if (AtomicOps_Internalx86CPUFeatures.has_amd_lock_mb_bug) {
+ __asm__ __volatile__("lfence" : : : "memory");
+ }
return x;
}
@@ -246,6 +257,9 @@ inline Atomic64 NoBarrier_AtomicExchange(volatile Atomic64* ptr,
inline Atomic64 Acquire_AtomicExchange(volatile Atomic64* ptr,
Atomic64 new_value) {
Atomic64 old_val = NoBarrier_AtomicExchange(ptr, new_value);
+ if (AtomicOps_Internalx86CPUFeatures.has_amd_lock_mb_bug) {
+ __asm__ __volatile__("lfence" : : : "memory");
+ }
return old_val;
}
@@ -273,6 +287,9 @@ inline Atomic64 Barrier_AtomicIncrement(volatile Atomic64* ptr,
: "+r" (temp), "+m" (*ptr)
: : "memory");
// temp now contains the previous value of *ptr
+ if (AtomicOps_Internalx86CPUFeatures.has_amd_lock_mb_bug) {
+ __asm__ __volatile__("lfence" : : : "memory");
+ }
return temp + increment;
}
@@ -386,6 +403,9 @@ inline Atomic64 Acquire_AtomicExchange(volatile Atomic64* ptr,
Atomic64 new_val) {
CheckNaturalAlignment(ptr);
Atomic64 old_val = NoBarrier_AtomicExchange(ptr, new_val);
+ if (AtomicOps_Internalx86CPUFeatures.has_amd_lock_mb_bug) {
+ __asm__ __volatile__("lfence" : : : "memory");
+ }
return old_val;
}
@@ -411,6 +431,9 @@ inline Atomic64 Barrier_AtomicIncrement(volatile Atomic64* ptr,
Atomic64 increment) {
CheckNaturalAlignment(ptr);
Atomic64 new_val = NoBarrier_AtomicIncrement(ptr, increment);
+ if (AtomicOps_Internalx86CPUFeatures.has_amd_lock_mb_bug) {
+ __asm__ __volatile__("lfence" : : : "memory");
+ }
return new_val;
}
@@ -470,6 +493,9 @@ inline Atomic64 Acquire_CompareAndSwap(volatile Atomic64* ptr,
Atomic64 old_value,
Atomic64 new_value) {
Atomic64 x = NoBarrier_CompareAndSwap(ptr, old_value, new_value);
+ if (AtomicOps_Internalx86CPUFeatures.has_amd_lock_mb_bug) {
+ __asm__ __volatile__("lfence" : : : "memory");
+ }
return x;
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/b5592dc3/src/kudu/util/init.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/init.cc b/src/kudu/util/init.cc
index 45640f4..3fa634a 100644
--- a/src/kudu/util/init.cc
+++ b/src/kudu/util/init.cc
@@ -22,7 +22,6 @@
#include <string>
-#include "kudu/gutil/atomicops.h"
#include "kudu/gutil/cpu.h"
#include "kudu/gutil/strings/substitute.h"
#include "kudu/util/status.h"
@@ -80,9 +79,6 @@ Status CheckCPUFlags() {
void InitKuduOrDie() {
CheckStandardFds();
CHECK_OK(CheckCPUFlags());
-#ifndef THREAD_SANITIZER // Thread Sanitizer uses its own special atomicops implementation.
- AtomicOps_x86CPUFeaturesInit();
-#endif
// NOTE: this function is called before flags are parsed.
// Do not add anything in here which is flag-dependent.
}