libitm: Fix privatization safety during upgrades to serial mode.
authorTorvald Riegel <triegel@redhat.com>
Sat, 24 Dec 2011 01:42:35 +0000 (01:42 +0000)
committerTorvald Riegel <torvald@gcc.gnu.org>
Sat, 24 Dec 2011 01:42:35 +0000 (01:42 +0000)
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.

From-SVN: r182675

libitm/ChangeLog
libitm/beginend.cc
libitm/config/linux/rwlock.cc
libitm/config/linux/rwlock.h
libitm/config/posix/rwlock.cc
libitm/config/posix/rwlock.h
libitm/libitm_i.h
libitm/method-serial.cc

index 9a835e2860b20d296659496cfce5281290b722a5..abdf4fb62d8a226947f21a044fe1e2ac99d34ab3 100644 (file)
@@ -1,3 +1,18 @@
+2011-12-24  Torvald Riegel  <triegel@redhat.com>
+
+       * 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  <triegel@redhat.com>
 
        * beginend.cc (GTM::gtm_thread::begin_transaction): Add comment.
index d0ad5a7fc2a9901279cbd69b47c13be5c185887a..17f9d7490d898f4caa0e2c75429fd5eb3ef89914 100644 (file)
@@ -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.
index f87be2e880a6ddb4046b56c8bd6367b84e3d09dc..24e7042ff1002574d323d014323073e8c26dfcd6 100644 (file)
@@ -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
index e5a53c054f6095bdfde3c9346fe50aaef642fe7c..987e58014092ce3976faf6fd81cadc59b21f5469 100644 (file)
@@ -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);
index 7ef39982ccf064b0fb0a785490d8fe3cf3789294..f93baf2e10081b3c0d5a688427e0f67c22bc07c7 100644 (file)
@@ -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
index 359991ab45b852864e69cbca253291fd3a47c4ce..a1a6042e562dd24ecb6d58bc6164cc8f4095d25d 100644 (file)
@@ -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);
index d57872ede7bef1c713fa7bb1676109603d3a7db2..b53792a378105b4527801eecd4ee65b857863b5d 100644 (file)
@@ -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();
index 5e85653a2f450295cc582e266690b813975dc495..bf7982650fff0eb4ebb69c24f189b14d14ab08a8 100644 (file)
@@ -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