From fc3d31683a0120ba005f45f98dcbe1001064dafb Mon Sep 17 00:00:00 2001 From: Andy McFadden Date: Thu, 5 Aug 2010 14:34:26 -0700 Subject: [PATCH] More SMP fixes. Convert some ANDROID_MEMBAR_FULL uses into equivalent atomic ops. A couple of "bool" had to convert to "int" since we don't have atomic ops for bools. Replaced a local implementation of atomic inc with a call to the atomic inc function. Change-Id: I948b8080d743552bde014d3a6e716ed2c30ebef8 --- vm/AtomicCache.c | 17 +++++++++-------- vm/AtomicCache.h | 14 ++++++-------- vm/Globals.h | 2 +- vm/Profile.c | 4 +--- vm/Profile.h | 2 +- vm/compiler/codegen/arm/Assemble.c | 4 ++-- vm/interp/Interp.c | 8 ++++++++ vm/jdwp/JdwpMain.c | 3 +-- vm/jdwp/JdwpPriv.h | 2 +- vm/native/InternalNative.c | 3 ++- vm/oo/Class.c | 11 +---------- 11 files changed, 33 insertions(+), 37 deletions(-) diff --git a/vm/AtomicCache.c b/vm/AtomicCache.c index bf8639e44..a6f48c2c7 100644 --- a/vm/AtomicCache.c +++ b/vm/AtomicCache.c @@ -135,24 +135,25 @@ void dvmUpdateAtomicCache(u4 key1, u4 key2, u4 value, AtomicCacheEntry* pEntry, * while we work. We use memory barriers to ensure that the state * is always consistent when the version number is even. */ - pEntry->version++; - ANDROID_MEMBAR_FULL(); + u4 newVersion = (firstVersion | ATOMIC_LOCK_FLAG) + 1; + assert((newVersion & 0x01) == 1); - pEntry->key1 = key1; + pEntry->version = newVersion; + + android_atomic_release_store(key1, (int32_t*) &pEntry->key1); pEntry->key2 = key2; pEntry->value = value; - ANDROID_MEMBAR_FULL(); - pEntry->version++; + newVersion++; + android_atomic_release_store(newVersion, (int32_t*) &pEntry->version); /* * Clear the lock flag. Nobody else should have been able to modify * pEntry->version, so if this fails the world is broken. */ - firstVersion += 2; - assert((firstVersion & 0x01) == 0); + assert(newVersion == ((firstVersion + 2) | ATOMIC_LOCK_FLAG)); if (android_atomic_release_cas( - firstVersion | ATOMIC_LOCK_FLAG, firstVersion, + newVersion, newVersion & ~ATOMIC_LOCK_FLAG, (volatile s4*) &pEntry->version) != 0) { //LOGE("unable to reset the instanceof cache ownership\n"); diff --git a/vm/AtomicCache.h b/vm/AtomicCache.h index 66d222e20..686eac5a3 100644 --- a/vm/AtomicCache.h +++ b/vm/AtomicCache.h @@ -95,28 +95,26 @@ typedef struct AtomicCache { #define ATOMIC_CACHE_LOOKUP(_cache, _cacheSize, _key1, _key2) ({ \ AtomicCacheEntry* pEntry; \ int hash; \ - u4 firstVersion; \ + u4 firstVersion, secondVersion; \ u4 value; \ \ /* simple hash function */ \ hash = (((u4)(_key1) >> 2) ^ (u4)(_key2)) & ((_cacheSize)-1); \ pEntry = (_cache)->entries + hash; \ \ - firstVersion = pEntry->version; \ - ANDROID_MEMBAR_FULL(); \ + firstVersion = android_atomic_acquire_load((int32_t*)&pEntry->version); \ \ if (pEntry->key1 == (u4)(_key1) && pEntry->key2 == (u4)(_key2)) { \ /* \ * The fields match. Get the value, then read the version a \ * second time to verify that we didn't catch a partial update. \ * We're also hosed if "firstVersion" was odd, indicating that \ - * an update was in progress before we got here. \ + * an update was in progress before we got here (unlikely). \ */ \ - value = pEntry->value; \ - ANDROID_MEMBAR_FULL(); \ + value = android_atomic_acquire_load((int32_t*) &pEntry->value); \ + secondVersion = pEntry->version; \ \ - if ((firstVersion & 0x01) != 0 || firstVersion != pEntry->version) \ - { \ + if ((firstVersion & 0x01) != 0 || firstVersion != secondVersion) { \ /* \ * We clashed with another thread. Instead of sitting and \ * spinning, which might not complete if we're a high priority \ diff --git a/vm/Globals.h b/vm/Globals.h index 90e9e88c2..954bb5d3f 100644 --- a/vm/Globals.h +++ b/vm/Globals.h @@ -296,7 +296,7 @@ struct DvmGlobals { //int offJavaNioDirectByteBufferImpl_pointer; /* method pointers - java.security.AccessController */ - volatile bool javaSecurityAccessControllerReady; + volatile int javaSecurityAccessControllerReady; Method* methJavaSecurityAccessController_doPrivileged[4]; /* constructor method pointers; no vtable involved, so use Method* */ diff --git a/vm/Profile.c b/vm/Profile.c index 57c96dff1..92d4001f1 100644 --- a/vm/Profile.c +++ b/vm/Profile.c @@ -407,13 +407,11 @@ void dvmMethodTraceStart(const char* traceFileName, int traceFd, int bufferSize, storeLongLE(state->buf + 8, state->startWhen); state->curOffset = TRACE_HEADER_LEN; - ANDROID_MEMBAR_FULL(); - /* * Set the "enabled" flag. Once we do this, threads will wait to be * signaled before exiting, so we have to make sure we wake them up. */ - state->traceEnabled = true; + android_atomic_release_store(true, &state->traceEnabled); dvmUnlockMutex(&state->startStopLock); return; diff --git a/vm/Profile.h b/vm/Profile.h index a294f831b..afa529da8 100644 --- a/vm/Profile.h +++ b/vm/Profile.h @@ -53,7 +53,7 @@ typedef struct MethodTraceState { int bufferSize; int flags; - bool traceEnabled; + int traceEnabled; u1* buf; volatile int curOffset; u8 startWhen; diff --git a/vm/compiler/codegen/arm/Assemble.c b/vm/compiler/codegen/arm/Assemble.c index 5f54ebe0d..94e2d1cb8 100644 --- a/vm/compiler/codegen/arm/Assemble.c +++ b/vm/compiler/codegen/arm/Assemble.c @@ -1490,8 +1490,8 @@ static void inlineCachePatchEnqueue(PredictedChainingCell *cellAddr, * The update order matters - make sure clazz is updated last since it * will bring the uninitialized chaining cell to life. */ - ANDROID_MEMBAR_FULL(); - cellAddr->clazz = newContent->clazz; + android_atomic_release_store((int32_t)newContent->clazz, + (void*) &cellAddr->clazz); cacheflush((intptr_t) cellAddr, (intptr_t) (cellAddr+1), 0); UPDATE_CODE_CACHE_PATCHES(); diff --git a/vm/interp/Interp.c b/vm/interp/Interp.c index ab6188232..d3de7305b 100644 --- a/vm/interp/Interp.c +++ b/vm/interp/Interp.c @@ -450,6 +450,14 @@ void dvmClearBreakAddr(Method* method, unsigned int instrOffset) #ifdef WITH_DEBUGGER /* * Get the original opcode from under a breakpoint. + * + * On SMP hardware it's possible one core might try to execute a breakpoint + * after another core has cleared it. We need to handle the case where + * there's no entry in the breakpoint set. (The memory barriers in the + * locks and in the breakpoint update code should ensure that, once we've + * observed the absence of a breakpoint entry, we will also now observe + * the restoration of the original opcode. The fact that we're holding + * the lock prevents other threads from confusing things further.) */ u1 dvmGetOriginalOpCode(const u2* addr) { diff --git a/vm/jdwp/JdwpMain.c b/vm/jdwp/JdwpMain.c index 1688e5e7b..24e5c6c3c 100644 --- a/vm/jdwp/JdwpMain.c +++ b/vm/jdwp/JdwpMain.c @@ -231,8 +231,7 @@ static void* jdwpThreadStart(void* arg) */ state->debugThreadHandle = dvmThreadSelf()->handle; state->run = true; - ANDROID_MEMBAR_FULL(); - state->debugThreadStarted = true; // touch this last + android_atomic_release_store(true, &state->debugThreadStarted); dvmDbgLockMutex(&state->threadStartLock); dvmDbgCondBroadcast(&state->threadStartCond); diff --git a/vm/jdwp/JdwpPriv.h b/vm/jdwp/JdwpPriv.h index 85f9ec235..bc249f10f 100644 --- a/vm/jdwp/JdwpPriv.h +++ b/vm/jdwp/JdwpPriv.h @@ -79,7 +79,7 @@ struct JdwpState { pthread_mutex_t threadStartLock; pthread_cond_t threadStartCond; - bool debugThreadStarted; + int debugThreadStarted; pthread_t debugThreadHandle; ObjectId debugThreadId; bool run; diff --git a/vm/native/InternalNative.c b/vm/native/InternalNative.c index 700c8afe6..62d4d04ae 100644 --- a/vm/native/InternalNative.c +++ b/vm/native/InternalNative.c @@ -335,7 +335,8 @@ bool dvmIsPrivilegedMethod(const Method* method) } /* all good, raise volatile readiness flag */ - gDvm.javaSecurityAccessControllerReady = true; + android_atomic_release_store(true, + &gDvm.javaSecurityAccessControllerReady); } for (i = 0; i < NUM_DOPRIV_FUNCS; i++) { diff --git a/vm/oo/Class.c b/vm/oo/Class.c index 1f6e68544..e0d002d5d 100644 --- a/vm/oo/Class.c +++ b/vm/oo/Class.c @@ -1123,17 +1123,8 @@ static void removeClassFromHash(ClassObject* clazz) */ void dvmSetClassSerialNumber(ClassObject* clazz) { - u4 oldValue, newValue; - assert(clazz->serialNumber == 0); - - do { - oldValue = gDvm.classSerialNumber; - newValue = oldValue + 1; - } while (android_atomic_release_cas(oldValue, newValue, - &gDvm.classSerialNumber) != 0); - - clazz->serialNumber = (u4) oldValue; + clazz->serialNumber = android_atomic_inc(&gDvm.classSerialNumber); } -- 2.11.0