This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]