Patchwork [trans-mem] Beginning of refactoring

On Tue, 2011-07-19 at 01:22 +0200, Torvald Riegel wrote:
> On Tue, 2011-07-19 at 01:19 +0200, Torvald Riegel wrote:
> > On Sat, 2011-07-09 at 16:30 +0200, Torvald Riegel wrote:
> > > The attached patch makes flat nesting the default, while still
> > > supporting closed nesting on demand (for user-controlled aborts via
> > > __transaction_cancel).
> > > Previously, a new transaction object was created for each nested
> > > transaction. The patch changes this to using one transaction object
> > > during the whole lifetime of a thread and keeping checkpoints of the
> > > transaction state when starting closed nested transactions. This allows
> > > us to easily use flat nesting and decreases the transaction start/commit
> > > overheads to some extent.
> > > 
> > > OK for branch?
> > 
> > I split the patch, as requested.
> > 
> > First three parts are smaller changes:
> > 
> > patch1: New erase method and placement new for aatree.
> > patch2: Change pr_hasElse to the value specified in the ABI.
> > patch3: Add information to dispatch about closed nesting and
> > uninstrumented code.
> > 
> > Then, in preparation for the following flat-transaction object design:
> > patch4: Use vector instead of list to store user actions.
> > patch5: Add closed nesting as restart reason.
> > 
> > And the actual core change:
> > patch6: Make flat nesting the default, use closed nesting on demand.
> > 
> > This last patch is still rather large, but it is one logical piece.
> > beginend.cc has most changes, but splitting this one up would rather
> > create incomplete intermediate steps than make the direction of this
> > whole patch clear.
> > 
> > As discussed offline, reducing the one extra copying of jmpbuf for
> > actual closed-nested transactions can be addressed in a future patch by
> > modifying _ITM_beginTransaction, which could additionally remove the
> > extra copying from stack to jmpbuf for all transactions as it existed
> > before this patch here.
> > 
> > Ok for branch, or is further splitting required?
> > 
> 
> This time with patches attached, sorry.

Attached are two small (in size) fixes for this patch set.

patch7: gtm_transaction::decide_begin_dispatch() gets the transaction
properties from the caller instead of reading from
gtm_transaction::prop, which might not have been updated by the caller
yet.

patch8: Fix nesting level reset when rolling back the outermost
transaction. Before, this was incorrectly reset to zero, which caused
transactions to not get committed. This didn't show up in previous
testing because previously, only serial-mode TM methods were available.

OK for branch, together with the previous patches?
commit f3d24b9933d2124fe330e0513f14ce0711402e88
Author: Torvald Riegel <triegel@redhat.com>
Date:   Tue Jul 26 20:28:54 2011 +0200

    Fix gtm_transaction::decide_begin_dispatch().
    
    	* retry.cc (GTM::gtm_transaction::decide_begin_dispatch): Get transaction
    	properties from the caller instead of from the transaction object.
    	* libitm_i.h: Same.
    	* beginend.cc (GTM::gtm_transaction::begin_transaction): Same.
commit f85b6ce7b67cecfd9c056620c7ef2aaa173fe5bc
Author: Torvald Riegel <triegel@redhat.com>
Date:   Wed Jul 27 12:27:10 2011 +0200

    Fix nesting level reset when rolling back the outermost transaction.
    
    	* beginend.cc (GTM::gtm_transaction::rollback): Fix nesting level
    	rollback.

diff --git a/libitm/beginend.cc b/libitm/beginend.cc
index 417b41a..ba90af2 100644
--- a/libitm/beginend.cc
+++ b/libitm/beginend.cc
@@ -291,6 +291,7 @@ GTM::gtm_transaction::rollback (gtm_transaction_cp *cp)
     }
   else
     {
+      // Roll back to the outermost transaction.
       // Restore the jump buffer and transaction properties, which we will
       // need for the longjmp used to restart or abort the transaction.
       if (parent_txns.size() > 0)
@@ -298,9 +299,11 @@ GTM::gtm_transaction::rollback (gtm_transaction_cp *cp)
           jb = parent_txns[0].jb;
           prop = parent_txns[0].prop;
         }
-      // Reset the transaction. Do not reset state, which is handled by the
-      // callers.
-      nesting = 0;
+      // Reset the transaction. Do not reset this->state, which is handled by
+      // the callers. Note that we reset the transaction to the point after
+      // having executed begin_transaction (we will return from it), so the
+      // nesting level must be one, not zero.
+      nesting = 1;
       parent_txns.clear();
     }

On 07/27/2011 03:35 AM, Torvald Riegel wrote:
> patch7: gtm_transaction::decide_begin_dispatch() gets the transaction
> properties from the caller instead of reading from
> gtm_transaction::prop, which might not have been updated by the caller
> yet.
> 
> patch8: Fix nesting level reset when rolling back the outermost
> transaction. Before, this was incorrectly reset to zero, which caused
> transactions to not get committed. This didn't show up in previous
> testing because previously, only serial-mode TM methods were available.
> 
> OK for branch, together with the previous patches?

Both ok.


r~