Message ID | 545CFDAE.1060401@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
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] */
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" \