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

[Florian Weimer <fweimer at redhat dot com>]

* 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.

Thanks,
Florian