From: torvald Date: Sat, 24 Dec 2011 01:42:35 +0000 (+0000) Subject: libitm: Fix privatization safety during upgrades to serial mode. X-Git-Url: http://git.sourceforge.jp/view?p=pf3gnuchains%2Fgcc-fork.git;a=commitdiff_plain;h=c4c0a912745588307c7be984615a825edd421bbd libitm: Fix privatization safety during upgrades to serial mode. libitm/ * beginend.cc (GTM::gtm_thread::restart): Add and handle finish_serial_upgrade parameter. * libitm.h (GTM::gtm_thread::restart): Adapt declaration. * config/linux/rwlock.cc (GTM::gtm_rwlock::write_lock_generic): Don't unset reader flag. (GTM::gtm_rwlock::write_upgrade_finish): New. * config/posix/rwlock.cc: Same. * config/linux/rwlock.h (GTM::gtm_rwlock::write_upgrade_finish): Declare. * config/posix/rwlock.h: Same. * method-serial.cc (GTM::gtm_thread::serialirr_mode): Unset reader flag after commit or after rollback when restarting. git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@182675 138bc75d-0d04-0410-961f-82ee72b054a4 --- diff --git a/libitm/ChangeLog b/libitm/ChangeLog index 9a835e2860b..abdf4fb62d8 100644 --- a/libitm/ChangeLog +++ b/libitm/ChangeLog @@ -1,5 +1,20 @@ 2011-12-24 Torvald Riegel + * beginend.cc (GTM::gtm_thread::restart): Add and handle + finish_serial_upgrade parameter. + * libitm.h (GTM::gtm_thread::restart): Adapt declaration. + * config/linux/rwlock.cc (GTM::gtm_rwlock::write_lock_generic): + Don't unset reader flag. + (GTM::gtm_rwlock::write_upgrade_finish): New. + * config/posix/rwlock.cc: Same. + * config/linux/rwlock.h (GTM::gtm_rwlock::write_upgrade_finish): + Declare. + * config/posix/rwlock.h: Same. + * method-serial.cc (GTM::gtm_thread::serialirr_mode): Unset reader + flag after commit or after rollback when restarting. + +2011-12-24 Torvald Riegel + * beginend.cc (GTM::gtm_thread::begin_transaction): Add comment. (GTM::gtm_thread::try_commit): Changed memory order. * config/linux/alpha/futex_bits.h (sys_futex0): Take atomic int diff --git a/libitm/beginend.cc b/libitm/beginend.cc index d0ad5a7fc2a..17f9d7490d8 100644 --- a/libitm/beginend.cc +++ b/libitm/beginend.cc @@ -511,11 +511,19 @@ GTM::gtm_thread::trycommit () } void ITM_NORETURN -GTM::gtm_thread::restart (gtm_restart_reason r) +GTM::gtm_thread::restart (gtm_restart_reason r, bool finish_serial_upgrade) { // Roll back to outermost transaction. Do not reset transaction state because // we will continue executing this transaction. rollback (); + + // If we have to restart while an upgrade of the serial lock is happening, + // we need to finish this here, after rollback (to ensure privatization + // safety despite undo writes) and before deciding about the retry strategy + // (which could switch to/from serial mode). + if (finish_serial_upgrade) + gtm_thread::serial_lock.write_upgrade_finish(this); + decide_retry_strategy (r); // Run dispatch-specific restart code. Retry until we succeed. diff --git a/libitm/config/linux/rwlock.cc b/libitm/config/linux/rwlock.cc index f87be2e880a..24e7042ff10 100644 --- a/libitm/config/linux/rwlock.cc +++ b/libitm/config/linux/rwlock.cc @@ -121,17 +121,13 @@ gtm_rwlock::write_lock_generic (gtm_thread *tx) // readers that might still be active. // We don't need an extra barrier here because the CAS and the xchg // operations have full barrier semantics already. - - // If this is an upgrade, we are not a reader anymore. This is only safe to - // do after we have acquired the writer lock. // TODO In the worst case, this requires one wait/wake pair for each // active reader. Reduce this! - if (tx != 0) - tx->shared_state.store (-1, memory_order_relaxed); - for (gtm_thread *it = gtm_thread::list_of_threads; it != 0; it = it->next_thread) { + if (it == tx) + continue; // Use a loop here to check reader flags again after waiting. while (it->shared_state.load (memory_order_relaxed) != ~(typeof it->shared_state)0) @@ -175,6 +171,18 @@ gtm_rwlock::write_upgrade (gtm_thread *tx) } +// Has to be called iff the previous upgrade was successful and after it is +// safe for the transaction to not be marked as a reader anymore. + +void +gtm_rwlock::write_upgrade_finish (gtm_thread *tx) +{ + // We are not a reader anymore. This is only safe to do after we have + // acquired the writer lock. + tx->shared_state.store (-1, memory_order_release); +} + + // Release a RW lock from reading. void diff --git a/libitm/config/linux/rwlock.h b/libitm/config/linux/rwlock.h index e5a53c054f6..987e5801409 100644 --- a/libitm/config/linux/rwlock.h +++ b/libitm/config/linux/rwlock.h @@ -57,6 +57,7 @@ class gtm_rwlock void write_unlock (); bool write_upgrade (gtm_thread *tx); + void write_upgrade_finish (gtm_thread *tx); protected: bool write_lock_generic (gtm_thread *tx); diff --git a/libitm/config/posix/rwlock.cc b/libitm/config/posix/rwlock.cc index 7ef39982ccf..f93baf2e100 100644 --- a/libitm/config/posix/rwlock.cc +++ b/libitm/config/posix/rwlock.cc @@ -155,10 +155,6 @@ gtm_rwlock::write_lock_generic (gtm_thread *tx) // path of read_lock()). atomic_thread_fence(memory_order_seq_cst); - // If this is an upgrade, we are not a reader anymore. - if (tx != 0) - tx->shared_state.store(-1, memory_order_relaxed); - // Count the number of active readers to be able to decrease the number of // wake-ups and wait calls that are necessary. // @@ -194,6 +190,8 @@ gtm_rwlock::write_lock_generic (gtm_thread *tx) it = it->next_thread) { // Don't count ourself if this is an upgrade. + if (it == tx) + continue; if (it->shared_state.load(memory_order_relaxed) != (gtm_word)-1) readers++; } @@ -231,6 +229,18 @@ gtm_rwlock::write_upgrade (gtm_thread *tx) } +// Has to be called iff the previous upgrade was successful and after it is +// safe for the transaction to not be marked as a reader anymore. + +void +gtm_rwlock::write_upgrade_finish (gtm_thread *tx) +{ + // We are not a reader anymore. This is only safe to do after we have + // acquired the writer lock. + tx->shared_state.store (-1, memory_order_release); +} + + // Release a RW lock from reading. void diff --git a/libitm/config/posix/rwlock.h b/libitm/config/posix/rwlock.h index 359991ab45b..a1a6042e562 100644 --- a/libitm/config/posix/rwlock.h +++ b/libitm/config/posix/rwlock.h @@ -72,6 +72,7 @@ class gtm_rwlock void write_unlock (); bool write_upgrade (gtm_thread *tx); + void write_upgrade_finish (gtm_thread *tx); protected: bool write_lock_generic (gtm_thread *tx); diff --git a/libitm/libitm_i.h b/libitm/libitm_i.h index d57872ede7b..b53792a3781 100644 --- a/libitm/libitm_i.h +++ b/libitm/libitm_i.h @@ -231,7 +231,8 @@ struct gtm_thread // In beginend.cc void rollback (gtm_transaction_cp *cp = 0, bool aborting = false); bool trycommit (); - void restart (gtm_restart_reason) ITM_NORETURN; + void restart (gtm_restart_reason, bool finish_serial_upgrade = false) + ITM_NORETURN; gtm_thread(); ~gtm_thread(); diff --git a/libitm/method-serial.cc b/libitm/method-serial.cc index 5e85653a2f4..bf7982650ff 100644 --- a/libitm/method-serial.cc +++ b/libitm/method-serial.cc @@ -239,7 +239,6 @@ void GTM::gtm_thread::serialirr_mode () { struct abi_dispatch *disp = abi_disp (); - bool need_restart = true; if (this->state & STATE_SERIAL) { @@ -254,7 +253,6 @@ GTM::gtm_thread::serialirr_mode () bool ok = disp->trycommit (priv_time); // Given that we're already serial, the trycommit better work. assert (ok); - need_restart = false; } else if (serial_lock.write_upgrade (this)) { @@ -263,18 +261,18 @@ GTM::gtm_thread::serialirr_mode () // would do for an outermost commit. // We have successfully upgraded to serial mode, so we don't need to // ensure privatization safety for other transactions here. + // However, we are still a reader (wrt. privatization safety) until we + // have either committed or restarted, so finish the upgrade after that. gtm_word priv_time = 0; - if (disp->trycommit (priv_time)) - need_restart = false; + if (!disp->trycommit (priv_time)) + restart (RESTART_SERIAL_IRR, true); + gtm_thread::serial_lock.write_upgrade_finish(this); } - - if (need_restart) - restart (RESTART_SERIAL_IRR); else - { - this->state |= (STATE_SERIAL | STATE_IRREVOCABLE); - set_abi_disp (dispatch_serialirr ()); - } + restart (RESTART_SERIAL_IRR, false); + + this->state |= (STATE_SERIAL | STATE_IRREVOCABLE); + set_abi_disp (dispatch_serialirr ()); } void ITM_REGPARM