This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] powerpc: More elision improvements
- From: Carlos Eduardo Seo <cseo at linux dot vnet dot ibm dot com>
- To: "Paul E. Murphy" <murphyp at linux dot vnet dot ibm dot com>
- Cc: "libc-alpha at sourceware dot org" <libc-alpha at sourceware dot org>, Tulio Machado <tuliom at linux dot vnet dot ibm dot com>, Torvald Riegel <triegel at redhat dot com>, Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- Date: Fri, 30 Oct 2015 11:42:14 -0200
- Subject: Re: [PATCH] powerpc: More elision improvements
- Authentication-results: sourceware.org; auth=none
- References: <56314D77 dot 5080703 at linux dot vnet dot ibm dot com>
LGTM.
--
Carlos Eduardo Seo
Software Engineer - Linux on Power Toolchain
cseo@linux.vnet.ibm.com
> On Oct 28, 2015, at 8:34 PM, Paul E. Murphy <murphyp@linux.vnet.ibm.com> wrote:
>
> __lll_trylock_elision sets the adapt_count variable too
> aggressively, and incorrectly on persistent aborts. Taking
> a cue from s390, adapt_count is only updated if the lock
> is locked, or a persistent failure occurs.
>
> In addition, the abort codes have been renumbered and
> refactored for clarity. As it stands, glibc only cares
> if the abort is persistent or not.
>
> All aborts are now persistent, excepting a busy lock. This
> includes changing _ABORT_NESTED_TRYLOCK into a persistent
> abort.
>
> 2015-10-28 Paul E. Murphy <murphyp@linux.vnet.ibm.com>
>
> * sysdeps/unix/sysv/linux/powerpc/elision-trylock.c
> (__lll_trylock_elision): Fix setting of adapt_count.
> * sysdeps/unix/sysv/linux/powerpc/htm.h
> (_ABORT_PERSISTENT): Define to clarify persistent aborts.
> (_ABORT_NESTED_TRYLOCK): Renumber, and make persistent.
> (_ABORT_SYSCALL): Renumber, and clarify definition.
> (_ABORT_LOCK_BUSY): Renumber, make non-persistent.
> ---
> sysdeps/unix/sysv/linux/powerpc/elision-trylock.c | 11 ++++++-----
> sysdeps/unix/sysv/linux/powerpc/htm.h | 12 +++++++-----
> 2 files changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/sysdeps/unix/sysv/linux/powerpc/elision-trylock.c b/sysdeps/unix/sysv/linux/powerpc/elision-trylock.c
> index 440939c..6f61eba 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/elision-trylock.c
> +++ b/sysdeps/unix/sysv/linux/powerpc/elision-trylock.c
> @@ -44,8 +44,12 @@ __lll_trylock_elision (int *futex, short *adapt_count)
> if (*futex == 0)
> return 0;
>
> - /* Lock was busy. Fall back to normal locking. */
> - __builtin_tabort (_ABORT_LOCK_BUSY);
> + /* Lock was busy. This is never a nested transaction.
> + End it, and set the adapt count. */
> + __builtin_tend (0);
> +
> + if (aconf.skip_lock_busy > 0)
> + *adapt_count = aconf.skip_lock_busy;
> }
> else
> {
> @@ -57,9 +61,6 @@ __lll_trylock_elision (int *futex, short *adapt_count)
> if (aconf.skip_trylock_internal_abort > 0)
> *adapt_count = aconf.skip_trylock_internal_abort;
> }
> -
> - if (aconf.skip_lock_busy > 0)
> - *adapt_count = aconf.skip_lock_busy;
> }
>
> use_lock:
> diff --git a/sysdeps/unix/sysv/linux/powerpc/htm.h b/sysdeps/unix/sysv/linux/powerpc/htm.h
> index 57d5cd6..5127b4b 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/htm.h
> +++ b/sysdeps/unix/sysv/linux/powerpc/htm.h
> @@ -129,10 +129,12 @@
>
> #endif /* __ASSEMBLER__ */
>
> -/* Definitions used for TEXASR Failure code (bits 0:6), they need to be even
> - because tabort. always sets the first bit. */
> -#define _ABORT_LOCK_BUSY 0x3f /* Lock already used. */
> -#define _ABORT_NESTED_TRYLOCK 0x3e /* Write operation in trylock. */
> -#define _ABORT_SYSCALL 0x3d /* Syscall issued. */
> +/* Definitions used for TEXASR Failure code (bits 0:7). If the failure
> + should be persistent, the abort code must be odd. 0xd0 through 0xff
> + are reserved for the kernel and potential hypervisor. */
> +#define _ABORT_PERSISTENT 0x01 /* An unspecified persistent abort. */
> +#define _ABORT_LOCK_BUSY 0x34 /* Busy lock, not persistent. */
> +#define _ABORT_NESTED_TRYLOCK (0x32 | _ABORT_PERSISTENT)
> +#define _ABORT_SYSCALL (0x30 | _ABORT_PERSISTENT)
>
> #endif
> --
> 2.4.3
>