This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 02/14] Add new internal mutex type flags for elision.
- 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>
- Date: Fri, 28 Jun 2013 02:28:34 -0400
- Subject: Re: [PATCH 02/14] Add new internal mutex type flags for elision.
- References: <1372398717-16530-1-git-send-email-andi at firstfloor dot org> <1372398717-16530-3-git-send-email-andi at firstfloor dot org>
On 06/28/2013 01:51 AM, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
>
> Add Enable/disable flags used internally
>
> Extend the mutex initializers to have the fields needed for
> elision. The layout stays the same, and this is not visible
> to programs.
>
> These changes are not exposed outside pthread
Thank you, this is exactly what we need for a first pass.
> 2013-06-18 Andi Kleen <ak@linux.intel.com>
>
> * pthreadP.h: Add elision types.
> (PTHREAD_MUTEX_TYPE_ELISION): Add.
> * sysdeps/pthread/pthread.h: Add elision initializers.
> (PTHREAD_MUTEX_ELISION_NP, PTHREAD_MUTEX_NO_ELISION_NP,
> PTHREAD_MUTEX_PSHARED_NP): Add new flags.
> (__PTHREAD_SPINS): Add.
This patch looks OK also. It doesn't add any ABI or API that is
exposed to the user. You are reserving two bits in the kind but
they are private and used internally to indicate if the mutex
should be elided and that's OK.
Please let me review 3, 4, and 5 before we look at committing these.
> ---
> nptl/pthreadP.h | 15 ++++++++++++++-
> nptl/sysdeps/pthread/pthread.h | 28 ++++++++++++++++++++--------
> 2 files changed, 34 insertions(+), 9 deletions(-)
>
> diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
> index 7883fdf..e6b80bf 100644
> --- a/nptl/pthreadP.h
> +++ b/nptl/pthreadP.h
> @@ -61,6 +61,10 @@
> enum
> {
> PTHREAD_MUTEX_KIND_MASK_NP = 3,
> +
> + PTHREAD_MUTEX_ELISION_NP = 256,
> + PTHREAD_MUTEX_NO_ELISION_NP = 512,
OK.
> +
> PTHREAD_MUTEX_ROBUST_NORMAL_NP = 16,
> PTHREAD_MUTEX_ROBUST_RECURSIVE_NP
> = PTHREAD_MUTEX_ROBUST_NORMAL_NP | PTHREAD_MUTEX_RECURSIVE_NP,
> @@ -93,12 +97,21 @@ enum
> PTHREAD_MUTEX_PP_ERRORCHECK_NP
> = PTHREAD_MUTEX_PRIO_PROTECT_NP | PTHREAD_MUTEX_ERRORCHECK_NP,
> PTHREAD_MUTEX_PP_ADAPTIVE_NP
> - = PTHREAD_MUTEX_PRIO_PROTECT_NP | PTHREAD_MUTEX_ADAPTIVE_NP
> + = PTHREAD_MUTEX_PRIO_PROTECT_NP | PTHREAD_MUTEX_ADAPTIVE_NP,
> + PTHREAD_MUTEX_ELISION_FLAGS_NP
> + = PTHREAD_MUTEX_ELISION_NP | PTHREAD_MUTEX_NO_ELISION_NP,
OK.
> +
> + PTHREAD_MUTEX_TIMED_ELISION_NP =
> + PTHREAD_MUTEX_TIMED_NP | PTHREAD_MUTEX_ELISION_NP,
> + PTHREAD_MUTEX_TIMED_NO_ELISION_NP =
> + PTHREAD_MUTEX_TIMED_NP | PTHREAD_MUTEX_NO_ELISION_NP,
OK.
> };
> #define PTHREAD_MUTEX_PSHARED_BIT 128
>
> #define PTHREAD_MUTEX_TYPE(m) \
> ((m)->__data.__kind & 127)
> +#define PTHREAD_MUTEX_TYPE_ELISION(m) \
> + ((m)->__data.__kind & (127|PTHREAD_MUTEX_ELISION_FLAGS_NP))
OK.
>
> #if LLL_PRIVATE == 0 && LLL_SHARED == 128
> # define PTHREAD_MUTEX_PSHARED(m) \
> diff --git a/nptl/sysdeps/pthread/pthread.h b/nptl/sysdeps/pthread/pthread.h
> index ded5ae5..61d5346 100644
> --- a/nptl/sysdeps/pthread/pthread.h
> +++ b/nptl/sysdeps/pthread/pthread.h
> @@ -83,27 +83,39 @@ enum
>
>
> /* Mutex initializers. */
> +#if __PTHREAD_MUTEX_HAVE_ELISION == 1 /* 64bit layout. */
> +#define __PTHREAD_SPINS 0, 0
> +#elif __PTHREAD_MUTEX_HAVE_ELISION == 2 /* 32bit layout. */
> +#define __PTHREAD_SPINS { 0, 0 }
> +#else
> +#define __PTHREAD_SPINS 0
> +#endif
OK.
> +
> #ifdef __PTHREAD_MUTEX_HAVE_PREV
> # define PTHREAD_MUTEX_INITIALIZER \
> - { { 0, 0, 0, 0, 0, 0, { 0, 0 } } }
> + { { 0, 0, 0, 0, 0, __PTHREAD_SPINS, { 0, 0 } } }
> # ifdef __USE_GNU
> # define PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP \
> - { { 0, 0, 0, 0, PTHREAD_MUTEX_RECURSIVE_NP, 0, { 0, 0 } } }
> + { { 0, 0, 0, 0, PTHREAD_MUTEX_RECURSIVE_NP, __PTHREAD_SPINS, { 0, 0 } } }
> # define PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP \
> - { { 0, 0, 0, 0, PTHREAD_MUTEX_ERRORCHECK_NP, 0, { 0, 0 } } }
> + { { 0, 0, 0, 0, PTHREAD_MUTEX_ERRORCHECK_NP, __PTHREAD_SPINS, { 0, 0 } } }
> # define PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP \
> - { { 0, 0, 0, 0, PTHREAD_MUTEX_ADAPTIVE_NP, 0, { 0, 0 } } }
> + { { 0, 0, 0, 0, PTHREAD_MUTEX_ADAPTIVE_NP, __PTHREAD_SPINS, { 0, 0 } } }
> +# define PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP \
> + { { 0, 0, 0, 0, PTHREAD_MUTEX_ADAPTIVE_NP, __PTHREAD_SPINS, { 0, 0 } } }
> +
> # endif
> #else
> # define PTHREAD_MUTEX_INITIALIZER \
> - { { 0, 0, 0, 0, 0, { 0 } } }
> + { { 0, 0, 0, 0, 0, { __PTHREAD_SPINS } } }
> # ifdef __USE_GNU
> # define PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP \
> - { { 0, 0, 0, PTHREAD_MUTEX_RECURSIVE_NP, 0, { 0 } } }
> + { { 0, 0, 0, PTHREAD_MUTEX_RECURSIVE_NP, 0, { __PTHREAD_SPINS } } }
> # define PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP \
> - { { 0, 0, 0, PTHREAD_MUTEX_ERRORCHECK_NP, 0, { 0 } } }
> + { { 0, 0, 0, PTHREAD_MUTEX_ERRORCHECK_NP, 0, { __PTHREAD_SPINS } } }
> # define PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP \
> - { { 0, 0, 0, PTHREAD_MUTEX_ADAPTIVE_NP, 0, { 0 } } }
> + { { 0, 0, 0, PTHREAD_MUTEX_ADAPTIVE_NP, 0, { __PTHREAD_SPINS } } }
> +
OK, since this doesn't change the value of the types exposed to the user.
> # endif
> #endif
>
>
Cheers,
Carlos.