This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] powerpc: Only enable TLE with PPC_FEATURE2_HTM_NOSC
On 20/09/2018 23:13, Florian Weimer wrote:
> * Adhemerval Zanella:
>
>> +* For powercp64le ABI, Transactional Lock Elision is now enabled iff kernel
>> + aborts the transaction prior kernel execution (PPC_FEATURE2_HTM_NOSC on
>> + hwcap2). On older kernels the transaction is suspended, and this caused
>> + some undefined side-effects issues when transactions relies on other
>> + synchronization mechanisms (futex for instance). GLIBC avoided it by
>> + abort transactions manually on each syscall, but it lead to performance
>> + issues on newer kernels where the HTM state is saved and restore lazily
>> + (the state being saved even when the process actually does not use HTM).
>
> I'd prefer “enabled iff the kernel indicates that it will abort the
> transaction prior to entering the kernel (PPC_FEATURE2_HTM_NOSC on
> hwcap2)” and “when transactions relie*d*”, “by aborting transactions
> manually”, “but it led”, “saved and restored”.
>
> And “GLIBC” should perhaps be spelled “glibc”.
>
>> diff --git a/sysdeps/unix/sysv/linux/powerpc/elision-conf.c b/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
>> index 906882a65e..60b19482bb 100644
>> --- a/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
>> +++ b/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
>> @@ -70,7 +70,7 @@ do_set_elision_enable (int32_t elision_enable)
>> & PPC_FEATURE2_HAS_HTM) ? 1 : 0;
>> }
>>
>> -/* The pthread->elision_enable tunable is 0 or 1 indicating that elision
>> +/* The pthread->elision_enable tunable is 0 or 1 indicating that elisionGLRO (dl_hwcap2) & PPC_FEATURE2_HTM_NOSC;
>> should be disabled or enabled respectively. The feature will only be used
>> if it's supported by the hardware. */
>
> Unrelated accidental change.
>
>>
>> @@ -127,6 +127,26 @@ elision_init (int argc __attribute__ ((unused)),
>> TUNABLE_CALLBACK (set_elision_skip_trylock_internal_abort));
>> #endif
>>
>> + /* Linux from 3.9 through 4.2 do not abort HTM transaction on syscalls,
>> + instead it suspend the transaction and resumes it when returning to
>
> “instead it suspends”
>
>> + usercode. The side-effects of the syscall will always remain visible,
>> + even if the transaction is aborted. This is an issue when transaction
>
> “when a transaction”
>
>> + is used along with futex syscall, on pthread_cond_wait for instance,
>> + where futex might succeed but the transaction is rolled back leading
>> + the pthread_cond object in an inconsistent state.
>
> “pthread_cond_t” (or “condition variable”).
>
>> + 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 superflouts. Worse, GLIBC
>> + transaction abortion leads to a performance issue on recent kernels.
>
> See above about GLIBC vs glibc. And “superfluous”, “glibc transaction
> abort*s* lead**”.
>
> The actual change looks good to me, based on the explanation in the
> patch.
Based on previous Tulio's ack, I fixed the points you and Joseph raised
and push upstream.