Message ID | 55C4C152.8030506@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
LGTM, thanks for checking on that. > Em 07/08/2015, às 16:31, Paul E. Murphy <murphyp@linux.vnet.ibm.com> escreveu: > > Fix usage of tabort in generated syscalls. r0 has special meaning > when used with this instruction, thus it will not generate > persistent errors, nor return an error code. This mitigates poor > CPU usage when performing elided critical sections. Kinda curious, could you elaborate about this performance issue? > > Additionally, transactions should be aborted when entering a user > invoked syscall. Otherwise the results of the transaction may be > undefined. > > 2015-08-03 Paul E. Murphy <murphyp@linux.vnet.ibm.com> > > * sysdeps/powerpc/powerpc32/sysdep.h (ABORT_TRANSACTION): Use > register other than r0 for tabort, it has special meaning. > * sysdeps/powerpc/powerpc64/sysdep.h (ABORT_TRANSACTION): Likewise > * sysdeps/unix.sysv/linux/powerpc/syscall.S (syscall): Abort > transaction before starting syscall. > --- > sysdeps/powerpc/powerpc32/sysdep.h | 4 ++-- > sysdeps/powerpc/powerpc64/sysdep.h | 4 ++-- > sysdeps/unix/sysv/linux/powerpc/syscall.S | 1 + > 3 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/sysdeps/powerpc/powerpc32/sysdep.h b/sysdeps/powerpc/powerpc32/sysdep.h > index e16fe3e..ecb492a 100644 > --- a/sysdeps/powerpc/powerpc32/sysdep.h > +++ b/sysdeps/powerpc/powerpc32/sysdep.h > @@ -95,8 +95,8 @@ GOT_LABEL: ; \ > lwz 0,TM_CAPABLE(2); \ > cmpwi 0,0; \ > beq 1f; \ > - li 0,_ABORT_SYSCALL; \ > - tabort. 0; \ > + li 11,_ABORT_SYSCALL; \ > + tabort. 11; \ > .align 4; \ > 1: > #else > diff --git a/sysdeps/powerpc/powerpc64/sysdep.h b/sysdeps/powerpc/powerpc64/sysdep.h > index bf2a884..a9d37ad 100644 > --- a/sysdeps/powerpc/powerpc64/sysdep.h > +++ b/sysdeps/powerpc/powerpc64/sysdep.h > @@ -279,8 +279,8 @@ LT_LABELSUFFIX(name,_name_end): ; \ > lwz 0,TM_CAPABLE(13); \ > cmpwi 0,0; \ > beq 1f; \ > - li 0,_ABORT_SYSCALL; \ > - tabort. 0; \ > + li 11,_ABORT_SYSCALL; \ > + tabort. 11; \ > .align 4; \ > 1: > #else > diff --git a/sysdeps/unix/sysv/linux/powerpc/syscall.S b/sysdeps/unix/sysv/linux/powerpc/syscall.S > index 157e3e3..4477303 100644 > --- a/sysdeps/unix/sysv/linux/powerpc/syscall.S > +++ b/sysdeps/unix/sysv/linux/powerpc/syscall.S > @@ -18,6 +18,7 @@ > #include <sysdep.h> > ENTRY (syscall) > + ABORT_TRANSACTION > mr r0,r3 > mr r3,r4 > mr r4,r5 > -- > 1.7.1 >
On 08/08/2015 01:06 AM, Adhemerval Zanella wrote: > LGTM, thanks for checking on that. > > > >> Em 07/08/2015, às 16:31, Paul E. Murphy <murphyp@linux.vnet.ibm.com> escreveu: >> >> Fix usage of tabort in generated syscalls. r0 has special meaning >> when used with this instruction, thus it will not generate >> persistent errors, nor return an error code. This mitigates poor >> CPU usage when performing elided critical sections. > Kinda curious, could you elaborate about this performance issue? Profiling the case where a lock is used to protect a system call resulted in much higher CPU usage than expected. Distilling the issue into a test case of the form: pthread_mutex_lock(&lock); write( fd, buf, len ); pthread_mutex_unlock(&lock); This showed about 1 abort per lock, when it should be 1 abort per 4 locks with the current adaptive lock settings.
On Fri, 2015-08-07 at 09:31 -0500, Paul E. Murphy wrote: > Fix usage of tabort in generated syscalls. r0 has special meaning > when used with this instruction, thus it will not generate > persistent errors, nor return an error code. This mitigates poor > CPU usage when performing elided critical sections. > > Additionally, transactions should be aborted when entering a user > invoked syscall. Otherwise the results of the transaction may be > undefined. > > 2015-08-03 Paul E. Murphy <murphyp@linux.vnet.ibm.com> > > * sysdeps/powerpc/powerpc32/sysdep.h (ABORT_TRANSACTION): Use > register other than r0 for tabort, it has special meaning. > * sysdeps/powerpc/powerpc64/sysdep.h (ABORT_TRANSACTION): Likewise > * sysdeps/unix.sysv/linux/powerpc/syscall.S (syscall): Abort > transaction before starting syscall. > --- LGTM and require for reliable function, Please apply
"Paul E. Murphy" <murphyp@linux.vnet.ibm.com> writes: > Fix usage of tabort in generated syscalls. r0 has special meaning > when used with this instruction, thus it will not generate > persistent errors, nor return an error code. This mitigates poor > CPU usage when performing elided critical sections. > > Additionally, transactions should be aborted when entering a user > invoked syscall. Otherwise the results of the transaction may be > undefined. This patch looks very good in theory, but it seems your mail client broke the patch and git is now complaining. Could you send a new version, please?
diff --git a/sysdeps/powerpc/powerpc32/sysdep.h b/sysdeps/powerpc/powerpc32/sysdep.h index e16fe3e..ecb492a 100644 --- a/sysdeps/powerpc/powerpc32/sysdep.h +++ b/sysdeps/powerpc/powerpc32/sysdep.h @@ -95,8 +95,8 @@ GOT_LABEL: ; \ lwz 0,TM_CAPABLE(2); \ cmpwi 0,0; \ beq 1f; \ - li 0,_ABORT_SYSCALL; \ - tabort. 0; \ + li 11,_ABORT_SYSCALL; \ + tabort. 11; \ .align 4; \ 1: #else diff --git a/sysdeps/powerpc/powerpc64/sysdep.h b/sysdeps/powerpc/powerpc64/sysdep.h index bf2a884..a9d37ad 100644 --- a/sysdeps/powerpc/powerpc64/sysdep.h +++ b/sysdeps/powerpc/powerpc64/sysdep.h @@ -279,8 +279,8 @@ LT_LABELSUFFIX(name,_name_end): ; \ lwz 0,TM_CAPABLE(13); \ cmpwi 0,0; \ beq 1f; \ - li 0,_ABORT_SYSCALL; \ - tabort. 0; \ + li 11,_ABORT_SYSCALL; \ + tabort. 11; \ .align 4; \ 1: #else diff --git a/sysdeps/unix/sysv/linux/powerpc/syscall.S b/sysdeps/unix/sysv/linux/powerpc/syscall.S index 157e3e3..4477303 100644 --- a/sysdeps/unix/sysv/linux/powerpc/syscall.S +++ b/sysdeps/unix/sysv/linux/powerpc/syscall.S @@ -18,6 +18,7 @@ #include <sysdep.h> ENTRY (syscall) + ABORT_TRANSACTION mr r0,r3 mr r3,r4 mr r4,r5 -- 1.7.1