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.

From-SVN: r182675

8 files changed, 65 insertions, 23 deletions

diff --git a/libitm/ChangeLog b/libitm/ChangeLog
index 9a835e2..abdf4fb 100644
--- a/libitm/ChangeLog
+++ b/libitm/ChangeLog
@@ -1,5 +1,20 @@
 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.
 	(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 d0ad5a7..17f9d74 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 f87be2e..24e7042 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 e5a53c0..987e580 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 7ef3998..f93baf2 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 359991a..a1a6042 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 d57872e..b53792a 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 5e85653..bf79826 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