Re: [PATCH] powerpc: Only enable TLE with PPC_FEATURE2_HTM_NOSC

[Tulio Magno Quites Machado Filho <tuliom at ascii dot art dot br>]

Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:

> On 27/08/2018 19:44, Tulio Magno Quites Machado Filho wrote:
>> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>> 
>>> diff --git a/sysdeps/unix/sysv/linux/powerpc/elision-conf.c b/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
>>> index 906882a..508b917 100644
>>> --- a/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
>>> +++ b/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
>>> @@ -127,6 +127,30 @@ elision_init (int argc __attribute__ ((unused)),
>>>  	       TUNABLE_CALLBACK (set_elision_skip_trylock_internal_abort));
>>>  #endif
>>>  
>>> +  /* Linux from 3.9 through 4.2 does not abort HTM transaction on syscalls,
>
> In fact it should be 4.1, 4.2 is the first release which actually abort transactions
> on syscalls.

Ack.

>>> +     instead it suspend and resume it when leaving the kernel.  The
>>> +     side-effects of the syscall will always remain visible, even if the
>>> +     transaction is aborted.  This is an issue when transaction is used along
>>> +     with futex syscall, on pthread_cond_wait for instance, where the futex
>>> +     call might succeed but the transaction is rolled back leading the
>>> +     pthread_cond object in an inconsistent state.
>>> +
>>> +     GLIBC used to prevent it by always aborting a transaction before issuing
>>> +     a syscall.  Linux 4.2 also decided to abort active transaction in
>>> +     syscalls which makes the GLIBC workaround superflours.  Worse, GLIBC
>>> +     transaction abortion leads to a performance issue on recent kernels
>>> +     where the HTM state is saved/restore lazily.  By aborting a transaction
>>> +     on every syscalls, regardless whether a transaction has being initiated
>>> +     before, glibc make the kernel always save/restore HTM state (it can not
>>> +     even lazily disable it after a certain number of syscall iterations).
>>> +
>>> +     Because of this shortcoming, Lock Elision is just enabled when it has
>>> +     been explictly set (either by tunables of by a configure switch) and if
>>> +     kernel aborts HTM transactions on syscalls (PPC_FEATURE2_HTM_NOSC)  */
>>> +
>>> +  __pthread_force_elision = __pthread_force_elision &&
>>> +			    GLRO (dl_hwcap2) & PPC_FEATURE2_HTM_NOSC;
>> 
>> In other words: this patch is requiring Linux 4.3 for TLE.
>> 
>> Can't ABORT_TRANSACTION check for PPC_FEATURE2_HTM_NOSC in tcbhead_t.hwcap and
>> continue to support older Linux versions?
>> 
>
> I don't have a strong opinion here, but chatting with Breno I had the impression
> that TLE (and HTM in general) is only used on kernel with this recent fix. This
> patch is really a hack insertion that penalizes all syscall for an specific
> feature which may not be present in the kernel. It is unfortunate that kernel
> developers did not realize at feature inclusion this design for transaction 
> state is not the best one for library usage.

My point was actually related to the fact that INSTALL still mentions Linux 3.2.

I completely agree this solution is better in terms of performance and
maintenance in the long term.
Maybe we (me?) could document this problem somewhere (INSTALL?) pointing
users that want to run the latest glibc and use HTM on Linux to use
Linux >= 4.2.

With the removal of ABORT_TRANSACTION, it's also necessary to remove
sysdeps/unix/sysv/linux/powerpc/not-errno.h.

With that said, my questions have already been answered and this patch looks
good to me with the fix in the comment you mentioned and the removal of
not-errno.h.

-- 
Tulio Magno