On Sat, Mar 1, 2014 at 2:32 AM, Peter Bergner <berg...@vnet.ibm.com> wrote:
> 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.
Works for me in case nobody else objects within 24h.
Thanks,
Richard.
> 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;
> }
>
>
