From 629e47295b44d9adf01b66061dd891a25e567474 Mon Sep 17 00:00:00 2001
From: Torvald Riegel <triegel@redhat.com>
Date: Wed, 13 Jan 2016 12:40:34 +0000
Subject: [PATCH] libitm: Fix privatization safety interaction with serial
 mode.

From-SVN: r232322
---
 libitm/ChangeLog              | 12 ++++++++++++
 libitm/beginend.cc            | 34 +++++++++++++++++++++++++++++-----
 libitm/config/linux/rwlock.cc | 13 +++++++++++++
 libitm/config/posix/rwlock.cc | 20 ++++++++++++++++++++
 libitm/dispatch.h             |  4 ++++
 libitm/method-gl.cc           |  9 +++++++++
 libitm/method-ml.cc           | 18 ++++++++++++++++++
 libitm/method-serial.cc       |  4 ++++
 8 files changed, 109 insertions(+), 5 deletions(-)

diff --git a/libitm/ChangeLog b/libitm/ChangeLog
index e07cca9c71b5..03624d4a4ed0 100644
--- a/libitm/ChangeLog
+++ b/libitm/ChangeLog
@@ -1,3 +1,15 @@
+2016-01-13  Torvald Riegel  <triegel@redhat.com>
+
+	* beginend.cc (gtm_thread::trycommit): Fix privatization safety.
+	* config/linux/rwlock.cc (gtm_rwlock::write_lock_generic): Likewise.
+	* config/posix/rwlock.cc (gtm_rwlock::write_lock_generic): Likewise.
+	* dispatch.h (abi_dispatch::snapshot_most_recent): New.
+	* method-gl.cc (gl_wt_dispatch::snapshot_most_recent): New.
+	* method-ml.cc (ml_wt_dispatch::snapshot_most_recent): New.
+	* method-serial.cc (serial_dispatch::snapshot_most_recent): New.
+	(serialirr_dispatch::snapshot_most_recent): New.
+	(serialirr_onwrite_dispatch::snapshot_most_recent): New.
+
 2016-01-12  Torvald Riegel  <triegel@redhat.com>
 
 	* libitm_i.h (gtm_mask_stack): Remove.
diff --git a/libitm/beginend.cc b/libitm/beginend.cc
index c801dab005bc..85fb4f13ed5b 100644
--- a/libitm/beginend.cc
+++ b/libitm/beginend.cc
@@ -568,8 +568,9 @@ GTM::gtm_thread::trycommit ()
   gtm_word priv_time = 0;
   if (abi_disp()->trycommit (priv_time))
     {
-      // The transaction is now inactive. Everything that we still have to do
-      // will not synchronize with other transactions anymore.
+      // The transaction is now finished but we will still access some shared
+      // data if we have to ensure privatization safety.
+      bool do_read_unlock = false;
       if (state & gtm_thread::STATE_SERIAL)
         {
           gtm_thread::serial_lock.write_unlock ();
@@ -578,7 +579,27 @@ GTM::gtm_thread::trycommit ()
           priv_time = 0;
         }
       else
-	gtm_thread::serial_lock.read_unlock (this);
+	{
+	  // If we have to ensure privatization safety, we must not yet
+	  // release the read lock and become inactive because (1) we still
+	  // have to go through the list of all transactions, which can be
+	  // modified by serial mode threads, and (2) we interpret each
+	  // transactions' shared_state in the context of what we believe to
+	  // be the current method group (and serial mode transactions can
+	  // change the method group).  Therefore, if we have to ensure
+	  // privatization safety, delay becoming inactive but set a maximum
+	  // snapshot time (we have committed and thus have an empty snapshot,
+	  // so it will always be most recent).  Use release MO so that this
+	  // synchronizes with other threads observing our snapshot time.
+	  if (priv_time)
+	    {
+	      do_read_unlock = true;
+	      shared_state.store((~(typeof gtm_thread::shared_state)0) - 1,
+		  memory_order_release);
+	    }
+	  else
+	    gtm_thread::serial_lock.read_unlock (this);
+	}
       state = 0;
 
       // We can commit the undo log after dispatch-specific commit and after
@@ -618,8 +639,11 @@ GTM::gtm_thread::trycommit ()
 	    }
 	}
 
-      // After ensuring privatization safety, we execute potentially
-      // privatizing actions (e.g., calling free()). User actions are first.
+      // After ensuring privatization safety, we are now truly inactive and
+      // thus can release the read lock.  We will also execute potentially
+      // privatizing actions (e.g., calling free()).  User actions are first.
+      if (do_read_unlock)
+	gtm_thread::serial_lock.read_unlock (this);
       commit_user_actions ();
       commit_allocations (false, 0);
 
diff --git a/libitm/config/linux/rwlock.cc b/libitm/config/linux/rwlock.cc
index 46a775fd4e83..381a553171eb 100644
--- a/libitm/config/linux/rwlock.cc
+++ b/libitm/config/linux/rwlock.cc
@@ -158,6 +158,19 @@ gtm_rwlock::write_lock_generic (gtm_thread *tx)
       while (it->shared_state.load (memory_order_relaxed)
           != ~(typeof it->shared_state)0)
 	{
+	  // If this is an upgrade, we have to break deadlocks with
+	  // privatization safety.  This may fail on our side, in which
+	  // case we need to cancel our attempt to upgrade.  Also, we do not
+	  // block but just spin so that we never have to be woken.
+	  if (tx != 0)
+	    {
+	      if (!abi_disp()->snapshot_most_recent ())
+		{
+		  write_unlock ();
+		  return false;
+		}
+	      continue;
+	    }
 	  // An active reader. Wait until it has finished. To avoid lost
 	  // wake-ups, we need to use Dekker-like synchronization.
 	  // Note that we can reset writer_readers to zero when we see after
diff --git a/libitm/config/posix/rwlock.cc b/libitm/config/posix/rwlock.cc
index bf58ec051e6e..1e1eea820f29 100644
--- a/libitm/config/posix/rwlock.cc
+++ b/libitm/config/posix/rwlock.cc
@@ -200,6 +200,26 @@ gtm_rwlock::write_lock_generic (gtm_thread *tx)
       if (readers == 0)
 	break;
 
+      // If this is an upgrade, we have to break deadlocks with
+      // privatization safety.  This may fail on our side, in which
+      // case we need to cancel our attempt to upgrade.  Also, we do not
+      // block using the convdar but just spin so that we never have to be
+      // woken.
+      // FIXME This is horribly inefficient -- but so is not being able
+      // to use futexes in this case.
+      if (tx != 0)
+	{
+	  pthread_mutex_unlock (&this->mutex);
+	  if (!abi_disp ()->snapshot_most_recent ())
+	    {
+	      write_unlock ();
+	      return false;
+	    }
+	  pthread_mutex_lock (&this->mutex);
+	  continue;
+	}
+
+
       // We've seen a number of readers, so we publish this number and wait.
       this->a_readers = readers;
       pthread_cond_wait (&this->c_confirmed_writers, &this->mutex);
diff --git a/libitm/dispatch.h b/libitm/dispatch.h
index 0bf1a81b35e5..4ec563158293 100644
--- a/libitm/dispatch.h
+++ b/libitm/dispatch.h
@@ -291,6 +291,10 @@ public:
   // Rolls back a transaction. Called on abort or after trycommit() returned
   // false.
   virtual void rollback(gtm_transaction_cp *cp = 0) = 0;
+  // Returns true iff the snapshot is most recent, which will be the case if
+  // this transaction cannot be the reason why other transactions cannot
+  // ensure privatization safety.
+  virtual bool snapshot_most_recent() = 0;
 
   // Return an alternative method that is compatible with the current
   // method but supports closed nesting. Return zero if there is none.
diff --git a/libitm/method-gl.cc b/libitm/method-gl.cc
index 87d01dbf1f1d..b2e2bcad71e6 100644
--- a/libitm/method-gl.cc
+++ b/libitm/method-gl.cc
@@ -338,6 +338,15 @@ public:
 
   }
 
+  virtual bool snapshot_most_recent()
+  {
+    // This is the same check as in validate() except that we do not restart
+    // on failure but simply return the result.
+    return o_gl_mg.orec.load(memory_order_relaxed)
+	== gtm_thr()->shared_state.load(memory_order_relaxed);
+  }
+
+
   CREATE_DISPATCH_METHODS(virtual, )
   CREATE_DISPATCH_METHODS_MEM()
 
diff --git a/libitm/method-ml.cc b/libitm/method-ml.cc
index 1c2de7010ed7..723643ab97cd 100644
--- a/libitm/method-ml.cc
+++ b/libitm/method-ml.cc
@@ -604,6 +604,24 @@ public:
     tx->readlog.clear();
   }
 
+  virtual bool snapshot_most_recent()
+  {
+    // This is the same code as in extend() except that we do not restart
+    // on failure but simply return the result, and that we don't validate
+    // if our snapshot is already most recent.
+    gtm_thread* tx = gtm_thr();
+    gtm_word snapshot = o_ml_mg.time.load(memory_order_acquire);
+    if (snapshot == tx->shared_state.load(memory_order_relaxed))
+      return true;
+    if (!validate(tx))
+      return false;
+
+    // Update our public snapshot time.  Necessary so that we do not prevent
+    // other transactions from ensuring privatization safety.
+    tx->shared_state.store(snapshot, memory_order_release);
+    return true;
+  }
+
   virtual bool supports(unsigned number_of_threads)
   {
     // Each txn can commit and fail and rollback once before checking for
diff --git a/libitm/method-serial.cc b/libitm/method-serial.cc
index 82d407df8d39..1123e34d56ba 100644
--- a/libitm/method-serial.cc
+++ b/libitm/method-serial.cc
@@ -95,6 +95,7 @@ class serialirr_dispatch : public abi_dispatch
   virtual gtm_restart_reason begin_or_restart() { return NO_RESTART; }
   virtual bool trycommit(gtm_word& priv_time) { return true; }
   virtual void rollback(gtm_transaction_cp *cp) { abort(); }
+  virtual bool snapshot_most_recent() { return true; }
 
   virtual abi_dispatch* closed_nesting_alternative()
   {
@@ -149,6 +150,7 @@ public:
   // Local undo will handle this.
   // trydropreference() need not be changed either.
   virtual void rollback(gtm_transaction_cp *cp) { }
+  virtual bool snapshot_most_recent() { return true; }
 
   CREATE_DISPATCH_METHODS(virtual, )
   CREATE_DISPATCH_METHODS_MEM()
@@ -210,6 +212,8 @@ class serialirr_onwrite_dispatch : public serialirr_dispatch
     if (tx->state & gtm_thread::STATE_IRREVOCABLE)
       abort();
   }
+
+  virtual bool snapshot_most_recent() { return true; }
 };
 
 // This group is pure HTM with serial mode as a fallback.  There is no
-- 
GitLab