summaryrefslogtreecommitdiffstats
path: root/src
diff options
context:
space:
mode:
authorPatrick Williams <iawillia@us.ibm.com>2011-08-08 16:27:17 -0500
committerA. Patrick Williams III <iawillia@us.ibm.com>2011-08-09 13:54:57 -0500
commitbe61bd382972ec946396dadc161231c65ea9d968 (patch)
tree29acf3176ca910cd3f740d930b9d1fcee0c0e382 /src
parentaf0bcd44f7c475eb00178d4f448a14343c0354c0 (diff)
downloadtalos-hostboot-be61bd382972ec946396dadc161231c65ea9d968.tar.gz
talos-hostboot-be61bd382972ec946396dadc161231c65ea9d968.zip
Clean up mutex issues.
- Final fix for mutex bug. - Document weak-consistency decisions in mutex code. - Prevent aggressive optimizations around lwsync/isync instrs. - Fix minor bug in futex_wait system call. - Optimize futex path with likely/unlikely hints. Change-Id: I26b54dee7e45bcb42195f730474b350b44f53cfc Reviewed-on: http://gfw160.austin.ibm.com:8080/gerrit/233 Tested-by: Jenkins Server Reviewed-by: A. Patrick Williams III <iawillia@us.ibm.com>
Diffstat (limited to 'src')
-rw-r--r--src/include/arch/ppc.H9
-rw-r--r--src/kernel/futexmgr.C2
-rw-r--r--src/kernel/syscall.C9
-rw-r--r--src/lib/sync.C59
-rw-r--r--src/usr/testcore/lib/synctest.H15
5 files changed, 48 insertions, 46 deletions
diff --git a/src/include/arch/ppc.H b/src/include/arch/ppc.H
index 5280c82e4..405a2c297 100644
--- a/src/include/arch/ppc.H
+++ b/src/include/arch/ppc.H
@@ -64,7 +64,6 @@ inline void setHSRR1(uint64_t _hsrr1)
asm volatile("mtspr 315, %0" : : "r" (hsrr1));
}
-
ALWAYS_INLINE
inline uint64_t getPVR()
{
@@ -146,25 +145,25 @@ inline void setDEC(uint64_t _dec)
ALWAYS_INLINE
inline void sync()
{
- asm volatile("sync");
+ asm volatile("sync" ::: "memory");
}
ALWAYS_INLINE
inline void lwsync()
{
- asm volatile("lwsync");
+ asm volatile("lwsync" ::: "memory");
}
ALWAYS_INLINE
inline void isync()
{
- asm volatile("isync");
+ asm volatile("isync" ::: "memory");
}
ALWAYS_INLINE
inline void eieio()
{
- asm volatile("eieio");
+ asm volatile("eieio" ::: "memory");
}
ALWAYS_INLINE
diff --git a/src/kernel/futexmgr.C b/src/kernel/futexmgr.C
index 6271471aa..37ad54586 100644
--- a/src/kernel/futexmgr.C
+++ b/src/kernel/futexmgr.C
@@ -33,7 +33,7 @@ uint64_t FutexManager::_wait(task_t* i_task, uint64_t * i_addr, uint64_t i_val)
iv_lock.lock();
- if(*i_addr != i_val)
+ if(unlikely(*i_addr != i_val))
{
// some other thread has modified the futex
// bail-out retry required.
diff --git a/src/kernel/syscall.C b/src/kernel/syscall.C
index f96f8d372..1001ef361 100644
--- a/src/kernel/syscall.C
+++ b/src/kernel/syscall.C
@@ -356,11 +356,18 @@ namespace Systemcalls
uint64_t * uaddr = (uint64_t *) TASK_GETARG0(t);
uint64_t val = (uint64_t) TASK_GETARG1(t);
+ // Set RC to success initially.
+ TASK_SETRTN(t,0);
+
//TODO translate uaddr from user space to kernel space
// Right now they are the same.
uint64_t rc = FutexManager::wait(t,uaddr,val);
- TASK_SETRTN(t,rc);
+ if (rc != 0) // Can only set rc if we still have control of the task,
+ // which is only (for certain) on error rc's.
+ {
+ TASK_SETRTN(t,rc);
+ }
}
/**
diff --git a/src/lib/sync.C b/src/lib/sync.C
index d0d15aeb4..be20903d7 100644
--- a/src/lib/sync.C
+++ b/src/lib/sync.C
@@ -85,50 +85,31 @@ void mutex_destroy(mutex_t *& i_mutex)
void mutex_lock(mutex_t * i_mutex)
{
+ // Weak consistency notes:
+ // Since this is a lock, we do not need to ensure that all writes
+ // are globally visible prior to execution (lwsync) but we do need
+ // to ensure that all instructions finish completion prior to
+ // leaving (isync). Both __sync_val_compare_and_swap and
+ // __sync_lock_test_and_set have an implied isync.
uint64_t l_count = __sync_val_compare_and_swap(&(i_mutex->iv_val),0,1);
- if(l_count != 0)
+ if(unlikely(l_count != 0))
{
- if (l_count != 2)
+ if (likely(l_count != 2))
{
- lwsync();
l_count = __sync_lock_test_and_set(&(i_mutex->iv_val), 2);
}
while( l_count != 0 )
{
futex_wait( &(i_mutex->iv_val), 2);
- lwsync();
l_count = __sync_lock_test_and_set(&(i_mutex->iv_val),2);
// if more than one thread gets out - one continues while
// the rest get blocked again.
}
}
-#ifdef __IDEA_2
- // Idea # 2
- while(1)
- {
- uint64_t l_count = __sync_val_compare_and_swap(&(i_mutex->iv_val),0,1);
- if( 0 == l_count ) // uncontended lock
- {
- break;
- }
- else if ( 2 == l_count ) // already contended
- {
- futex_wait( &(i_mutex->iv_val), l_count);
- }
- else if ( 1 == l_count ) // lock now contended
- {
- if (__sync_bool_compare_and_swap(&(i_mutex->iv_val),1,2))
- {
- futex_wait( &(i_mutex->iv_val), 2);
- }
- }
- }
-#endif
-
return;
}
@@ -136,21 +117,21 @@ void mutex_lock(mutex_t * i_mutex)
void mutex_unlock(mutex_t * i_mutex)
{
-#ifdef __IDEA_1
- if (__sync_fetch_and_sub(&(i_mutex->iv_val), 1) != 1)
- {
- i_mutex->iv_val = 0; // the thread that is started will set this back to 2
- futex_wake(&(i_mutex->iv_val), 1); // wake one thread
- }
-#endif
- // #idea 2
- lwsync();
- uint64_t l_count = __sync_lock_test_and_set(&(i_mutex->iv_val),0);
- if(2 == l_count)
+ // Weak consistency notes:
+ // Since this is an unlock we need to ensure that all writes are
+ // globally visible prior to execution (lwsync). The
+ // __sync_fetch_and_sub has an implied lwsync. If we need to
+ // release a task, due to a contended mutex, the write to iv_val
+ // and futex_wake pair will appear globally ordered due to the
+ // context synchronizing nature of the 'sc' instruction.
+
+ uint64_t l_count = __sync_fetch_and_sub(&(i_mutex->iv_val),1);
+ if(unlikely(2 <= l_count))
{
+ i_mutex->iv_val = 0;
futex_wake(&(i_mutex->iv_val), 1); // wake one thread
}
-
+
return;
}
diff --git a/src/usr/testcore/lib/synctest.H b/src/usr/testcore/lib/synctest.H
index 5a53d2e0a..8e7619d11 100644
--- a/src/usr/testcore/lib/synctest.H
+++ b/src/usr/testcore/lib/synctest.H
@@ -9,6 +9,7 @@
#include <cxxtest/TestSuite.H>
#include <sys/sync.h>
#include <sys/task.h>
+#include <sys/time.h>
class SyncTest: public CxxTest::TestSuite
{
@@ -30,6 +31,20 @@ class SyncTest: public CxxTest::TestSuite
TS_TRACE("ALL THREADS ENDED");
}
+ void testMutexDoubleWait()
+ {
+ mutex_init(&mutex);
+ barrier_init(&barrier, 3);
+
+ mutex_lock(&mutex);
+ task_create(func2, this);
+ task_create(func2, this);
+ nanosleep(1,0);
+ mutex_unlock(&mutex);
+ barrier_wait(&barrier);
+ TS_TRACE("ALL THREADS ENDED");
+ }
+
void testBarrier()
{
barrier_t barrier;
OpenPOWER on IntegriCloud