Re: [PATCH 11/14] Add elision test cases

["Carlos O'Donell" <carlos at redhat dot com>]

On 06/28/2013 04:53 PM, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> These rely on the in program tuning interfaces

Thanks for the clarification, thought it seems this depends on slightly
more things than the new API? You have some #ifdef'd code that may no
longer belong. See below.

> 2013-06-27  Andi Kleen  <ak@linux.intel.com>
> 
> 	* Makefile (tst-elision1, tst-elision2): Add.
> 	* tst-elision1.c: New file to test elision.
> 	* tst-elision2.c: Likewise.
> 	* tst-elision-common.c: Likewise.
> ---
>  nptl/Makefile             |   3 +-
>  nptl/tst-elision-common.c | 265 ++++++++++++++++++++++++++++++++++++++++++++++
>  nptl/tst-elision1.c       | 115 ++++++++++++++++++++
>  nptl/tst-elision2.c       |  84 +++++++++++++++
>  4 files changed, 466 insertions(+), 1 deletion(-)
>  create mode 100644 nptl/tst-elision-common.c
>  create mode 100644 nptl/tst-elision1.c
>  create mode 100644 nptl/tst-elision2.c
> 
> diff --git a/nptl/Makefile b/nptl/Makefile
> index d076b68..35199fa 100644
> --- a/nptl/Makefile
> +++ b/nptl/Makefile
> @@ -269,7 +269,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-elision2
>  xtests = tst-setuid1 tst-setuid1-static tst-mutexpp1 tst-mutexpp6 tst-mutexpp10
>  test-srcs = tst-oddstacklimit
>  

OK.

> diff --git a/nptl/tst-elision-common.c b/nptl/tst-elision-common.c
> new file mode 100644
> index 0000000..8d0212d
> --- /dev/null
> +++ b/nptl/tst-elision-common.c
> @@ -0,0 +1,265 @@
> +/* 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

Why 10 and 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);

If the API bits are a prerequisite then you don't need the ifdef?

> +#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 (void)
> +{
> +#ifdef ENABLE_LOCK_ELISION
> +  return 1;
> +#else
> +  return 0;
> +#endif
> +}
> +
> +int
> +mutex_test (void)
> +{
> +  int ret = 0;
> +  int default_iter = default_elision_enabled () ? ITER : 0;
> +
> +  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

Where is the definition of this? ADAPTIVE can't support elision
becuase it's NORMAL-like in its requirement for relock after
deadlock.

Dead code?

> +  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

No need to wrap in #ifdef?

> +  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 () ? 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

We don't define this initializer. Please remove the test or predicate
this testing on more than just the patches that add the new API for
elision control.

> +  // 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;
> +}
> diff --git a/nptl/tst-elision1.c b/nptl/tst-elision1.c
> new file mode 100644
> index 0000000..ba71446
> --- /dev/null
> +++ b/nptl/tst-elision1.c
> @@ -0,0 +1,115 @@
> +/* 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>
> +# include <config.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 txn:%d\n", name, lock, txn);
> +	  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;
> +    }
> +
> +#ifdef ENABLE_LOCK_ELISION
> +  forced = 1;
> +#else
> +  disabled = 1;
> +#endif
> +
> +  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"
> 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.

Cheers,
Carlos.