diff options
author | Patrick Williams <iawillia@us.ibm.com> | 2011-08-08 16:27:17 -0500 |
---|---|---|
committer | A. Patrick Williams III <iawillia@us.ibm.com> | 2011-08-09 13:54:57 -0500 |
commit | be61bd382972ec946396dadc161231c65ea9d968 (patch) | |
tree | 29acf3176ca910cd3f740d930b9d1fcee0c0e382 /src/lib/sync.C | |
parent | af0bcd44f7c475eb00178d4f448a14343c0354c0 (diff) | |
download | talos-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/lib/sync.C')
-rw-r--r-- | src/lib/sync.C | 59 |
1 files changed, 20 insertions, 39 deletions
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; } |