[3/3] PowerPC: abort transaction in syscalls
Commit Message
Linux kernel powerpc documentation states issuing a syscall inside a
transaction is not recommended and may lead to undefined behavior. It
also states syscalls does not abort transactoin neither they run in
transactional state.
To avoid side-effects being visible outside transactions, GLIBC with lock
elision enable issues a transaction abort instruction just before all
syscalls if hardware supports hardware transactions.
--
* sysdeps/powerpc/nptl/tls.h (tcbhead_t): Add tm_capable field.
(TLS_INIT_TP): Add tm_capable initialization.
(TLS_DEFINE_INIT_TP): Likewise.
(THREAD_GET_TM_CAPABLE): New file: get tm_capable field value from
TCB.
(THREAD_SET_TM_CAPABLE): New file: set tm_capable field value in TCB.
* sysdeps/powerpc/nptl/tcb-offsets.sym (TM_CAPABLE): Add field offset
calculation.
* sysdeps/powerpc/powerpc32/sysdep.h (DO_CALL): Abort hardware
transactoion is lock elision is built and TCB tm_capable is set.
* sysdeps/powerpc/powerpc64/sysdep.h (DO_CALL): Likewise.
* sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h
(INTERNAL_SYSCALL_NCS): Likewise.
* sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h
(INTERNAL_SYSCALL_NCS): Likewise.
* sysdeps/powerpc/sysdep.h (ABORT_TRANSACTION): New define.
---
Comments
On 07-11-2014 15:13, Adhemerval Zanella wrote:
> Linux kernel powerpc documentation states issuing a syscall inside a
> transaction is not recommended and may lead to undefined behavior. It
> also states syscalls does not abort transactoin neither they run in
> transactional state.
>
> To avoid side-effects being visible outside transactions, GLIBC with lock
> elision enable issues a transaction abort instruction just before all
> syscalls if hardware supports hardware transactions.
>
> --
>
> * sysdeps/powerpc/nptl/tls.h (tcbhead_t): Add tm_capable field.
> (TLS_INIT_TP): Add tm_capable initialization.
> (TLS_DEFINE_INIT_TP): Likewise.
> (THREAD_GET_TM_CAPABLE): New file: get tm_capable field value from
> TCB.
> (THREAD_SET_TM_CAPABLE): New file: set tm_capable field value in TCB.
> * sysdeps/powerpc/nptl/tcb-offsets.sym (TM_CAPABLE): Add field offset
> calculation.
> * sysdeps/powerpc/powerpc32/sysdep.h (DO_CALL): Abort hardware
> transactoion is lock elision is built and TCB tm_capable is set.
> * sysdeps/powerpc/powerpc64/sysdep.h (DO_CALL): Likewise.
> * sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h
> (INTERNAL_SYSCALL_NCS): Likewise.
> * sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h
> (INTERNAL_SYSCALL_NCS): Likewise.
> * sysdeps/powerpc/sysdep.h (ABORT_TRANSACTION): New define.
>
> ---
>
> diff --git a/sysdeps/powerpc/nptl/tcb-offsets.sym b/sysdeps/powerpc/nptl/tcb-offsets.sym
> index f996759..d955142 100644
> --- a/sysdeps/powerpc/nptl/tcb-offsets.sym
> +++ b/sysdeps/powerpc/nptl/tcb-offsets.sym
> @@ -19,6 +19,7 @@ POINTER_GUARD (offsetof (tcbhead_t, pointer_guard) - TLS_TCB_OFFSET - sizeof (
> TAR_SAVE (offsetof (tcbhead_t, tar_save) - TLS_TCB_OFFSET - sizeof (tcbhead_t))
> DSO_SLOT1 (offsetof (tcbhead_t, dso_slot1) - TLS_TCB_OFFSET - sizeof (tcbhead_t))
> DSO_SLOT2 (offsetof (tcbhead_t, dso_slot2) - TLS_TCB_OFFSET - sizeof (tcbhead_t))
> +TM_CAPABLE (offsetof (tcbhead_t, tm_capable) - TLS_TCB_OFFSET - sizeof (tcbhead_t))
> #ifndef __ASSUME_PRIVATE_FUTEX
> PRIVATE_FUTEX_OFFSET thread_offsetof (header.private_futex)
> #endif
> diff --git a/sysdeps/powerpc/nptl/tls.h b/sysdeps/powerpc/nptl/tls.h
> index b80a5fb..37280d2 100644
> --- a/sysdeps/powerpc/nptl/tls.h
> +++ b/sysdeps/powerpc/nptl/tls.h
> @@ -63,6 +63,8 @@ typedef union dtv
> are private. */
> typedef struct
> {
> + /* Indicate if hwcap2 has PPC_FEATURE2_HAS_HTM capability. */
> + int tm_capable;
> /* Reservation for Dynamic System Optimizer ABI. */
> uintptr_t dso_slot2;
> uintptr_t dso_slot1;
> @@ -130,11 +132,17 @@ register void *__thread_register __asm__ ("r13");
> special attention since 'errno' is not yet available and if the
> operation can cause a failure 'errno' must not be touched. */
> # define TLS_INIT_TP(tcbp) \
> - (__thread_register = (void *) (tcbp) + TLS_TCB_OFFSET, NULL)
> + ({ \
> + __thread_register = (void *) (tcbp) + TLS_TCB_OFFSET; \
> + THREAD_SET_TM_CAPABLE (GLRO (dl_hwcap2) & PPC_FEATURE2_HAS_HTM ? 1 : 0); \
> + NULL; \
> + })
>
> /* Value passed to 'clone' for initialization of the thread register. */
> # define TLS_DEFINE_INIT_TP(tp, pd) \
> - void *tp = (void *) (pd) + TLS_TCB_OFFSET + TLS_PRE_TCB_SIZE
> + void *tp = (void *) (pd) + TLS_TCB_OFFSET + TLS_PRE_TCB_SIZE; \
> + (((tcbhead_t *) ((char *) tp - TLS_TCB_OFFSET))[-1].tm_capable) = \
> + THREAD_GET_TM_CAPABLE ();
>
> /* Return the address of the dtv for the current thread. */
> # define THREAD_DTV() \
> @@ -188,6 +196,13 @@ register void *__thread_register __asm__ ("r13");
> + TLS_PRE_TCB_SIZE))[-1].pointer_guard \
> = THREAD_GET_POINTER_GUARD())
>
> +/* tm_capable field in TCB head. */
> +# define THREAD_GET_TM_CAPABLE() \
> + (((tcbhead_t *) ((char *) __thread_register \
> + - TLS_TCB_OFFSET))[-1].tm_capable)
> +# define THREAD_SET_TM_CAPABLE(value) \
> + (THREAD_GET_TM_CAPABLE () = (value))
> +
> /* l_tls_offset == 0 is perfectly valid on PPC, so we have to use some
> different value to mean unset l_tls_offset. */
> # define NO_TLS_OFFSET -1
> diff --git a/sysdeps/powerpc/powerpc32/sysdep.h b/sysdeps/powerpc/powerpc32/sysdep.h
> index c8a56aa..c4b3ca8 100644
> --- a/sysdeps/powerpc/powerpc32/sysdep.h
> +++ b/sysdeps/powerpc/powerpc32/sysdep.h
> @@ -88,7 +88,23 @@ GOT_LABEL: ; \
> cfi_endproc; \
> ASM_SIZE_DIRECTIVE(name)
>
> +#if !defined IS_IN_rtld && defined (ENABLE_LOCK_ELISION)
> +# define ABORT_TRANSACTION \
> + cmpwi 2,0; \
> + beq 1f; \
> + lwz 0,TM_CAPABLE(2); \
> + cmpwi 0,0; \
> + beq 1f; \
> + li 0,_ABORT_SYSCALL; \
> + tabort. 0; \
> + .align 4; \
> +1:
> +#else
> +# define ABORT_TRANSACTION
> +#endif
> +
> #define DO_CALL(syscall) \
> + ABORT_TRANSACTION \
> li 0,syscall; \
> sc
>
> diff --git a/sysdeps/powerpc/powerpc64/sysdep.h b/sysdeps/powerpc/powerpc64/sysdep.h
> index b28fb9d..78722c6 100644
> --- a/sysdeps/powerpc/powerpc64/sysdep.h
> +++ b/sysdeps/powerpc/powerpc64/sysdep.h
> @@ -283,7 +283,23 @@ LT_LABELSUFFIX(name,_name_end): ; \
> TRACEBACK_MASK(name,mask) \
> END_2(name)
>
> +#if !defined IS_IN_rtld && defined (ENABLE_LOCK_ELISION)
> +# define ABORT_TRANSACTION \
> + cmpdi 13,0; \
> + beq 1f; \
> + lwz 0,TM_CAPABLE(13); \
> + cmpwi 0,0; \
> + beq 1f; \
> + li 0,_ABORT_SYSCALL; \
> + tabort. 0; \
> + .align 4; \
> +1:
> +#else
> +# define ABORT_TRANSACTION
> +#endif
> +
> #define DO_CALL(syscall) \
> + ABORT_TRANSACTION \
> li 0,syscall; \
> sc
>
> diff --git a/sysdeps/powerpc/sysdep.h b/sysdeps/powerpc/sysdep.h
> index e6627c0..c683066 100644
> --- a/sysdeps/powerpc/sysdep.h
> +++ b/sysdeps/powerpc/sysdep.h
> @@ -21,6 +21,10 @@
> */
> #define _SYSDEPS_SYSDEP_H 1
> #include <bits/hwcap.h>
> +#ifdef ENABLE_LOCK_ELISION
> +#include <tls.h>
> +#include <htm.h>
> +#endif
>
> #define PPC_FEATURE_970 (PPC_FEATURE_POWER4 + PPC_FEATURE_HAS_ALTIVEC)
>
> @@ -164,4 +168,21 @@
> #define ALIGNARG(log2) log2
> #define ASM_SIZE_DIRECTIVE(name) .size name,.-name
>
> +#else
> +
> +/* Linux kernel powerpc documentation states issuing a syscall inside a
> + transaction is not recommended and may lead to undefined behavior. It
> + also states syscalls does not abort transactoin neither run in
> + transactional state. To avoid such traps, we abort transaction just
> + before syscalls. */
> +#if !defined IS_IN_rtld && defined (ENABLE_LOCK_ELISION)
> +# define ABORT_TRANSACTION \
> + ({ \
> + if (THREAD_GET_TM_CAPABLE ()) \
> + __builtin_tabort (_ABORT_SYSCALL); \
> + })
> +#else
> +# define ABORT_TRANSACTION
> +#endif
> +
> #endif /* __ASSEMBLER__ */
> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h b/sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h
> index 1a5e37a..0947ca3 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h
> +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h
> @@ -194,6 +194,7 @@
> register long int r11 __asm__ ("r11"); \
> register long int r12 __asm__ ("r12"); \
> LOADARGS_##nr(name, args); \
> + ABORT_TRANSACTION; \
> __asm__ __volatile__ \
> ("sc \n\t" \
> "mfcr %0" \
> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h b/sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h
> index 93e454e..a3cc302 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h
> +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h
> @@ -201,6 +201,7 @@
> register long int r7 __asm__ ("r7"); \
> register long int r8 __asm__ ("r8"); \
> LOADARGS_##nr (name, ##args); \
> + ABORT_TRANSACTION; \
> __asm__ __volatile__ \
> ("sc\n\t" \
> "mfcr %0\n\t" \
>
Ping.
On Thu, 2014-11-27 at 15:36 -0200, Adhemerval Zanella wrote:
> On 07-11-2014 15:13, Adhemerval Zanella wrote:
> > Linux kernel powerpc documentation states issuing a syscall inside a
> > transaction is not recommended and may lead to undefined behavior. It
> > also states syscalls does not abort transactoin neither they run in
> > transactional state.
A few cosmetics, and questions sprinkled throughout. No issues with
code functionality.
> >
> > To avoid side-effects being visible outside transactions, GLIBC with lock
> > elision enable issues a transaction abort instruction just before all
s/enable issues/enabled will issue/
> > syscalls if hardware supports hardware transactions.
> >
> > --
> >
> > * sysdeps/powerpc/nptl/tls.h (tcbhead_t): Add tm_capable field.
> > (TLS_INIT_TP): Add tm_capable initialization.
> > (TLS_DEFINE_INIT_TP): Likewise.
> > (THREAD_GET_TM_CAPABLE): New file: get tm_capable field value from
> > TCB.
> > (THREAD_SET_TM_CAPABLE): New file: set tm_capable field value in TCB.
> > * sysdeps/powerpc/nptl/tcb-offsets.sym (TM_CAPABLE): Add field offset
> > calculation.
> > * sysdeps/powerpc/powerpc32/sysdep.h (DO_CALL): Abort hardware
> > transactoion is lock elision is built and TCB tm_capable is set.
> > * sysdeps/powerpc/powerpc64/sysdep.h (DO_CALL): Likewise.
> > * sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h
> > (INTERNAL_SYSCALL_NCS): Likewise.
> > * sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h
> > (INTERNAL_SYSCALL_NCS): Likewise.
> > * sysdeps/powerpc/sysdep.h (ABORT_TRANSACTION): New define.
> >
> > ---
> >
> > diff --git a/sysdeps/powerpc/nptl/tcb-offsets.sym b/sysdeps/powerpc/nptl/tcb-offsets.sym
> > index f996759..d955142 100644
> > --- a/sysdeps/powerpc/nptl/tcb-offsets.sym
> > +++ b/sysdeps/powerpc/nptl/tcb-offsets.sym
> > @@ -19,6 +19,7 @@ POINTER_GUARD (offsetof (tcbhead_t, pointer_guard) - TLS_TCB_OFFSET - sizeof (
> > TAR_SAVE (offsetof (tcbhead_t, tar_save) - TLS_TCB_OFFSET - sizeof (tcbhead_t))
> > DSO_SLOT1 (offsetof (tcbhead_t, dso_slot1) - TLS_TCB_OFFSET - sizeof (tcbhead_t))
> > DSO_SLOT2 (offsetof (tcbhead_t, dso_slot2) - TLS_TCB_OFFSET - sizeof (tcbhead_t))
> > +TM_CAPABLE (offsetof (tcbhead_t, tm_capable) - TLS_TCB_OFFSET - sizeof (tcbhead_t))
> > #ifndef __ASSUME_PRIVATE_FUTEX
> > PRIVATE_FUTEX_OFFSET thread_offsetof (header.private_futex)
> > #endif
> > diff --git a/sysdeps/powerpc/nptl/tls.h b/sysdeps/powerpc/nptl/tls.h
> > index b80a5fb..37280d2 100644
> > --- a/sysdeps/powerpc/nptl/tls.h
> > +++ b/sysdeps/powerpc/nptl/tls.h
> > @@ -63,6 +63,8 @@ typedef union dtv
> > are private. */
> > typedef struct
> > {
> > + /* Indicate if hwcap2 has PPC_FEATURE2_HAS_HTM capability. */
> > + int tm_capable;
The comment here is possibly a bit verbose. This field doesn't need to
care where/or how we determine the value. How about just "Indicate if
HTM capable".
> > /* Reservation for Dynamic System Optimizer ABI. */
> > uintptr_t dso_slot2;
> > uintptr_t dso_slot1;
> > @@ -130,11 +132,17 @@ register void *__thread_register __asm__ ("r13");
> > special attention since 'errno' is not yet available and if the
> > operation can cause a failure 'errno' must not be touched. */
> > # define TLS_INIT_TP(tcbp) \
> > - (__thread_register = (void *) (tcbp) + TLS_TCB_OFFSET, NULL)
> > + ({ \
> > + __thread_register = (void *) (tcbp) + TLS_TCB_OFFSET; \
> > + THREAD_SET_TM_CAPABLE (GLRO (dl_hwcap2) & PPC_FEATURE2_HAS_HTM ? 1 : 0); \
> > + NULL; \
> > + })
> >
> > /* Value passed to 'clone' for initialization of the thread register. */
> > # define TLS_DEFINE_INIT_TP(tp, pd) \
> > - void *tp = (void *) (pd) + TLS_TCB_OFFSET + TLS_PRE_TCB_SIZE
> > + void *tp = (void *) (pd) + TLS_TCB_OFFSET + TLS_PRE_TCB_SIZE; \
> > + (((tcbhead_t *) ((char *) tp - TLS_TCB_OFFSET))[-1].tm_capable) = \
> > + THREAD_GET_TM_CAPABLE ();
> >
> > /* Return the address of the dtv for the current thread. */
> > # define THREAD_DTV() \
> > @@ -188,6 +196,13 @@ register void *__thread_register __asm__ ("r13");
> > + TLS_PRE_TCB_SIZE))[-1].pointer_guard \
> > = THREAD_GET_POINTER_GUARD())
> >
> > +/* tm_capable field in TCB head. */
> > +# define THREAD_GET_TM_CAPABLE() \
> > + (((tcbhead_t *) ((char *) __thread_register \
> > + - TLS_TCB_OFFSET))[-1].tm_capable)
> > +# define THREAD_SET_TM_CAPABLE(value) \
> > + (THREAD_GET_TM_CAPABLE () = (value))
> > +
> > /* l_tls_offset == 0 is perfectly valid on PPC, so we have to use some
> > different value to mean unset l_tls_offset. */
> > # define NO_TLS_OFFSET -1
Ok.
> > diff --git a/sysdeps/powerpc/powerpc32/sysdep.h b/sysdeps/powerpc/powerpc32/sysdep.h
> > index c8a56aa..c4b3ca8 100644
> > --- a/sysdeps/powerpc/powerpc32/sysdep.h
> > +++ b/sysdeps/powerpc/powerpc32/sysdep.h
> > @@ -88,7 +88,23 @@ GOT_LABEL: ; \
> > cfi_endproc; \
> > ASM_SIZE_DIRECTIVE(name)
> >
> > +#if !defined IS_IN_rtld && defined (ENABLE_LOCK_ELISION)
> > +# define ABORT_TRANSACTION \
> > + cmpwi 2,0; \
> > + beq 1f; \
> > + lwz 0,TM_CAPABLE(2); \
> > + cmpwi 0,0; \
> > + beq 1f; \
> > + li 0,_ABORT_SYSCALL; \
> > + tabort. 0; \
> > + .align 4; \
> > +1:
> > +#else
> > +# define ABORT_TRANSACTION
> > +#endif
> > +
> > #define DO_CALL(syscall) \
> > + ABORT_TRANSACTION \
> > li 0,syscall; \
> > sc
> >
> > diff --git a/sysdeps/powerpc/powerpc64/sysdep.h b/sysdeps/powerpc/powerpc64/sysdep.h
> > index b28fb9d..78722c6 100644
> > --- a/sysdeps/powerpc/powerpc64/sysdep.h
> > +++ b/sysdeps/powerpc/powerpc64/sysdep.h
> > @@ -283,7 +283,23 @@ LT_LABELSUFFIX(name,_name_end): ; \
> > TRACEBACK_MASK(name,mask) \
> > END_2(name)
> >
> > +#if !defined IS_IN_rtld && defined (ENABLE_LOCK_ELISION)
> > +# define ABORT_TRANSACTION \
> > + cmpdi 13,0; \
> > + beq 1f; \
> > + lwz 0,TM_CAPABLE(13); \
> > + cmpwi 0,0; \
> > + beq 1f; \
> > + li 0,_ABORT_SYSCALL; \
> > + tabort. 0; \
> > + .align 4; \
> > +1:
> > +#else
> > +# define ABORT_TRANSACTION
> > +#endif
> > +
> > #define DO_CALL(syscall) \
> > + ABORT_TRANSACTION \
> > li 0,syscall; \
> > sc
> >
> > diff --git a/sysdeps/powerpc/sysdep.h b/sysdeps/powerpc/sysdep.h
> > index e6627c0..c683066 100644
> > --- a/sysdeps/powerpc/sysdep.h
> > +++ b/sysdeps/powerpc/sysdep.h
> > @@ -21,6 +21,10 @@
> > */
> > #define _SYSDEPS_SYSDEP_H 1
> > #include <bits/hwcap.h>
> > +#ifdef ENABLE_LOCK_ELISION
> > +#include <tls.h>
> > +#include <htm.h>
> > +#endif
> >
> > #define PPC_FEATURE_970 (PPC_FEATURE_POWER4 + PPC_FEATURE_HAS_ALTIVEC)
> >
> > @@ -164,4 +168,21 @@
> > #define ALIGNARG(log2) log2
> > #define ASM_SIZE_DIRECTIVE(name) .size name,.-name
> >
> > +#else
> > +
> > +/* Linux kernel powerpc documentation states issuing a syscall inside a
> > + transaction is not recommended and may lead to undefined behavior. It
pointer to documentation?
> > + also states syscalls does not abort transactoin neither run in
transaction
'neither' is possibly out of place.
> > + transactional state. To avoid such traps, we abort transaction just
> > + before syscalls. */
> > +#if !defined IS_IN_rtld && defined (ENABLE_LOCK_ELISION)
> > +# define ABORT_TRANSACTION \
> > + ({ \
> > + if (THREAD_GET_TM_CAPABLE ()) \
> > + __builtin_tabort (_ABORT_SYSCALL); \
> > + })
> > +#else
> > +# define ABORT_TRANSACTION
> > +#endif
> > +
> > #endif /* __ASSEMBLER__ */
> > diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h b/sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h
> > index 1a5e37a..0947ca3 100644
> > --- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h
> > +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h
> > @@ -194,6 +194,7 @@
> > register long int r11 __asm__ ("r11"); \
> > register long int r12 __asm__ ("r12"); \
> > LOADARGS_##nr(name, args); \
> > + ABORT_TRANSACTION; \
> > __asm__ __volatile__ \
> > ("sc \n\t" \
> > "mfcr %0" \
> > diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h b/sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h
> > index 93e454e..a3cc302 100644
> > --- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h
> > +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h
> > @@ -201,6 +201,7 @@
> > register long int r7 __asm__ ("r7"); \
> > register long int r8 __asm__ ("r8"); \
> > LOADARGS_##nr (name, ##args); \
> > + ABORT_TRANSACTION; \
> > __asm__ __volatile__ \
> > ("sc\n\t" \
> > "mfcr %0\n\t" \
> >
> Ping.
>
Hi Will,
As previously messages, I will send an updated patchset soon.
On 10-12-2014 18:20, Will Schmidt wrote:
> On Thu, 2014-11-27 at 15:36 -0200, Adhemerval Zanella wrote:
>> On 07-11-2014 15:13, Adhemerval Zanella wrote:
>>> Linux kernel powerpc documentation states issuing a syscall inside a
>>> transaction is not recommended and may lead to undefined behavior. It
>>> also states syscalls does not abort transactoin neither they run in
>>> transactional state.
> A few cosmetics, and questions sprinkled throughout. No issues with
> code functionality.
>
>
>>> To avoid side-effects being visible outside transactions, GLIBC with lock
>>> elision enable issues a transaction abort instruction just before all
> s/enable issues/enabled will issue/
Fixed.
>
>
>>> syscalls if hardware supports hardware transactions.
>>> --
>>>
>>> * sysdeps/powerpc/nptl/tls.h (tcbhead_t): Add tm_capable field.
>>> (TLS_INIT_TP): Add tm_capable initialization.
>>> (TLS_DEFINE_INIT_TP): Likewise.
>>> (THREAD_GET_TM_CAPABLE): New file: get tm_capable field value from
>>> TCB.
>>> (THREAD_SET_TM_CAPABLE): New file: set tm_capable field value in TCB.
>>> * sysdeps/powerpc/nptl/tcb-offsets.sym (TM_CAPABLE): Add field offset
>>> calculation.
>>> * sysdeps/powerpc/powerpc32/sysdep.h (DO_CALL): Abort hardware
>>> transactoion is lock elision is built and TCB tm_capable is set.
>>> * sysdeps/powerpc/powerpc64/sysdep.h (DO_CALL): Likewise.
>>> * sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h
>>> (INTERNAL_SYSCALL_NCS): Likewise.
>>> * sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h
>>> (INTERNAL_SYSCALL_NCS): Likewise.
>>> * sysdeps/powerpc/sysdep.h (ABORT_TRANSACTION): New define.
>>>
>>> ---
>>>
>>> diff --git a/sysdeps/powerpc/nptl/tcb-offsets.sym b/sysdeps/powerpc/nptl/tcb-offsets.sym
>>> index f996759..d955142 100644
>>> --- a/sysdeps/powerpc/nptl/tcb-offsets.sym
>>> +++ b/sysdeps/powerpc/nptl/tcb-offsets.sym
>>> @@ -19,6 +19,7 @@ POINTER_GUARD (offsetof (tcbhead_t, pointer_guard) - TLS_TCB_OFFSET - sizeof (
>>> TAR_SAVE (offsetof (tcbhead_t, tar_save) - TLS_TCB_OFFSET - sizeof (tcbhead_t))
>>> DSO_SLOT1 (offsetof (tcbhead_t, dso_slot1) - TLS_TCB_OFFSET - sizeof (tcbhead_t))
>>> DSO_SLOT2 (offsetof (tcbhead_t, dso_slot2) - TLS_TCB_OFFSET - sizeof (tcbhead_t))
>>> +TM_CAPABLE (offsetof (tcbhead_t, tm_capable) - TLS_TCB_OFFSET - sizeof (tcbhead_t))
>>> #ifndef __ASSUME_PRIVATE_FUTEX
>>> PRIVATE_FUTEX_OFFSET thread_offsetof (header.private_futex)
>>> #endif
>>> diff --git a/sysdeps/powerpc/nptl/tls.h b/sysdeps/powerpc/nptl/tls.h
>>> index b80a5fb..37280d2 100644
>>> --- a/sysdeps/powerpc/nptl/tls.h
>>> +++ b/sysdeps/powerpc/nptl/tls.h
>>> @@ -63,6 +63,8 @@ typedef union dtv
>>> are private. */
>>> typedef struct
>>> {
>>> + /* Indicate if hwcap2 has PPC_FEATURE2_HAS_HTM capability. */
>>> + int tm_capable;
> The comment here is possibly a bit verbose. This field doesn't need to
> care where/or how we determine the value. How about just "Indicate if
> HTM capable".
Changed.
>>
>> diff --git a/sysdeps/powerpc/sysdep.h b/sysdeps/powerpc/sysdep.h
>> index e6627c0..c683066 100644
>> --- a/sysdeps/powerpc/sysdep.h
>> +++ b/sysdeps/powerpc/sysdep.h
>> @@ -21,6 +21,10 @@
>> */
>> #define _SYSDEPS_SYSDEP_H 1
>> #include <bits/hwcap.h>
>> +#ifdef ENABLE_LOCK_ELISION
>> +#include <tls.h>
>> +#include <htm.h>
>> +#endif
>>
>> #define PPC_FEATURE_970 (PPC_FEATURE_POWER4 + PPC_FEATURE_HAS_ALTIVEC)
>>
>> @@ -164,4 +168,21 @@
>> #define ALIGNARG(log2) log2
>> #define ASM_SIZE_DIRECTIVE(name) .size name,.-name
>>
>> +#else
>> +
>> +/* Linux kernel powerpc documentation states issuing a syscall inside a
>> + transaction is not recommended and may lead to undefined behavior. It
> pointer to documentation?
>
>>> + also states syscalls does not abort transactoin neither run in
> transaction
> 'neither' is possibly out of place.
>
>>> + transactional state. To avoid such traps, we abort transaction just
>>> + before syscalls. */
Right, I will change to :
/* Linux kernel powerpc documentation [1] states issuing a syscall inside a
transaction is not recommended and may lead to undefined behavior. It
also states syscalls do not abort transactions. To avoid such traps,
we abort transaction just before syscalls.
[1] Documentation/powerpc/transactional_memory.txt [Syscalls] */
@@ -19,6 +19,7 @@ POINTER_GUARD (offsetof (tcbhead_t, pointer_guard) - TLS_TCB_OFFSET - sizeof (
TAR_SAVE (offsetof (tcbhead_t, tar_save) - TLS_TCB_OFFSET - sizeof (tcbhead_t))
DSO_SLOT1 (offsetof (tcbhead_t, dso_slot1) - TLS_TCB_OFFSET - sizeof (tcbhead_t))
DSO_SLOT2 (offsetof (tcbhead_t, dso_slot2) - TLS_TCB_OFFSET - sizeof (tcbhead_t))
+TM_CAPABLE (offsetof (tcbhead_t, tm_capable) - TLS_TCB_OFFSET - sizeof (tcbhead_t))
#ifndef __ASSUME_PRIVATE_FUTEX
PRIVATE_FUTEX_OFFSET thread_offsetof (header.private_futex)
#endif
@@ -63,6 +63,8 @@ typedef union dtv
are private. */
typedef struct
{
+ /* Indicate if hwcap2 has PPC_FEATURE2_HAS_HTM capability. */
+ int tm_capable;
/* Reservation for Dynamic System Optimizer ABI. */
uintptr_t dso_slot2;
uintptr_t dso_slot1;
@@ -130,11 +132,17 @@ register void *__thread_register __asm__ ("r13");
special attention since 'errno' is not yet available and if the
operation can cause a failure 'errno' must not be touched. */
# define TLS_INIT_TP(tcbp) \
- (__thread_register = (void *) (tcbp) + TLS_TCB_OFFSET, NULL)
+ ({ \
+ __thread_register = (void *) (tcbp) + TLS_TCB_OFFSET; \
+ THREAD_SET_TM_CAPABLE (GLRO (dl_hwcap2) & PPC_FEATURE2_HAS_HTM ? 1 : 0); \
+ NULL; \
+ })
/* Value passed to 'clone' for initialization of the thread register. */
# define TLS_DEFINE_INIT_TP(tp, pd) \
- void *tp = (void *) (pd) + TLS_TCB_OFFSET + TLS_PRE_TCB_SIZE
+ void *tp = (void *) (pd) + TLS_TCB_OFFSET + TLS_PRE_TCB_SIZE; \
+ (((tcbhead_t *) ((char *) tp - TLS_TCB_OFFSET))[-1].tm_capable) = \
+ THREAD_GET_TM_CAPABLE ();
/* Return the address of the dtv for the current thread. */
# define THREAD_DTV() \
@@ -188,6 +196,13 @@ register void *__thread_register __asm__ ("r13");
+ TLS_PRE_TCB_SIZE))[-1].pointer_guard \
= THREAD_GET_POINTER_GUARD())
+/* tm_capable field in TCB head. */
+# define THREAD_GET_TM_CAPABLE() \
+ (((tcbhead_t *) ((char *) __thread_register \
+ - TLS_TCB_OFFSET))[-1].tm_capable)
+# define THREAD_SET_TM_CAPABLE(value) \
+ (THREAD_GET_TM_CAPABLE () = (value))
+
/* l_tls_offset == 0 is perfectly valid on PPC, so we have to use some
different value to mean unset l_tls_offset. */
# define NO_TLS_OFFSET -1
@@ -88,7 +88,23 @@ GOT_LABEL: ; \
cfi_endproc; \
ASM_SIZE_DIRECTIVE(name)
+#if !defined IS_IN_rtld && defined (ENABLE_LOCK_ELISION)
+# define ABORT_TRANSACTION \
+ cmpwi 2,0; \
+ beq 1f; \
+ lwz 0,TM_CAPABLE(2); \
+ cmpwi 0,0; \
+ beq 1f; \
+ li 0,_ABORT_SYSCALL; \
+ tabort. 0; \
+ .align 4; \
+1:
+#else
+# define ABORT_TRANSACTION
+#endif
+
#define DO_CALL(syscall) \
+ ABORT_TRANSACTION \
li 0,syscall; \
sc
@@ -283,7 +283,23 @@ LT_LABELSUFFIX(name,_name_end): ; \
TRACEBACK_MASK(name,mask) \
END_2(name)
+#if !defined IS_IN_rtld && defined (ENABLE_LOCK_ELISION)
+# define ABORT_TRANSACTION \
+ cmpdi 13,0; \
+ beq 1f; \
+ lwz 0,TM_CAPABLE(13); \
+ cmpwi 0,0; \
+ beq 1f; \
+ li 0,_ABORT_SYSCALL; \
+ tabort. 0; \
+ .align 4; \
+1:
+#else
+# define ABORT_TRANSACTION
+#endif
+
#define DO_CALL(syscall) \
+ ABORT_TRANSACTION \
li 0,syscall; \
sc
@@ -21,6 +21,10 @@
*/
#define _SYSDEPS_SYSDEP_H 1
#include <bits/hwcap.h>
+#ifdef ENABLE_LOCK_ELISION
+#include <tls.h>
+#include <htm.h>
+#endif
#define PPC_FEATURE_970 (PPC_FEATURE_POWER4 + PPC_FEATURE_HAS_ALTIVEC)
@@ -164,4 +168,21 @@
#define ALIGNARG(log2) log2
#define ASM_SIZE_DIRECTIVE(name) .size name,.-name
+#else
+
+/* Linux kernel powerpc documentation states issuing a syscall inside a
+ transaction is not recommended and may lead to undefined behavior. It
+ also states syscalls does not abort transactoin neither run in
+ transactional state. To avoid such traps, we abort transaction just
+ before syscalls. */
+#if !defined IS_IN_rtld && defined (ENABLE_LOCK_ELISION)
+# define ABORT_TRANSACTION \
+ ({ \
+ if (THREAD_GET_TM_CAPABLE ()) \
+ __builtin_tabort (_ABORT_SYSCALL); \
+ })
+#else
+# define ABORT_TRANSACTION
+#endif
+
#endif /* __ASSEMBLER__ */
@@ -194,6 +194,7 @@
register long int r11 __asm__ ("r11"); \
register long int r12 __asm__ ("r12"); \
LOADARGS_##nr(name, args); \
+ ABORT_TRANSACTION; \
__asm__ __volatile__ \
("sc \n\t" \
"mfcr %0" \
@@ -201,6 +201,7 @@
register long int r7 __asm__ ("r7"); \
register long int r8 __asm__ ("r8"); \
LOADARGS_##nr (name, ##args); \
+ ABORT_TRANSACTION; \
__asm__ __volatile__ \
("sc\n\t" \
"mfcr %0\n\t" \