@@ -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.
@@ -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
@@ -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);
@@ -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
@@ -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);
@@ -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();
@@ -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
During working on the atomics-in-libitm patch, I noticed that during upgrades to serial mode, we were clearing the reader flag of the upgrading transaction too early. This would be fine for just the serial lock but we also use this flag in the quiescence-based privatization safety implementation; clearing the flag early would expose the upgrading transaction's redo writes during commit or the undo writes during rollback to nontransactional code running after the other privatizing transaction (which in turn can lead to race conditions). The attached patch fixes that by deferring unsetting the reader flag until commit or rollback of the upgrader have finished. OK for trunk? commit c2a7ff4cc87c5bc19e88a30a454755a6725bd746 Author: Torvald Riegel <triegel@redhat.com> Date: Thu Dec 22 20:11:37 2011 +0100 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.