This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 09/14] Add a new pthread_mutexattr_setelision_np interface.
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: Andi Kleen <andi at firstfloor dot org>
- Cc: libc-alpha at sourceware dot org, Andi Kleen <ak at linux dot jf dot intel dot com>, Roland McGrath <roland at hack dot frob dot com>
- Date: Mon, 01 Jul 2013 19:11:14 -0400
- Subject: Re: [PATCH 09/14] Add a new pthread_mutexattr_setelision_np interface.
- References: <1372452807-25216-1-git-send-email-andi at firstfloor dot org> <1372452807-25216-10-git-send-email-andi at firstfloor dot org> <51D09CA6 dot 7060700 at redhat dot com> <20130701214443 dot GT6123 at two dot firstfloor dot org>
On 07/01/2013 05:44 PM, Andi Kleen wrote:
> [Address feedback]
>
> Add a new pthread_mutexattr_setelision_np() export function
> that allows enabling or disabling elision for a mutex.
>
> This now implies no behaviour changes that are not undefined or
> are "may" in POSIX to my knowledge. The previous trylock
> changes have been eliminated.
>From my perspective I think that this patch is a useful API.
You've corrected all of my complaints and I'm happy with it.
In my opinion we will never get a lock hinting API that works
100% of the time and users will want access to disable elision
(talking about the abstract model now) for a lock. We will also
never get an elision algorithm that works 100% of the time, and
again users will want to disable elision. That doesn't mean we
don't want this feature, since it can be a very useful performance
improvement.
While I agree with Roland's opinion that an API should only deal
with those things that are articulable, there is a part of me that
considers such perfect APIs to be anathema to the practical realities
of the systems we are designing.
Unfortunately Roland's objection, and my affirmation mean that
the community as a whole needs to talk this out more.
Andi, You need to lead that discussion, but it's going to be
in the 2.19 timeframe. We can't keep delaying this release.
I have done what I could in good faith to try push this issue
forward. I have unfortunately failed in this regard. I'll post
a followup email to talk about what you can commit if you wish
to commit something for 2.18, which I suggest you do.
> It is purely a tuning hint.
>
> nptl/:
> 2013-06-27 Andi Kleen <ak@linux.intel.com>
>
> * Makefile: Add pthread_mutexattr_setelision_np.
> * Versions: Add pthread_mutexattr_setelision_np for 2.18.
> * pthread_mutexattr_setelision_np.c (pthread_mutexattr_setelision_np):
> New file to set elision attributes for a mutex attr.
> * sysdeps/unix/sysv/linux/x86/pthread_mutexattr_setelision_np.c: New file.
> * nptl/sysdeps/pthread/pthread.h (pthread_mutexattr_settype_np):
> New prototype.
>
> /:
> 2013-06-27 Andi Kleen <ak@linux.intel.com>
>
> * sysdeps/unix/sysv/linux/i386/nptl/libpthread.abilist: Add
> pthread_mutexattr_setelision_np and related functions.
> * sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/nptl/libpthread.abilist: dito.
> * sysdeps/unix/sysv/linux/powerpc/powerpc64/nptl/libpthread.abilist: dito.
> * sysdeps/unix/sysv/linux/s390/s390-32/nptl/libpthread.abilist: dito.
> * sysdeps/unix/sysv/linux/s390/s390-64/nptl/libpthread.abilist: dito.
> * sysdeps/unix/sysv/linux/sh/nptl/libpthread.abilist: dito.
> * sysdeps/unix/sysv/linux/sparc/sparc32/nptl/libpthread.abilist: dito.
> * sysdeps/unix/sysv/linux/sparc/sparc64/nptl/libpthread.abilist: dito.
> * sysdeps/unix/sysv/linux/x86_64/64/nptl/libpthread.abilist: dito.
> * sysdeps/unix/sysv/linux/x86_64/x32/nptl/libpthread.abilist: dito.
> ---
> nptl/Makefile | 3 +-
> nptl/Versions | 1 +
> nptl/pthread_mutexattr_setelision_np.c | 41 ++++++++++++++++++++
> nptl/sysdeps/pthread/pthread.h | 16 ++++++++
> .../linux/x86/pthread_mutexattr_setelision_np.c | 20 ++++++++++
> .../unix/sysv/linux/i386/nptl/libpthread.abilist | 1 +
> .../powerpc/powerpc32/fpu/nptl/libpthread.abilist | 1 +
> .../powerpc/powerpc64/nptl/libpthread.abilist | 1 +
> .../linux/s390/s390-32/nptl/libpthread.abilist | 1 +
> .../linux/s390/s390-64/nptl/libpthread.abilist | 1 +
> sysdeps/unix/sysv/linux/sh/nptl/libpthread.abilist | 1 +
> .../linux/sparc/sparc32/nptl/libpthread.abilist | 1 +
> .../linux/sparc/sparc64/nptl/libpthread.abilist | 1 +
> .../sysv/linux/x86_64/64/nptl/libpthread.abilist | 1 +
> .../sysv/linux/x86_64/x32/nptl/libpthread.abilist | 1 +
> 15 files changed, 90 insertions(+), 1 deletions(-)
> create mode 100644 nptl/pthread_mutexattr_setelision_np.c
> create mode 100644 nptl/sysdeps/unix/sysv/linux/x86/pthread_mutexattr_setelision_np.c
>
> diff --git a/nptl/Makefile b/nptl/Makefile
> index cd601e5..883c9a8 100644
> --- a/nptl/Makefile
> +++ b/nptl/Makefile
> @@ -126,7 +126,8 @@ libpthread-routines = nptl-init vars events version \
> pthread_mutex_getprioceiling \
> pthread_mutex_setprioceiling \
> pthread_setname pthread_getname \
> - pthread_setattr_default_np pthread_getattr_default_np
> + pthread_setattr_default_np pthread_getattr_default_np \
> + pthread_mutexattr_setelision_np
> # pthread_setuid pthread_seteuid pthread_setreuid \
> # pthread_setresuid \
> # pthread_setgid pthread_setegid pthread_setregid \
> diff --git a/nptl/Versions b/nptl/Versions
> index 7a3da25..1ca3943 100644
> --- a/nptl/Versions
> +++ b/nptl/Versions
> @@ -259,6 +259,7 @@ libpthread {
> pthread_mutexattr_settype;
> pthread_mutexattr_setkind_np;
> __pthread_mutexattr_settype;
> + pthread_mutexattr_setelision_np;
> };
>
> GLIBC_PRIVATE {
> diff --git a/nptl/pthread_mutexattr_setelision_np.c b/nptl/pthread_mutexattr_setelision_np.c
> new file mode 100644
> index 0000000..1bcc939
> --- /dev/null
> +++ b/nptl/pthread_mutexattr_setelision_np.c
> @@ -0,0 +1,41 @@
> +/* Copyright (C) 2013 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <pthreadP.h>
> +
> +#ifndef ENABLE_ELISION
> +# define ENABLE_ELISION 0
> +#endif
> +
> +int
> +pthread_mutexattr_setelision_np (pthread_mutexattr_t *attr, int flag)
> +{
> + struct pthread_mutexattr *iattr = (struct pthread_mutexattr *) attr;
> +
> + iattr->mutexkind &= ~PTHREAD_MUTEX_ELISION_FLAGS_NP;
> + if (ENABLE_ELISION)
> + {
> + if (flag == PTHREAD_MUTEX_ELISION_ENABLE_NP)
> + iattr->mutexkind |= PTHREAD_MUTEX_ELISION_NP;
> + else if (flag == PTHREAD_MUTEX_ELISION_DISABLE_NP)
> + iattr->mutexkind |= PTHREAD_MUTEX_NO_ELISION_NP;
> + else
> + return EINVAL;
> + }
> + return 0;
> +}
> +
> diff --git a/nptl/sysdeps/pthread/pthread.h b/nptl/sysdeps/pthread/pthread.h
> index df4b725..0687aa0 100644
> --- a/nptl/sysdeps/pthread/pthread.h
> +++ b/nptl/sysdeps/pthread/pthread.h
> @@ -853,6 +853,22 @@ extern int pthread_mutexattr_settype (pthread_mutexattr_t *__attr, int __kind)
> __THROW __nonnull ((1));
> #endif
>
> +#if defined __USE_GNU
> +
> +/* Elision tuning hints. */
> +enum
> +{
> + PTHREAD_MUTEX_ELISION_DISABLE_NP,
> + PTHREAD_MUTEX_ELISION_ENABLE_NP
> +};
> +
> +/* Set elision tuning hints for *ATTR. When FLAG is PTHREAD_MUTEX_ELISION_ENABLE_NP
> + enable elision for the mutex. When FLAG is PTHREAD_MUTEX_ELISION_DISABLE_NP,
> + disable elision for the mutex. This is purely a tuning hint. */
> +extern int pthread_mutexattr_setelision_np (pthread_mutexattr_t *__attr,
> + int __flag);
> +#endif
> +
> /* Return in *PROTOCOL the mutex protocol attribute in *ATTR. */
> extern int pthread_mutexattr_getprotocol (const pthread_mutexattr_t *
> __restrict __attr,
> diff --git a/nptl/sysdeps/unix/sysv/linux/x86/pthread_mutexattr_setelision_np.c b/nptl/sysdeps/unix/sysv/linux/x86/pthread_mutexattr_setelision_np.c
> new file mode 100644
> index 0000000..9c453fc
> --- /dev/null
> +++ b/nptl/sysdeps/unix/sysv/linux/x86/pthread_mutexattr_setelision_np.c
> @@ -0,0 +1,20 @@
> +/* Copyright (C) 2013 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <elision-conf.h>
> +#define ENABLE_ELISION (__elision_available != 0)
> +#include "nptl/pthread_mutexattr_setelision_np.c"
> diff --git a/sysdeps/unix/sysv/linux/i386/nptl/libpthread.abilist b/sysdeps/unix/sysv/linux/i386/nptl/libpthread.abilist
> index 3437fde..56b3c98 100644
> --- a/sysdeps/unix/sysv/linux/i386/nptl/libpthread.abilist
> +++ b/sysdeps/unix/sysv/linux/i386/nptl/libpthread.abilist
> @@ -178,6 +178,7 @@ GLIBC_2.18
> GLIBC_2.18 A
> __pthread_mutexattr_settype F
> pthread_getattr_default_np F
> + pthread_mutexattr_setelision_np F
> pthread_mutexattr_setkind_np F
> pthread_mutexattr_settype F
> pthread_setattr_default_np F
> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/nptl/libpthread.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/nptl/libpthread.abilist
> index 6927ce1..347bb94 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/nptl/libpthread.abilist
> +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/nptl/libpthread.abilist
> @@ -178,6 +178,7 @@ GLIBC_2.18
> GLIBC_2.18 A
> __pthread_mutexattr_settype F
> pthread_getattr_default_np F
> + pthread_mutexattr_setelision_np F
> pthread_mutexattr_setkind_np F
> pthread_mutexattr_settype F
> pthread_setattr_default_np F
> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/nptl/libpthread.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/nptl/libpthread.abilist
> index 1821f67..3631e20 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/nptl/libpthread.abilist
> +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/nptl/libpthread.abilist
> @@ -12,6 +12,7 @@ GLIBC_2.18
> GLIBC_2.18 A
> __pthread_mutexattr_settype F
> pthread_getattr_default_np F
> + pthread_mutexattr_setelision_np F
> pthread_mutexattr_setkind_np F
> pthread_mutexattr_settype F
> pthread_setattr_default_np F
> diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/nptl/libpthread.abilist b/sysdeps/unix/sysv/linux/s390/s390-32/nptl/libpthread.abilist
> index 3437fde..56b3c98 100644
> --- a/sysdeps/unix/sysv/linux/s390/s390-32/nptl/libpthread.abilist
> +++ b/sysdeps/unix/sysv/linux/s390/s390-32/nptl/libpthread.abilist
> @@ -178,6 +178,7 @@ GLIBC_2.18
> GLIBC_2.18 A
> __pthread_mutexattr_settype F
> pthread_getattr_default_np F
> + pthread_mutexattr_setelision_np F
> pthread_mutexattr_setkind_np F
> pthread_mutexattr_settype F
> pthread_setattr_default_np F
> diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/nptl/libpthread.abilist b/sysdeps/unix/sysv/linux/s390/s390-64/nptl/libpthread.abilist
> index f69b3c8..7558db8 100644
> --- a/sysdeps/unix/sysv/linux/s390/s390-64/nptl/libpthread.abilist
> +++ b/sysdeps/unix/sysv/linux/s390/s390-64/nptl/libpthread.abilist
> @@ -12,6 +12,7 @@ GLIBC_2.18
> GLIBC_2.18 A
> __pthread_mutexattr_settype F
> pthread_getattr_default_np F
> + pthread_mutexattr_setelision_np F
> pthread_mutexattr_setkind_np F
> pthread_mutexattr_settype F
> pthread_setattr_default_np F
> diff --git a/sysdeps/unix/sysv/linux/sh/nptl/libpthread.abilist b/sysdeps/unix/sysv/linux/sh/nptl/libpthread.abilist
> index f69b3c8..7558db8 100644
> --- a/sysdeps/unix/sysv/linux/sh/nptl/libpthread.abilist
> +++ b/sysdeps/unix/sysv/linux/sh/nptl/libpthread.abilist
> @@ -12,6 +12,7 @@ GLIBC_2.18
> GLIBC_2.18 A
> __pthread_mutexattr_settype F
> pthread_getattr_default_np F
> + pthread_mutexattr_setelision_np F
> pthread_mutexattr_setkind_np F
> pthread_mutexattr_settype F
> pthread_setattr_default_np F
> diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/nptl/libpthread.abilist b/sysdeps/unix/sysv/linux/sparc/sparc32/nptl/libpthread.abilist
> index 809c823..ff636f8 100644
> --- a/sysdeps/unix/sysv/linux/sparc/sparc32/nptl/libpthread.abilist
> +++ b/sysdeps/unix/sysv/linux/sparc/sparc32/nptl/libpthread.abilist
> @@ -178,6 +178,7 @@ GLIBC_2.18
> GLIBC_2.18 A
> __pthread_mutexattr_settype F
> pthread_getattr_default_np F
> + pthread_mutexattr_setelision_np F
> pthread_mutexattr_setkind_np F
> pthread_mutexattr_settype F
> pthread_setattr_default_np F
> diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/nptl/libpthread.abilist b/sysdeps/unix/sysv/linux/sparc/sparc64/nptl/libpthread.abilist
> index 29de71d..4a963be 100644
> --- a/sysdeps/unix/sysv/linux/sparc/sparc64/nptl/libpthread.abilist
> +++ b/sysdeps/unix/sysv/linux/sparc/sparc64/nptl/libpthread.abilist
> @@ -13,6 +13,7 @@ GLIBC_2.18
> pthread_getattr_default_np F
> pthread_setattr_default_np F
> __pthread_mutexattr_settype F
> + pthread_mutexattr_setelision_np F
> pthread_mutexattr_setkind_np F
> pthread_mutexattr_settype F
> GLIBC_2.2
> diff --git a/sysdeps/unix/sysv/linux/x86_64/64/nptl/libpthread.abilist b/sysdeps/unix/sysv/linux/x86_64/64/nptl/libpthread.abilist
> index 5efbc4b..b2c5f9e 100644
> --- a/sysdeps/unix/sysv/linux/x86_64/64/nptl/libpthread.abilist
> +++ b/sysdeps/unix/sysv/linux/x86_64/64/nptl/libpthread.abilist
> @@ -12,6 +12,7 @@ GLIBC_2.18
> GLIBC_2.18 A
> __pthread_mutexattr_settype F
> pthread_getattr_default_np F
> + pthread_mutexattr_setelision_np F
> pthread_mutexattr_setkind_np F
> pthread_mutexattr_settype F
> pthread_setattr_default_np F
> diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/nptl/libpthread.abilist b/sysdeps/unix/sysv/linux/x86_64/x32/nptl/libpthread.abilist
> index 8f5f841..eb6cd21 100644
> --- a/sysdeps/unix/sysv/linux/x86_64/x32/nptl/libpthread.abilist
> +++ b/sysdeps/unix/sysv/linux/x86_64/x32/nptl/libpthread.abilist
> @@ -226,6 +226,7 @@ GLIBC_2.18
> GLIBC_2.18 A
> __pthread_mutexattr_settype F
> pthread_getattr_default_np F
> + pthread_mutexattr_setelision_np F
> pthread_mutexattr_setkind_np F
> pthread_mutexattr_settype F
> pthread_setattr_default_np F
>