| Message ID | 6ebcb9f982f7cb100aef954cd7dc6a6081cd9134.1426740212.git.sam.bobroff@au1.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On Thu, 2015-03-19 at 15:43 +1100, Sam Bobroff wrote: > This patch changes the syscall handler to doom (tabort) active > transactions when a syscall is made and return immediately without > performing the syscall. > > Currently, the system call instruction automatically suspends an > active transaction which causes side effects to persist when an active > transaction fails. > > This does change the kernel's behaviour, but in a way that was > documented as unsupported. It doesn't reduce functionality because > syscalls will still be performed after tsuspend. It also provides a > consistent interface and makes the behaviour of user code > substantially the same across powerpc and platforms that do not > support suspended transactions (e.g. x86 and s390). > > Performance measurements using > http://ozlabs.org/~anton/junkcode/null_syscall.c > indicate the cost of a system call increases by about 0.5%. > > Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com> Thanks Sam! Acked-By: Michael Neuling <mikey@neuling.org> > --- > Documentation/powerpc/transactional_memory.txt | 33 ++++++++++++------------ > arch/powerpc/include/uapi/asm/tm.h | 2 +- > arch/powerpc/kernel/entry_64.S | 19 ++++++++++++++ > 3 files changed, 37 insertions(+), 17 deletions(-) > > diff --git a/Documentation/powerpc/transactional_memory.txt b/Documentation/powerpc/transactional_memory.txt > index 9791e98..4167bc2 100644 > --- a/Documentation/powerpc/transactional_memory.txt > +++ b/Documentation/powerpc/transactional_memory.txt > @@ -74,22 +74,23 @@ Causes of transaction aborts > Syscalls > ======== > > -Performing syscalls from within transaction is not recommended, and can lead > -to unpredictable results. > - > -Syscalls do not by design abort transactions, but beware: The kernel code will > -not be running in transactional state. The effect of syscalls will always > -remain visible, but depending on the call they may abort your transaction as a > -side-effect, read soon-to-be-aborted transactional data that should not remain > -invisible, etc. If you constantly retry a transaction that constantly aborts > -itself by calling a syscall, you'll have a livelock & make no progress. > - > -Simple syscalls (e.g. sigprocmask()) "could" be OK. Even things like write() > -from, say, printf() should be OK as long as the kernel does not access any > -memory that was accessed transactionally. > - > -Consider any syscalls that happen to work as debug-only -- not recommended for > -production use. Best to queue them up till after the transaction is over. > +Syscalls made from within an active transaction will not be performed and the > +transaction will be doomed by the kernel with the failure code TM_CAUSE_SYSCALL > +| TM_CAUSE_PERSISTENT. > + > +Syscalls made from within a suspended transaction are performed as normal and > +the transaction is not explicitly doomed by the kernel. However, what the > +kernel does to perform the syscall may result in the transaction being doomed > +by the hardware. The syscall is performed in suspended mode so any side > +effects will be persistent, independent of transaction success or failure. No > +guarantees are provided by the kernel about which syscalls will affect > +transaction success. > + > +Care must be taken when relying on syscalls to abort during active transactions > +if the calls are made via a library. Libraries may cache values (which may > +give the appearence of success) or perform operations that cause transaction > +failure before entering the kernel (which may produce different failure codes). > +Examples are glibc's getpid() and lazy symbol resolution. > > > Signals > diff --git a/arch/powerpc/include/uapi/asm/tm.h b/arch/powerpc/include/uapi/asm/tm.h > index 5d836b7..5047659 100644 > --- a/arch/powerpc/include/uapi/asm/tm.h > +++ b/arch/powerpc/include/uapi/asm/tm.h > @@ -11,7 +11,7 @@ > #define TM_CAUSE_RESCHED 0xde > #define TM_CAUSE_TLBI 0xdc > #define TM_CAUSE_FAC_UNAV 0xda > -#define TM_CAUSE_SYSCALL 0xd8 /* future use */ > +#define TM_CAUSE_SYSCALL 0xd8 > #define TM_CAUSE_MISC 0xd6 /* future use */ > #define TM_CAUSE_SIGNAL 0xd4 > #define TM_CAUSE_ALIGNMENT 0xd2 > diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S > index d180caf2..85bf81d 100644 > --- a/arch/powerpc/kernel/entry_64.S > +++ b/arch/powerpc/kernel/entry_64.S > @@ -34,6 +34,7 @@ > #include <asm/ftrace.h> > #include <asm/hw_irq.h> > #include <asm/context_tracking.h> > +#include <asm/tm.h> > > /* > * System calls. > @@ -145,6 +146,24 @@ END_FW_FTR_SECTION_IFSET(FW_FEATURE_SPLPAR) > andi. r11,r10,_TIF_SYSCALL_DOTRACE > bne syscall_dotrace > .Lsyscall_dotrace_cont: > +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM > +BEGIN_FTR_SECTION > + b 1f > +END_FTR_SECTION_IFCLR(CPU_FTR_TM) > + extrdi. r11, r12, 1, (63-MSR_TS_T_LG) /* transaction active? */ > + beq+ 1f > + > + /* Doom the transaction and don't perform the syscall: */ > + mfmsr r11 > + li r12, 1 > + rldimi r11, r12, MSR_TM_LG, 63-MSR_TM_LG > + mtmsrd r11, 0 > + li r11, (TM_CAUSE_SYSCALL|TM_CAUSE_PERSISTENT) > + tabort. r11 > + > + b .Lsyscall_exit > +1: > +#endif > cmpldi 0,r0,NR_syscalls > bge- syscall_enosys >
On 03/19/2015 10:13 AM, Sam Bobroff wrote: > This patch changes the syscall handler to doom (tabort) active > transactions when a syscall is made and return immediately without > performing the syscall. > > Currently, the system call instruction automatically suspends an > active transaction which causes side effects to persist when an active > transaction fails. > > This does change the kernel's behaviour, but in a way that was > documented as unsupported. It doesn't reduce functionality because > syscalls will still be performed after tsuspend. It also provides a > consistent interface and makes the behaviour of user code > substantially the same across powerpc and platforms that do not > support suspended transactions (e.g. x86 and s390). > > Performance measurements using > http://ozlabs.org/~anton/junkcode/null_syscall.c > indicate the cost of a system call increases by about 0.5%. Performance test results verified which shows an improvement of around ~0.52% with the patch compared to without it. [With patch] null_syscall: 757.59 cycles 100.00% [Without patch]] null_syscall: 753.66 cycles 100.00% > > Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com> Tested-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
On Fri, 2015-03-20 at 14:34 +0530, Anshuman Khandual wrote: > On 03/19/2015 10:13 AM, Sam Bobroff wrote: > > This patch changes the syscall handler to doom (tabort) active > > transactions when a syscall is made and return immediately without > > performing the syscall. > > > > Currently, the system call instruction automatically suspends an > > active transaction which causes side effects to persist when an active > > transaction fails. > > > > This does change the kernel's behaviour, but in a way that was > > documented as unsupported. It doesn't reduce functionality because > > syscalls will still be performed after tsuspend. It also provides a > > consistent interface and makes the behaviour of user code > > substantially the same across powerpc and platforms that do not > > support suspended transactions (e.g. x86 and s390). > > > > Performance measurements using > > http://ozlabs.org/~anton/junkcode/null_syscall.c > > indicate the cost of a system call increases by about 0.5%. > > Performance test results verified which shows an improvement of > around ~0.52% with the patch compared to without it. > > [With patch] null_syscall: 757.59 cycles 100.00% > [Without patch]] null_syscall: 753.66 cycles 100.00% No that's a performance *decrease* with the patch applied. ? cheers
On 03/24/2015 07:34 AM, Michael Ellerman wrote: > On Fri, 2015-03-20 at 14:34 +0530, Anshuman Khandual wrote: >> On 03/19/2015 10:13 AM, Sam Bobroff wrote: >>> This patch changes the syscall handler to doom (tabort) active >>> transactions when a syscall is made and return immediately without >>> performing the syscall. >>> >>> Currently, the system call instruction automatically suspends an >>> active transaction which causes side effects to persist when an active >>> transaction fails. >>> >>> This does change the kernel's behaviour, but in a way that was >>> documented as unsupported. It doesn't reduce functionality because >>> syscalls will still be performed after tsuspend. It also provides a >>> consistent interface and makes the behaviour of user code >>> substantially the same across powerpc and platforms that do not >>> support suspended transactions (e.g. x86 and s390). >>> >>> Performance measurements using >>> http://ozlabs.org/~anton/junkcode/null_syscall.c >>> indicate the cost of a system call increases by about 0.5%. >> >> Performance test results verified which shows an improvement of >> around ~0.52% with the patch compared to without it. >> >> [With patch] null_syscall: 757.59 cycles 100.00% >> [Without patch]] null_syscall: 753.66 cycles 100.00% > > No that's a performance *decrease* with the patch applied. ? Micheal, thats true. The cycle counts are more with patch being applied. Here are some more results on mainline. Without the patch: null_syscall: 754.14 cycles 100.00% null_syscall: 754.50 cycles 100.00% null_syscall: 754.17 cycles 100.00% null_syscall: 754.43 cycles 100.00% null_syscall: 754.17 cycles 100.00% null_syscall: 754.20 cycles 100.00% null_syscall: 754.47 cycles 100.00% null_syscall: 754.70 cycles 100.00% null_syscall: 754.43 cycles 100.00% null_syscall: 753.72 cycles 100.00% With the patch: null_syscall: 756.76 cycles 100.00% null_syscall: 756.40 cycles 100.00% null_syscall: 756.46 cycles 100.00% null_syscall: 756.86 cycles 100.00% null_syscall: 756.56 cycles 100.00% null_syscall: 756.66 cycles 100.00% null_syscall: 756.86 cycles 100.00% null_syscall: 756.86 cycles 100.00% null_syscall: 757.02 cycles 100.00% null_syscall: 756.93 cycles 100.00%
On Tue, 2015-03-24 at 09:56 +0530, Anshuman Khandual wrote: > On 03/24/2015 07:34 AM, Michael Ellerman wrote: > > On Fri, 2015-03-20 at 14:34 +0530, Anshuman Khandual wrote: > >> On 03/19/2015 10:13 AM, Sam Bobroff wrote: > >>> This patch changes the syscall handler to doom (tabort) active > >>> transactions when a syscall is made and return immediately without > >>> performing the syscall. > >>> > >>> Currently, the system call instruction automatically suspends an > >>> active transaction which causes side effects to persist when an active > >>> transaction fails. > >>> > >>> This does change the kernel's behaviour, but in a way that was > >>> documented as unsupported. It doesn't reduce functionality because > >>> syscalls will still be performed after tsuspend. It also provides a > >>> consistent interface and makes the behaviour of user code > >>> substantially the same across powerpc and platforms that do not > >>> support suspended transactions (e.g. x86 and s390). > >>> > >>> Performance measurements using > >>> http://ozlabs.org/~anton/junkcode/null_syscall.c > >>> indicate the cost of a system call increases by about 0.5%. > >> > >> Performance test results verified which shows an improvement of > >> around ~0.52% with the patch compared to without it. > >> > >> [With patch] null_syscall: 757.59 cycles 100.00% > >> [Without patch]] null_syscall: 753.66 cycles 100.00% > > > > No that's a performance *decrease* with the patch applied. ? > > Micheal, thats true. The cycle counts are more with patch being applied. > Here are some more results on mainline. Right, more cycles == worse :) But it's not much more so we'll merge it. cheers
diff --git a/Documentation/powerpc/transactional_memory.txt b/Documentation/powerpc/transactional_memory.txt index 9791e98..4167bc2 100644 --- a/Documentation/powerpc/transactional_memory.txt +++ b/Documentation/powerpc/transactional_memory.txt @@ -74,22 +74,23 @@ Causes of transaction aborts Syscalls ======== -Performing syscalls from within transaction is not recommended, and can lead -to unpredictable results. - -Syscalls do not by design abort transactions, but beware: The kernel code will -not be running in transactional state. The effect of syscalls will always -remain visible, but depending on the call they may abort your transaction as a -side-effect, read soon-to-be-aborted transactional data that should not remain -invisible, etc. If you constantly retry a transaction that constantly aborts -itself by calling a syscall, you'll have a livelock & make no progress. - -Simple syscalls (e.g. sigprocmask()) "could" be OK. Even things like write() -from, say, printf() should be OK as long as the kernel does not access any -memory that was accessed transactionally. - -Consider any syscalls that happen to work as debug-only -- not recommended for -production use. Best to queue them up till after the transaction is over. +Syscalls made from within an active transaction will not be performed and the +transaction will be doomed by the kernel with the failure code TM_CAUSE_SYSCALL +| TM_CAUSE_PERSISTENT. + +Syscalls made from within a suspended transaction are performed as normal and +the transaction is not explicitly doomed by the kernel. However, what the +kernel does to perform the syscall may result in the transaction being doomed +by the hardware. The syscall is performed in suspended mode so any side +effects will be persistent, independent of transaction success or failure. No +guarantees are provided by the kernel about which syscalls will affect +transaction success. + +Care must be taken when relying on syscalls to abort during active transactions +if the calls are made via a library. Libraries may cache values (which may +give the appearence of success) or perform operations that cause transaction +failure before entering the kernel (which may produce different failure codes). +Examples are glibc's getpid() and lazy symbol resolution. Signals diff --git a/arch/powerpc/include/uapi/asm/tm.h b/arch/powerpc/include/uapi/asm/tm.h index 5d836b7..5047659 100644 --- a/arch/powerpc/include/uapi/asm/tm.h +++ b/arch/powerpc/include/uapi/asm/tm.h @@ -11,7 +11,7 @@ #define TM_CAUSE_RESCHED 0xde #define TM_CAUSE_TLBI 0xdc #define TM_CAUSE_FAC_UNAV 0xda -#define TM_CAUSE_SYSCALL 0xd8 /* future use */ +#define TM_CAUSE_SYSCALL 0xd8 #define TM_CAUSE_MISC 0xd6 /* future use */ #define TM_CAUSE_SIGNAL 0xd4 #define TM_CAUSE_ALIGNMENT 0xd2 diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S index d180caf2..85bf81d 100644 --- a/arch/powerpc/kernel/entry_64.S +++ b/arch/powerpc/kernel/entry_64.S @@ -34,6 +34,7 @@ #include <asm/ftrace.h> #include <asm/hw_irq.h> #include <asm/context_tracking.h> +#include <asm/tm.h> /* * System calls. @@ -145,6 +146,24 @@ END_FW_FTR_SECTION_IFSET(FW_FEATURE_SPLPAR) andi. r11,r10,_TIF_SYSCALL_DOTRACE bne syscall_dotrace .Lsyscall_dotrace_cont: +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM +BEGIN_FTR_SECTION + b 1f +END_FTR_SECTION_IFCLR(CPU_FTR_TM) + extrdi. r11, r12, 1, (63-MSR_TS_T_LG) /* transaction active? */ + beq+ 1f + + /* Doom the transaction and don't perform the syscall: */ + mfmsr r11 + li r12, 1 + rldimi r11, r12, MSR_TM_LG, 63-MSR_TM_LG + mtmsrd r11, 0 + li r11, (TM_CAUSE_SYSCALL|TM_CAUSE_PERSISTENT) + tabort. r11 + + b .Lsyscall_exit +1: +#endif cmpldi 0,r0,NR_syscalls bge- syscall_enosys
This patch changes the syscall handler to doom (tabort) active transactions when a syscall is made and return immediately without performing the syscall. Currently, the system call instruction automatically suspends an active transaction which causes side effects to persist when an active transaction fails. This does change the kernel's behaviour, but in a way that was documented as unsupported. It doesn't reduce functionality because syscalls will still be performed after tsuspend. It also provides a consistent interface and makes the behaviour of user code substantially the same across powerpc and platforms that do not support suspended transactions (e.g. x86 and s390). Performance measurements using http://ozlabs.org/~anton/junkcode/null_syscall.c indicate the cost of a system call increases by about 0.5%. Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com> --- Documentation/powerpc/transactional_memory.txt | 33 ++++++++++++------------ arch/powerpc/include/uapi/asm/tm.h | 2 +- arch/powerpc/kernel/entry_64.S | 19 ++++++++++++++ 3 files changed, 37 insertions(+), 17 deletions(-)