Re: [patch] libitm: Fix handling of reentrancy in the HTM fastpath

[Torvald Riegel]

On Wed, 2013-06-19 at 14:43 -0500, Peter Bergner wrote: 
> On Wed, 2013-06-19 at 10:57 -0500, Peter Bergner wrote:
> > On Wed, 2013-06-19 at 10:49 -0500, Peter Bergner wrote:
> > > This is due to the following in _ITM_inTransaction():
> > > 
> > > 47          if (tx && (tx->nesting > 0))
> > > (gdb) p tx
> > > $2 = (GTM::gtm_thread *) 0x10901bf0
> > > (gdb) p tx->nesting
> > > $3 = 1
> > > (gdb) step
> > > 49              if (tx->state & gtm_thread::STATE_IRREVOCABLE)
> > > (gdb) p tx->state
> > > $4 = 3
> > > (gdb) p gtm_thread::STATE_IRREVOCABLE
> > > $5 = 2
> > > (gdb) step
> > > 50                return inIrrevocableTransaction;
> > 
> > Bah, ignore this.  It's a different call that is returning something other
> > than inIrrevocableTransaction.  Unfortunately, gdb is having problems inside
> > hw txns and I'm having trouble seeing why/when _ITM_inTransaction() is
> > returning something other than inIrrevocableTransaction.  I'll see if I can
> > determine why and will report back.
> 
> Ok, we return outsideTransaction because the nesting level (tx->nesting)
> is zero.

That's a second bug in libitm, sorry.  Can you try with the attached
patch additionally to the previous one?  Thanks!

Torvald



commit 02dde6bb91107792fb0cb9f5c4785d25b6aa0e3c
Author: Torvald Riegel <trie...@redhat.com>
Date:   Thu Jun 20 00:46:59 2013 +0200

    libitm: Handle HTM fastpath in status query functions.

diff --git a/libitm/config/x86/target.h b/libitm/config/x86/target.h
index 77b627f..063c09e 100644
--- a/libitm/config/x86/target.h
+++ b/libitm/config/x86/target.h
@@ -125,6 +125,13 @@ htm_abort_should_retry (uint32_t begin_ret)
 {
   return begin_ret & _XABORT_RETRY;
 }
+
+/* Returns true iff a hardware transaction is currently being executed.  */
+static inline bool
+htm_transaction_active ()
+{
+  return _xtest() != 0;
+}
 #endif
 
 
diff --git a/libitm/query.cc b/libitm/query.cc
index 5707321..0ac3eda 100644
--- a/libitm/query.cc
+++ b/libitm/query.cc
@@ -43,6 +43,15 @@ _ITM_libraryVersion (void)
 _ITM_howExecuting ITM_REGPARM
 _ITM_inTransaction (void)
 {
+#if defined(USE_HTM_FASTPATH)
+  // If we use the HTM fastpath, we cannot reliably detect whether we are
+  // in a transaction because this function can be called outside of
+  // a transaction and thus we can't deduce this by looking at just the serial
+  // lock.  This function isn't used in practice currently, so the easiest
+  // way to handle it is to just abort.
+  if (htm_transaction_active())
+    htm_abort();
+#endif
   struct gtm_thread *tx = gtm_thr();
   if (tx && (tx->nesting > 0))
     {
@@ -58,6 +67,11 @@ _ITM_inTransaction (void)
 _ITM_transactionId_t ITM_REGPARM
 _ITM_getTransactionId (void)
 {
+#if defined(USE_HTM_FASTPATH)
+  // See ITM_inTransaction.
+  if (htm_transaction_active())
+    htm_abort();
+#endif
   struct gtm_thread *tx = gtm_thr();
   return (tx && (tx->nesting > 0)) ? tx->id : _ITM_noTransactionId;
 }