| Message ID | 1371682261.964.3.camel@triegel.csb |
|---|---|
| State | New |
| Headers | show |
On Thu, 2013-06-20 at 00:51 +0200, Torvald Riegel wrote: > On Wed, 2013-06-19 at 14:43 -0500, Peter Bergner wrote: > >> 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! Ok, with this patch, plus adding a powerpc implementation of htm_transaction_active(), reentrant.c now executes correctly on both HTM and non-HTM hardware for me. Thanks for all of your help with this! I'd still like to hear from Andreas, whether the reentrant.c test case with both patches, now works on S390. I'll note unlike your x86 htm_transaction_active() implementation, my implementation has to check for htm_available() first before executing the htm instruction which tells me whether I'm in transaction state or not, otherwise I'll get a SIGILL on non-HTM hardware. Unfortunately, my htm_available() call uses getauxval() to query the AUXV for a hwcap bit. Is there a place I can stash the result of the first call, so that subsequent calls use the cached value? Normally, I could use a static var, but I doubt that is what we want to do in static inline functions. Peter
On 20/06/13 05:13, Peter Bergner wrote: > On Thu, 2013-06-20 at 00:51 +0200, Torvald Riegel wrote: >> On Wed, 2013-06-19 at 14:43 -0500, Peter Bergner wrote: > I'd still like to hear from Andreas, whether the reentrant.c test case > with both patches, now works on S390. The patches fix the testcase also on s390. Thanks! Bye, -Andreas-
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; }