I'd like to ask for permission to backport the following two LIBITM bug
fixes to the FSF 4.8 branch. Although these are not technically fixing
regressions, they do fix the libitm.c/reentrant.c testsuite failure on
s390 and powerpc (or at least it will when we finally get our power8
code backported to FSF 4.8). It also fixes a real bug on x86 that is
latent because we don't currently have a test case that warms up the
x86's RTM hardware enough such that its xbegin succeeds exposing the
bug. I'd like this backport so that the 4.8 based distros won't need
to carry this as an add-on patch.
It should also be fairly safe as well, since the fixed code is limited
to the arches (x86, s390 and powerpc) that define USE_HTM_FASTPATH,
so all others definitely won't see a difference.
I'll note I CC'd some of the usual suspects interested in TM as well
as the normal RMs, because LIBITM doesn't seem to have a maintainer
or reviewer listed in the MAINTAINERS file. Is that an oversight or???
Peter
Backport from mainline
2013-06-20 Torvald Riegel <trie...@redhat.com>
* query.cc (_ITM_inTransaction): Abort when using the HTM fastpath.
(_ITM_getTransactionId): Same.
* config/x86/target.h (htm_transaction_active): New.
2013-06-20 Torvald Riegel <trie...@redhat.com>
PR libitm/57643
* beginend.cc (gtm_thread::begin_transaction): Handle reentrancy in
the HTM fastpath.
Index: libitm/beginend.cc
===================================================================
--- libitm/beginend.cc (revision 208151)
+++ libitm/beginend.cc (working copy)
@@ -197,6 +197,8 @@
// We are executing a transaction now.
// Monitor the writer flag in the serial-mode lock, and abort
// if there is an active or waiting serial-mode transaction.
+ // Note that this can also happen due to an enclosing
+ // serial-mode transaction; we handle this case below.
if (unlikely(serial_lock.is_write_locked()))
htm_abort();
else
@@ -219,6 +221,14 @@
tx = new gtm_thread();
set_gtm_thr(tx);
}
+ // Check whether there is an enclosing serial-mode transaction;
+ // if so, we just continue as a nested transaction and don't
+ // try to use the HTM fastpath. This case can happen when an
+ // outermost relaxed transaction calls unsafe code that starts
+ // a transaction.
+ if (tx->nesting > 0)
+ break;
+ // Another thread is running a serial-mode transaction. Wait.
serial_lock.read_lock(tx);
serial_lock.read_unlock(tx);
// TODO We should probably reset the retry count t here, unless
Index: libitm/config/x86/target.h
===================================================================
--- libitm/config/x86/target.h (revision 208151)
+++ libitm/config/x86/target.h (working copy)
@@ -125,6 +125,13 @@
{
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
Index: libitm/query.cc
===================================================================
--- libitm/query.cc (revision 208151)
+++ libitm/query.cc (working copy)
@@ -43,6 +43,15 @@
_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_fastpath && htm_transaction_active())
+ htm_abort();
+#endif
struct gtm_thread *tx = gtm_thr();
if (tx && (tx->nesting > 0))
{
@@ -58,6 +67,11 @@
_ITM_transactionId_t ITM_REGPARM
_ITM_getTransactionId (void)
{
+#if defined(USE_HTM_FASTPATH)
+ // See ITM_inTransaction.
+ if (htm_fastpath && htm_transaction_active())
+ htm_abort();
+#endif
struct gtm_thread *tx = gtm_thr();
return (tx && (tx->nesting > 0)) ? tx->id : _ITM_noTransactionId;
}
