This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 06/14] Extend the test suite for TSX and add some new tests to test 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 03:57:39 -0400
- Subject: Re: [PATCH 06/14] Extend the test suite for TSX and add some new tests to test elision
- References: <1372398717-16530-1-git-send-email-andi at firstfloor dot org> <1372398717-16530-7-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>
>
> - Change some existing tests that expect (undefined) behaviour not
> guaranteed by elision.
> - I added some ifdefs to these tests and add new variants that
> set the defines to test elision behaviour.
> - Add new tests that explicitely check for elision.
> Note strictly this violates the TSX spec which never guarantees any transaction
> ever succeeding. However we assume simple transactions will succeed and will
> fail if not.
> - Add tests covering the new lock kinds.
>
This patch depends on pthread_mutexattr_setelision_np from patch #11.
Can we avoid this and thus get the testsuite changes into glibc before
we discuss the new interface?
There are a couple of nits that prevent us from getting this patch into
glibc as-is, I'm sure you can clean those up quickly though.
Please repost a v2 of this patch without reposting the entire patchset,
that way we can drill down until this patch is ready.
> 2013-06-18 Andi Kleen <ak@linux.intel.com>
>
> * Makefile: Add elision tests. Disable elision for some existing
> tests. Add comments describing this.
> * tst-abstime.c: Fix for elision.
> * tst-elision-common.c: dito.
> * tst-elision1.c: dito.
> * tst-elision1b.c: dito.
> * tst-elision2.c: dito.
> * tst-mutex5.c: Handle lock elision.
> * tst-mutex5b.c: New file.
> * tst-mutex5c.c: dito.
> * tst-mutex5d.c: dito
> * tst-mutex5e.c: dito.
> * tst-mutex7b.c: dito.
> * tst-mutex7c.c: dito.
> * tst-mutex7d.c: dito.
> * tst-mutex7e.c: dito.
> * tst-mutex8.c: Handle lock elision.
> * tst-mutex8b.c: New file.
> * tst-mutex8c.c: dito.
> * tst-mutex8d.c: dito.
> * tst-mutex8e.c: dito.
> * tst-mutex8f.c: dito.
> ---
> nptl/Makefile | 8 +-
> nptl/tst-abstime.c | 8 +-
> nptl/tst-elision-common.c | 271 +++++++++++++++++++++++++++++++++++++++++
> nptl/tst-elision1.c | 123 +++++++++++++++++++
> nptl/tst-elision1b.c | 1 +
> nptl/tst-elision2.c | 84 +++++++++++++
> nptl/tst-initializers2-c89.c | 1 +
> nptl/tst-initializers2-c99.c | 1 +
> nptl/tst-initializers2-gnu89.c | 1 +
> nptl/tst-initializers2-gnu99.c | 1 +
> nptl/tst-mutex5.c | 13 +-
> nptl/tst-mutex5b.c | 6 +
> nptl/tst-mutex5c.c | 2 +
> nptl/tst-mutex7.c | 10 +-
> nptl/tst-mutex7b.c | 3 +
> nptl/tst-mutex7c.c | 2 +
> nptl/tst-mutex8.c | 49 +++++++-
> nptl/tst-mutex8a.c | 1 +
> nptl/tst-mutex8b.c | 7 ++
> nptl/tst-mutex8d.c | 4 +
> nptl/tst-mutex8f.c | 3 +
> 21 files changed, 589 insertions(+), 10 deletions(-)
> create mode 100644 nptl/tst-elision-common.c
> create mode 100644 nptl/tst-elision1.c
> create mode 100644 nptl/tst-elision1b.c
> create mode 100644 nptl/tst-elision2.c
> create mode 100644 nptl/tst-initializers2-c89.c
> create mode 100644 nptl/tst-initializers2-c99.c
> create mode 100644 nptl/tst-initializers2-gnu89.c
> create mode 100644 nptl/tst-initializers2-gnu99.c
> create mode 100644 nptl/tst-mutex5b.c
> create mode 100644 nptl/tst-mutex5c.c
> create mode 100644 nptl/tst-mutex7b.c
> create mode 100644 nptl/tst-mutex7c.c
> create mode 100644 nptl/tst-mutex8a.c
> create mode 100644 nptl/tst-mutex8b.c
> create mode 100644 nptl/tst-mutex8d.c
> create mode 100644 nptl/tst-mutex8f.c
>
> diff --git a/nptl/Makefile b/nptl/Makefile
> index cd601e5..8513505 100644
> --- a/nptl/Makefile
> +++ b/nptl/Makefile
> @@ -204,7 +204,10 @@ CFLAGS-pt-system.c = -fexceptions
> tests = tst-typesizes \
> tst-attr1 tst-attr2 tst-attr3 tst-default-attr \
> tst-mutex1 tst-mutex2 tst-mutex3 tst-mutex4 tst-mutex5 tst-mutex6 \
> - tst-mutex7 tst-mutex8 tst-mutex9 tst-mutex5a tst-mutex7a \
> + tst-mutex7 tst-mutex8 tst-mutex8a tst-mutex9 tst-mutex5a tst-mutex7a \
> + tst-mutex8b tst-mutex8d tst-mutex8f \
> + tst-mutex5b tst-mutex5c \
> + tst-mutex7b tst-mutex7c \
OK.
> tst-mutexpi1 tst-mutexpi2 tst-mutexpi3 tst-mutexpi4 tst-mutexpi5 \
> tst-mutexpi5a tst-mutexpi6 tst-mutexpi7 tst-mutexpi7a tst-mutexpi8 \
> tst-mutexpi9 \
> @@ -267,7 +270,8 @@ tests = tst-typesizes \
> tst-abstime \
> tst-vfork1 tst-vfork2 tst-vfork1x tst-vfork2x \
> tst-getpid1 tst-getpid2 tst-getpid3 \
> - tst-initializers1 $(patsubst %,tst-initializers1-%,c89 gnu89 c99 gnu99)
> + tst-initializers1 $(patsubst %,tst-initializers1-%,c89 gnu89 c99 gnu99) \
> + tst-elision1 tst-elision1b tst-elision2
OK.
> xtests = tst-setuid1 tst-setuid1-static tst-mutexpp1 tst-mutexpp6 tst-mutexpp10
> test-srcs = tst-oddstacklimit
>
> diff --git a/nptl/tst-abstime.c b/nptl/tst-abstime.c
> index 99fc7c1..3c91c66 100644
> --- a/nptl/tst-abstime.c
> +++ b/nptl/tst-abstime.c
> @@ -21,9 +21,13 @@
> #include <semaphore.h>
> #include <stdio.h>
>
> +#ifndef MUTEX_TYPE
> +#define MUTEX_TYPE PTHREAD_MUTEX_INITIALIZER
> +#endif
> +
OK.
> static pthread_cond_t c = PTHREAD_COND_INITIALIZER;
> -static pthread_mutex_t m1 = PTHREAD_MUTEX_INITIALIZER;
> -static pthread_mutex_t m2 = PTHREAD_MUTEX_INITIALIZER;
> +static pthread_mutex_t m1 = MUTEX_TYPE;
> +static pthread_mutex_t m2 = MUTEX_TYPE;
OK.
> static pthread_rwlock_t rw1 = PTHREAD_RWLOCK_INITIALIZER;
> static pthread_rwlock_t rw2 = PTHREAD_RWLOCK_INITIALIZER;
> static sem_t sem;
> diff --git a/nptl/tst-elision-common.c b/nptl/tst-elision-common.c
> new file mode 100644
> index 0000000..8e65f49
> --- /dev/null
> +++ b/nptl/tst-elision-common.c
> @@ -0,0 +1,271 @@
> +/* tst-elision-common: Elision test harness.
> + 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 <stdio.h>
> +#include <string.h>
> +#include <stdlib.h>
> +#include "config.h"
> +
> +#define CPUID_FEATURE_RTM (1U << 11)
> +
> +static int
> +cpu_has_rtm (void)
> +{
> + if (__get_cpuid_max (0, NULL) >= 7)
> + {
> + unsigned a, b, c, d;
> +
> + __cpuid_count (7, 0, a, b, c, d);
> + if (b & CPUID_FEATURE_RTM)
> + return 1;
> + }
> + return 0;
> +}
> +
> +#define ITER 10
> +#define MAXTRY 100
> +
> +pthread_mutex_t lock;
> +
> +#ifndef USE_TRYLOCK_ONLY
> +static int
> +pthread_mutex_timedlock_wrapper(pthread_mutex_t *l)
> +{
> + struct timespec wait = { 0, 0 };
> + return pthread_mutex_timedlock (l, &wait);
> +}
> +#endif
> +
> +/* Note this test program will fail when single stepped.
> + It also assumes that simple transactions always work. There is no
> + guarantee in the architecture that this is the case. We do some
> + retries to handle random abort cases like interrupts. But it's
> + not fully guaranteed. However when this fails it is somewhat worrying. */
> +
> +int
> +run_mutex (int expected, const char *name, int force)
> +{
> + int i;
> + int try = 0;
> + int txn __attribute__((unused));
> + int err;
> +
> +#ifndef USE_TRYLOCK_ONLY
> + TESTLOCK(lock, pthread_mutex_lock, pthread_mutex_unlock, force);
> + TESTLOCK(lock, pthread_mutex_timedlock_wrapper, pthread_mutex_unlock, force);
> + TESTLOCK(lock, pthread_mutex_trylock, pthread_mutex_unlock, force);
> +#else
> + TESTTRYLOCK(lock, pthread_mutex_lock, pthread_mutex_trylock, pthread_mutex_unlock, force);
> +#endif
> +
> + err = pthread_mutex_destroy (&lock);
> + if (err != 0)
> + {
> + printf ("destroy for %s failed: %d\n", name, err);
> + return 1;
> + }
> + return 0;
> +}
> +
> +static int
> +run_mutex_init (int iter, const char *name, int type, int has_type, int force,
> + int elision)
> +{
> + pthread_mutexattr_t attr;
> +
> + pthread_mutexattr_init (&attr);
> + if (type)
> + pthread_mutexattr_settype (&attr, type);
> +#ifdef __PTHREAD_SET_ELISION
> + if (elision >= 0)
> + pthread_mutexattr_setelision_np (&attr, elision);
> +#endif
> +
> + pthread_mutex_init (&lock, has_type ? &attr : NULL);
> + return run_mutex (iter, name, force);
> +}
> +
> +static int
> +default_elision_enabled (const char *name)
> +{
> + char *s = getenv (name);
> +
> + if (s != NULL && strncmp (s, "elision", 7) == 0)
> + return 1;
> + if (s != NULL && strcmp (s, "none") == 0)
> + return 0;
> +#ifdef ENABLE_LOCK_ELISION
> + return 1;
> +#else
> + return 0;
> +#endif
> +}
> +
> +int
> +mutex_test (void)
> +{
> + int ret = 0;
> + int default_iter = default_elision_enabled ("GLIBC_PTHREAD_MUTEX") ? ITER : 0;
Please simplify this.
If elision is configured on then we should test it, otherwise we shouldn't.
> +
> + lock = (pthread_mutex_t) PTHREAD_MUTEX_INITIALIZER;
> + ret += run_mutex (default_iter, "default initializer timed", 0);
> +
> +#ifdef ADAPTIVE_SUPPORTS_ELISION
> + lock = (pthread_mutex_t) PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP;
> + run_mutex (default_iter, "adaptive initializer default", 0);
> +#endif
> +
> + ret += run_mutex_init (default_iter, "timed init default", 0, 0, 0, -1);
> +#ifdef __PTHREAD_SET_ELISION
> + ret += run_mutex_init (ITER, "timed init elision",
> + PTHREAD_MUTEX_TIMED_NP, 1, 1, 1);
> + ret += run_mutex_init (ITER, "timed init no elision",
> + PTHREAD_MUTEX_TIMED_NP, 1, 2, 0);
> +#endif
> + ret += run_mutex_init (0, "normal no elision",
> + PTHREAD_MUTEX_NORMAL, 1, 2, -1);
> +
> +#ifdef ADAPTIVE_SUPPORTS_ELISION
> + ret += run_mutex_init (default_iter, "adaptive init default",
> + PTHREAD_MUTEX_ADAPTIVE_NP, 1, 0, -1);
> + ret += run_mutex_init (ITER, "adaptive init elision",
> + PTHREAD_MUTEX_ADAPTIVE_NP, 1, 1, 1);
> + ret += run_mutex_init (0, "adaptive init no elision",
> + PTHREAD_MUTEX_ADAPTIVE_NP,
> + 1, 2, 0);
> +#endif
> +
> + return ret;
> +}
> +
> +pthread_rwlock_t rwlock;
> +
> +#ifndef USE_TRYLOCK_ONLY
> +static int
> +pthread_rwlock_timedwrlock_wrapper(pthread_rwlock_t *l)
> +{
> + struct timespec wait = { 0, 0 };
> + return pthread_rwlock_timedwrlock (l, &wait);
> +}
> +
> +static int
> +pthread_rwlock_timedrdlock_wrapper(pthread_rwlock_t *l)
> +{
> + struct timespec wait = { 0, 0 };
> + return pthread_rwlock_timedrdlock (l, &wait);
> +}
> +#endif
> +
> +int
> +run_rwlock (int expected, const char *name, int force)
> +{
> + int i;
> + int try = 0;
> + int txn __attribute__((unused));
> + int err;
> +
> +#ifndef USE_TRYLOCK_ONLY
> + TESTLOCK(rwlock, pthread_rwlock_rdlock, pthread_rwlock_unlock, force);
> + TESTLOCK(rwlock, pthread_rwlock_wrlock, pthread_rwlock_unlock, force);
> + TESTLOCK(rwlock, pthread_rwlock_rdlock, pthread_rwlock_unlock, force);
> + TESTLOCK(rwlock, pthread_rwlock_tryrdlock, pthread_rwlock_unlock, force);
> + TESTLOCK(rwlock, pthread_rwlock_trywrlock, pthread_rwlock_unlock, force);
> + TESTLOCK(rwlock, pthread_rwlock_timedrdlock_wrapper,
> + pthread_rwlock_unlock, force);
> + TESTLOCK(rwlock, pthread_rwlock_timedwrlock_wrapper,
> + pthread_rwlock_unlock, force);
> +#else
> + TESTTRYLOCK(rwlock, pthread_rwlock_wrlock, pthread_rwlock_trywrlock,
> + pthread_rwlock_unlock, force);
> +#endif
> +
> + err = pthread_rwlock_destroy (&rwlock);
> + if (err != 0)
> + {
> + printf ("pthread_rwlock_destroy for %s failed: %d\n", name, err);
> + return 1;
> + }
> + return 0;
> +}
> +
> +int
> +run_rwlock_attr (int iter, const char *name, int type, int el, int force)
> +{
> + pthread_rwlockattr_t attr;
> + pthread_rwlockattr_init (&attr);
> + pthread_rwlockattr_setkind_np (&attr, type);
> +#ifdef __PTHREAD_RWLOCK_SET_ELISION
> + if (el >= 0)
> + pthread_rwlockattr_setelision_np (&attr, el);
> +#endif
> + pthread_rwlock_init (&rwlock, &attr);
> + return run_rwlock (iter, name, force);
> +}
> +
> +int
> +run_rwlock_attr_set (int iter, const char *extra, int flag, int force)
> +{
> + char str[100];
> + int ret = 0;
> +
> + snprintf(str, sizeof str, "rwlock attr prefer reader %s", extra);
> + ret += run_rwlock_attr (iter, str,
> + PTHREAD_RWLOCK_PREFER_READER_NP, flag, force);
> + snprintf(str, sizeof str, "rwlock attr prefer writer %s", extra);
> + ret += run_rwlock_attr (iter, str,
> + PTHREAD_RWLOCK_PREFER_WRITER_NP, flag, force);
> + snprintf(str, sizeof str, "rwlock attr prefer writer non recursive %s", extra);
> + ret += run_rwlock_attr (iter, str,
> + PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP, flag, force);
> + return ret;
> +}
> +
> +
> +int
> +rwlock_test (void)
> +{
> + int ret = 0;
> + int default_iter = default_elision_enabled ("GLIBC_PTHREAD_RWLOCK") ? ITER : 0;
> +
> + pthread_rwlock_init (&rwlock, NULL);
> + ret += run_rwlock (default_iter, "rwlock created", 0);
> +
> + rwlock = (pthread_rwlock_t)PTHREAD_RWLOCK_INITIALIZER;
> + ret += run_rwlock (default_iter, "rwlock initialized", 0);
> +
> + rwlock = (pthread_rwlock_t)PTHREAD_RWLOCK_WRITER_NONRECURSIVE_INITIALIZER_NP;
> + ret += run_rwlock (default_iter, "rwlock initialized writer non recursive", 0);
> +
> + rwlock = (pthread_rwlock_t)PTHREAD_RWLOCK_WRITER_NONRECURSIVE_INITIALIZER_NP;
> + ret += run_rwlock (default_iter, "rwlock initialized writer non recursive", 0);
> +
> +#ifdef PTHREAD_RWLOCK_WRITER_INITIALIZER_NP
> + // XXX includes are missing PTHREAD_RWLOCK_WRITER_INITIALIZER_NP
> + rwlock = (pthread_rwlock_t)PTHREAD_RWLOCK_WRITER_INITIALIZER_NP;
> + ret += run_rwlock (default_iter, "rwlock initialized writer", 0);
> +#endif
> +
> + ret += run_rwlock_attr_set (default_iter, "", 0, 0);
> +
> +#ifdef __PTHREAD_RWLOCK_SET_ELISION
> + ret += run_rwlock_attr_set (ITER, "eliding", 1, 1);
> + ret += run_rwlock_attr_set (0, "not eliding", 0, 2);
> +#endif
> +
> + return ret;
> +}
OK.
> diff --git a/nptl/tst-elision1.c b/nptl/tst-elision1.c
> new file mode 100644
> index 0000000..18fe348
> --- /dev/null
> +++ b/nptl/tst-elision1.c
> @@ -0,0 +1,123 @@
> +/* tst-elision1: Test basic elision success.
> + 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/>. */
> +
> +/* To use on other architectures you would need an own version
> + of cpu_has_rtm and _xtest. */
> +#if defined(__i386__) || defined(__x86_64__)
> +# include <elision-conf.h>
> +# include <pthread.h>
> +# include <stdlib.h>
> +# include <string.h>
> +# include <stdio.h>
> +# include <hle.h>
> +
> +int disabled;
> +int forced;
> +
> +int
> +check (const char *name, const char *lock, int try, int txn, int max,
> + int override)
> +{
> + int should_run = 1;
> +
> + if (override == 0)
> + should_run = disabled == 0;
> + else if (override == 1)
> + should_run = 1;
> + else if (override == 2)
> + should_run = 0;
> +
> + /* forced noop right now, so not tested. But test if the defaults change. */
> + if (!should_run)
> + {
> + if (txn != 0)
> + {
> + printf ("%s %s transaction run unexpected\n", name, lock);
> + return 1;
> + }
> + }
> + else
> + {
> + if (try == max)
> + {
> + printf ("%s %s no transactions when expected\n", name, lock);
> + return 1;
> + }
> + }
> + return 0;
> +}
> +
> +# define TESTLOCK(l, lock, unlock, force)\
> + do \
> + { \
> + txn = 0; \
> + for (i = 0; i < ITER; i++) \
> + { \
> + lock (&l); \
> + if (_xtest ()) \
> + txn++; \
> + unlock (&l); \
> + } \
> + } \
> + while (try++ < MAXTRY && txn != expected); \
> + if (check (name, #lock, try, txn, MAXTRY, force)) \
> + return 1;
> +
> +# include "tst-elision-common.c"
> +
> +int
> +do_test (void)
> +{
> + if (cpu_has_rtm () == 0)
> + {
> + printf ("elision test requires RTM capable CPU. not tested\n");
> + return 0;
> + }
> +
> + char *s = getenv ("GLIBC_PTHREAD_MUTEX");
> + if (s != NULL)
> + {
> + char *o = getenv ("GLIBC_PTHREAD_RWLOCK");
> + if (o == NULL || strcmp (o, s))
> + {
> + puts("GLIBC_PTHREAD_MUTEX and GLIBC_PTHREAD_RWLOCK must match for test!\n");
> + return 1;
> + }
> + if (strcmp (s, "none") == 0)
> + disabled = 1;
> + if (strcmp (s, "elision") == 0)
> + forced = 1;
> + }
> +
> + if (mutex_test ())
> + return 1;
> +
> + if (rwlock_test ())
> + return 1;
> +
> + return 0;
> +}
> +#else
> +int do_test (void)
> +{
> + return 0;
> +}
> +#endif
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
OK.
> diff --git a/nptl/tst-elision1b.c b/nptl/tst-elision1b.c
> new file mode 100644
> index 0000000..9f5ec3d
> --- /dev/null
> +++ b/nptl/tst-elision1b.c
> @@ -0,0 +1 @@
> +#include "tst-elision1.c"
> diff --git a/nptl/tst-elision2.c b/nptl/tst-elision2.c
> new file mode 100644
> index 0000000..3b6ccc7
> --- /dev/null
> +++ b/nptl/tst-elision2.c
> @@ -0,0 +1,84 @@
> +/* tst-elision2: Test elided trylock semantics
> + 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/>. */
> +
> +/* To use on other architectures you would need an own version of
> + cpu_has_rtm. */
> +#if defined(__i386__) || defined(__x86_64__)
> +# include <pthread.h>
> +# include <stdio.h>
> +# include <hle.h>
> +# include <cpuid.h>
> +
> +int check (int success, int force)
> +{
> + /* Any nested trylock disabled right now. */
> + return success == 0;
> +}
> +
> +# define TESTTRYLOCK(l, lock, trylock, unlock, force) \
> + do \
> + { \
> + txn = 0; \
> + for (i = 0; i < ITER; i++) \
> + { \
> + lock (&l); \
> + if (trylock (&l) == 0) \
> + { \
> + txn++; \
> + unlock (&l); \
> + } \
> + unlock (&l); \
> + } \
> + } \
> + while (try++ < MAXTRY && txn != ITER); \
> + if (!check (txn == ITER, force)) \
> + { \
> + printf ("%s %s nested trylock check failed txn:%d iter:%d force:%d\n", \
> + name, #lock, txn, ITER, force); \
> + return 1; \
> + }
> +
> +# define USE_TRYLOCK_ONLY 1
> +# include "tst-elision-common.c"
> +
> +int
> +do_test (void)
> +{
> + if (cpu_has_rtm () == 0)
> + {
> + printf ("elision test requires RTM capable CPU. not tested\n");
> + return 0;
> + }
> +
> + if (mutex_test ())
> + return 1;
> +
> + if (rwlock_test ())
> + return 1;
> +
> + return 0;
> +}
> +#else
> +int do_test (void)
> +{
> + return 0;
> +}
> +#endif
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
OK.
> diff --git a/nptl/tst-initializers2-c89.c b/nptl/tst-initializers2-c89.c
> new file mode 100644
> index 0000000..1fb8af6
> --- /dev/null
> +++ b/nptl/tst-initializers2-c89.c
> @@ -0,0 +1 @@
> +#include "tst-initializers2.c"
> diff --git a/nptl/tst-initializers2-c99.c b/nptl/tst-initializers2-c99.c
> new file mode 100644
> index 0000000..1fb8af6
> --- /dev/null
> +++ b/nptl/tst-initializers2-c99.c
> @@ -0,0 +1 @@
> +#include "tst-initializers2.c"
> diff --git a/nptl/tst-initializers2-gnu89.c b/nptl/tst-initializers2-gnu89.c
> new file mode 100644
> index 0000000..1fb8af6
> --- /dev/null
> +++ b/nptl/tst-initializers2-gnu89.c
> @@ -0,0 +1 @@
> +#include "tst-initializers2.c"
> diff --git a/nptl/tst-initializers2-gnu99.c b/nptl/tst-initializers2-gnu99.c
> new file mode 100644
> index 0000000..1fb8af6
> --- /dev/null
> +++ b/nptl/tst-initializers2-gnu99.c
> @@ -0,0 +1 @@
> +#include "tst-initializers2.c"
> diff --git a/nptl/tst-mutex5.c b/nptl/tst-mutex5.c
> index f19cd8c..9d6c96b 100644
> --- a/nptl/tst-mutex5.c
> +++ b/nptl/tst-mutex5.c
> @@ -45,12 +45,20 @@ do_test (void)
> return 1;
> }
>
> - if (pthread_mutexattr_settype (&a, TYPE) != 0)
> + if (TYPE && pthread_mutexattr_settype (&a, TYPE) != 0)
Why are you doing this? To avoid disabling elision for the type?
Please add a comment about this, and the fact taht you assume
DEFAULT and NORMAL have the same value.
> {
> puts ("mutexattr_settype failed");
> return 1;
> }
>
> +#ifdef USE_ELISION
> + if (pthread_mutexattr_setelision_np (&a, 1) != 0)
> + {
> + puts ("pthread_mutexattr_setelision_np failed");
> + return 1;
> + }
> +#endif
Please remove this test and put it into a followup patch
to the testsuite that is pat of the pthread_mutexattr_setelision_np
patch. That way we can get these changes in now.
> +
> #ifdef ENABLE_PI
> if (pthread_mutexattr_setprotocol (&a, PTHREAD_PRIO_INHERIT) != 0)
> {
> @@ -85,6 +93,8 @@ do_test (void)
> return 1;
> }
>
> + /* Elided locks do not time out. */
> +#ifndef USE_ELISION
> if (pthread_mutex_trylock (&m) == 0)
> {
> puts ("mutex_trylock succeeded");
> @@ -180,6 +190,7 @@ do_test (void)
> puts ("3rd timedlock didn't return right away");
> return 1;
> }
> +#endif
>
> if (pthread_mutex_unlock (&m) != 0)
> {
> diff --git a/nptl/tst-mutex5b.c b/nptl/tst-mutex5b.c
> new file mode 100644
> index 0000000..04eacc9
> --- /dev/null
> +++ b/nptl/tst-mutex5b.c
> @@ -0,0 +1,6 @@
> +#define TYPE PTHREAD_MUTEX_TIMED_NP
> +#include <elision-conf.h>
> +#ifdef HAVE_ELISION
> +#define USE_ELISION 1
> +#endif
> +#include "tst-mutex5.c"
OK.
> diff --git a/nptl/tst-mutex5c.c b/nptl/tst-mutex5c.c
> new file mode 100644
> index 0000000..d987ce3
> --- /dev/null
> +++ b/nptl/tst-mutex5c.c
> @@ -0,0 +1,2 @@
> +#define TYPE PTHREAD_MUTEX_NORMAL
> +#include "tst-mutex5.c"
> diff --git a/nptl/tst-mutex7.c b/nptl/tst-mutex7.c
> index b39a9d0..9a99796 100644
> --- a/nptl/tst-mutex7.c
> +++ b/nptl/tst-mutex7.c
> @@ -74,12 +74,20 @@ do_test (void)
> exit (1);
> }
>
> - if (pthread_mutexattr_settype (&a, TYPE) != 0)
> + if (TYPE && pthread_mutexattr_settype (&a, TYPE) != 0)
Same comment as above.
> {
> puts ("mutexattr_settype failed");
> exit (1);
> }
>
> +#if defined(ENABLE_ELISION) && defined(__PTHREAD_SET_ELISION)
> + if (pthread_mutexattr_setelision_np (&a, 1) != 0)
> + {
> + puts ("mutexattr_setelision_np failed");
> + return 1;
> + }
> +#endif
Likewise.
> +
> #ifdef ENABLE_PI
> if (pthread_mutexattr_setprotocol (&a, PTHREAD_PRIO_INHERIT) != 0)
> {
> diff --git a/nptl/tst-mutex7b.c b/nptl/tst-mutex7b.c
> new file mode 100644
> index 0000000..f76e48b
> --- /dev/null
> +++ b/nptl/tst-mutex7b.c
> @@ -0,0 +1,3 @@
> +#define TYPE PTHREAD_MUTEX_TIMED_NP
> +#define ENABLE_ELISION 1
> +#include "tst-mutex7.c"
OK.
> diff --git a/nptl/tst-mutex7c.c b/nptl/tst-mutex7c.c
> new file mode 100644
> index 0000000..2a6c63d
> --- /dev/null
> +++ b/nptl/tst-mutex7c.c
> @@ -0,0 +1,2 @@
> +#define TYPE PTHREAD_MUTEX_NORMAL
> +#include "tst-mutex7.c"
OK.
> diff --git a/nptl/tst-mutex8.c b/nptl/tst-mutex8.c
> index 2089c50..70b528c 100644
> --- a/nptl/tst-mutex8.c
> +++ b/nptl/tst-mutex8.c
> @@ -23,6 +23,9 @@
> #include <stdio.h>
> #include <stdlib.h>
>
> +#ifndef NAME
> +#define NAME "normal"
> +#endif
>
> static pthread_mutex_t *m;
> static pthread_barrier_t b;
> @@ -93,6 +96,8 @@ tf (void *arg)
> static int
> check_type (const char *mas, pthread_mutexattr_t *ma)
> {
> + int e __attribute__((unused));
> +
> if (pthread_mutex_init (m, ma) != 0)
> {
> printf ("1st mutex_init failed for %s\n", mas);
> @@ -117,7 +122,10 @@ check_type (const char *mas, pthread_mutexattr_t *ma)
> return 1;
> }
>
> - int e = pthread_mutex_destroy (m);
> + /* Elided mutexes don't fail destroy. If elision is not explicitly disabled
> + we don't know, so can also not check this. */
> +#if !defined(USE_ELISION) && defined(USE_NO_ELISION)
> + e = pthread_mutex_destroy (m);
> if (e == 0)
> {
> printf ("mutex_destroy of self-locked mutex succeeded for %s\n", mas);
> @@ -129,6 +137,7 @@ check_type (const char *mas, pthread_mutexattr_t *ma)
> mas);
> return 1;
> }
> +#endif
OK.
>
> if (pthread_mutex_unlock (m) != 0)
> {
> @@ -142,6 +151,8 @@ check_type (const char *mas, pthread_mutexattr_t *ma)
> return 1;
> }
>
> + /* Elided mutexes don't fail destroy. */
> +#if !defined(USE_ELISION) && defined(USE_NO_ELISION)
> e = pthread_mutex_destroy (m);
> if (e == 0)
> {
> @@ -155,6 +166,7 @@ mutex_destroy of self-trylocked mutex did not return EBUSY %s\n",
> mas);
> return 1;
> }
> +#endif
OK.
>
> if (pthread_mutex_unlock (m) != 0)
> {
> @@ -189,6 +201,8 @@ mutex_destroy of self-trylocked mutex did not return EBUSY %s\n",
> return 1;
> }
>
> + /* Elided mutexes don't fail destroy. */
> +#if !defined(USE_ELISION) && defined(USE_NO_ELISION)
> e = pthread_mutex_destroy (m);
> if (e == 0)
> {
> @@ -201,6 +215,7 @@ mutex_destroy of self-trylocked mutex did not return EBUSY %s\n",
> mutex_destroy of condvar-used mutex did not return EBUSY for %s\n", mas);
> return 1;
> }
> +#endif
>
> done = true;
> if (pthread_cond_signal (&c) != 0)
> @@ -259,6 +274,8 @@ mutex_destroy of condvar-used mutex did not return EBUSY for %s\n", mas);
> return 1;
> }
>
> + /* Elided mutexes don't fail destroy. */
> +#if !defined(USE_ELISION) && defined(USE_NO_ELISION)
> e = pthread_mutex_destroy (m);
> if (e == 0)
> {
> @@ -273,6 +290,7 @@ mutex_destroy of condvar-used mutex did not return EBUSY for %s\n", mas);
> mas);
> return 1;
> }
> +#endif
>
> if (pthread_cancel (th) != 0)
> {
> @@ -304,6 +322,7 @@ mutex_destroy of condvar-used mutex did not return EBUSY for %s\n", mas);
> static int
> do_test (void)
> {
> + pthread_mutexattr_t ma;
> pthread_mutex_t mm;
> m = &mm;
>
> @@ -319,10 +338,32 @@ do_test (void)
> return 1;
> }
>
> - puts ("check normal mutex");
> - int res = check_type ("normal", NULL);
> +#ifdef TYPE
> + if (pthread_mutexattr_init (&ma) != 0)
> + {
> + puts ("0th mutexattr_init failed");
> + return 1;
> + }
> + if (TYPE && pthread_mutexattr_settype (&ma, TYPE) != 0)
> + {
> + puts ("0th mutexattr_settype failed");
> + return 1;
> + }
> +
> +#if defined(USE_ELISION) && defined(__PTHREAD_SET_ELISION)
See:
http://sourceware.org/glibc/wiki/Style_and_Conventions#Nested_C_Preprocessor_Directives
> + if (pthread_mutexattr_setelision_np (&ma, 1))
> + {
> + puts ("0th mutexattr_setelision_np failed");
> + return 1;
Same comment about testing interfaces we haven't reviewed yet.
Split it out and put it into the patch for the interface addition.
> + }
> +#endif
> + puts ("check " NAME " mutex");
> + int res = check_type (NAME, &ma);
> +#else
> + puts ("check " NAME " mutex");
> + int res = check_type (NAME, NULL);
> +#endif
>
> - pthread_mutexattr_t ma;
> if (pthread_mutexattr_init (&ma) != 0)
> {
> puts ("1st mutexattr_init failed");
> diff --git a/nptl/tst-mutex8a.c b/nptl/tst-mutex8a.c
> new file mode 100644
> index 0000000..d69ed49
> --- /dev/null
> +++ b/nptl/tst-mutex8a.c
> @@ -0,0 +1 @@
> +#include "tst-mutex8.c"
> diff --git a/nptl/tst-mutex8b.c b/nptl/tst-mutex8b.c
> new file mode 100644
> index 0000000..cba806b
> --- /dev/null
> +++ b/nptl/tst-mutex8b.c
> @@ -0,0 +1,7 @@
> +#define NAME "timed elided"
> +#include <elision-conf.h>
> +#ifdef HAVE_ELISION
> +#define USE_ELISION 1
> +#endif
> +#define TYPE PTHREAD_MUTEX_TIMED_NP
> +#include "tst-mutex8.c"
> diff --git a/nptl/tst-mutex8d.c b/nptl/tst-mutex8d.c
> new file mode 100644
> index 0000000..16bae30
> --- /dev/null
> +++ b/nptl/tst-mutex8d.c
> @@ -0,0 +1,4 @@
> +#define NAME "normal not elided"
> +#define USE_NO_ELISION 1
> +#define TYPE PTHREAD_MUTEX_NORMAL
> +#include "tst-mutex8.c"
> diff --git a/nptl/tst-mutex8f.c b/nptl/tst-mutex8f.c
> new file mode 100644
> index 0000000..9d203d2
> --- /dev/null
> +++ b/nptl/tst-mutex8f.c
> @@ -0,0 +1,3 @@
> +#define NAME "adaptive"
> +#define TYPE PTHREAD_MUTEX_ADAPTIVE_NP
> +#include "tst-mutex8.c"
>
Cheers,
Carlos.