This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Add runtime check if mutex will be elided in tst-mutex8 testcases.


On 02/05/2018 10:41 PM, Stefan Liebler wrote:
> Hi,
> 
> An elided mutex don't fail destroy. Elision was disabled for the
> test nptl/tst-mutex8 in nptl/Makefile. Thus we can run tests which
> destroy a locked mutex.
> 
> As elision is only disabled for tst-mutex8, the variants
> tst-mutex8-static, tst-mutexpi8 and tst-mutexpi8-static are still
> failing if lock elision is enabled.
> 
> This patch adds a runtime check, if the checked type of mutex will
> be elided. This check is using TUNABLE_GET_FULL to determine if
> elision is enabled via the tunables framework.
> The pthread_mutex_destroy tests are only run if we dont't assume an
> elided mutex.
> 
> This way, we can run the whole glibc testsuite with or without enabled
> lock elision.
> 
> Okay to commit?

LGTM.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> Bye
> Stefan
> 
> ChangeLog:
> 
>     * nptl/Makefile (tst-mutex8-ENV): Delete.
>     * nptl/tst-mutex8.c (check_type):
>     Add runtime check if mutex will be elided.
> 
> 20180205_tstmutex8_elision.patch
> 
> 
> commit eb4c91a86de283ff314c068d9d89368b229b9a33
> Author: Stefan Liebler <stli@linux.vnet.ibm.com>
> Date:   Mon Feb 5 09:16:14 2018 +0000
> 
>     Add runtime check if mutex will be elided in tst-mutex8 testcases.
>     
>     An elided mutex don't fail destroy. Elision was disabled for the
>     test nptl/tst-mutex8 in nptl/Makefile. Thus we can run tests which
>     destroy a locked mutex.
>     
>     As elision is only disabled for tst-mutex8, the variants
>     tst-mutex8-static, tst-mutexpi8 and tst-mutexpi8-static are still
>     failing if lock elision is enabled.
>     
>     This patch adds a runtime check, if the checked type of mutex will
>     be elided. This check is using TUNABLE_GET_FULL to determine if
>     elision is enabled via the tunables framework.
>     The pthread_mutex_destroy tests are only run if we dont't assume an
>     elided mutex.
>     
>     This way, we can run the whole glibc testsuite with or without enabled
>     lock elision.
>     
>     ChangeLog:
>     
>             * nptl/Makefile (tst-mutex8-ENV): Delete.
>             * nptl/tst-mutex8.c (check_type):
>             Add runtime check if mutex will be elided.
> 
> diff --git a/nptl/Makefile b/nptl/Makefile
> index 6fc2c8bb6a..9340f9f699 100644
> --- a/nptl/Makefile
> +++ b/nptl/Makefile
> @@ -726,10 +726,6 @@ endif
>  
>  $(objpfx)tst-compat-forwarder: $(objpfx)tst-compat-forwarder-mod.so
>  
> -# Disable elision for tst-mutex8 so it can verify error case for
> -# destroying a mutex.
> -tst-mutex8-ENV = GLIBC_TUNABLES=glibc.elision.enable=0
> -

OK.

>  # The tests here better do not run in parallel
>  ifneq ($(filter %tests,$(MAKECMDGOALS)),)
>  .NOTPARALLEL:
> diff --git a/nptl/tst-mutex8.c b/nptl/tst-mutex8.c
> index 516ef3809e..d31f342751 100644
> --- a/nptl/tst-mutex8.c
> +++ b/nptl/tst-mutex8.c
> @@ -22,7 +22,8 @@
>  #include <stdbool.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> -
> +#include <unistd.h>
> +#include <elf/dl-tunables.h>

OK.

>  
>  static pthread_mutex_t *m;
>  static pthread_barrier_t b;
> @@ -95,6 +96,30 @@ check_type (const char *mas, pthread_mutexattr_t *ma)
>  {
>    int e;
>  
> +  /* Check if a mutex will be elided.  Lock elision can only be activated via
> +     the tunables framework.  By default, lock elision is disabled.  */
> +  bool assume_elided_mutex = false;
> +#if HAVE_TUNABLES
> +  int ma_type = PTHREAD_MUTEX_TIMED_NP;
> +  if (ma != NULL)
> +    {
> +      e = pthread_mutexattr_gettype (ma, &ma_type);
> +      if (e != 0)
> +	{
> +	  printf ("pthread_mutexattr_gettype failed with %d (%m)\n", e);
> +	  return 1;
> +	}
> +    }
> +  if (ma_type == PTHREAD_MUTEX_TIMED_NP)
> +    {
> +      /* This type of mutex can be elided if elision is enabled via the tunables
> +	 framework.  Some tests below are failing if the mutex is elided.
> +	 Thus we only run those if we assume that the mutex won't be elided.  */
> +      if (TUNABLE_GET_FULL (glibc, elision, enable, int32_t, NULL) == 1)
> +	assume_elided_mutex = true;
> +    }
> +#endif

OK.

> +
>    e = pthread_mutex_init (m, ma);
>    if (e != 0)
>      {
> @@ -127,19 +152,23 @@ check_type (const char *mas, pthread_mutexattr_t *ma)
>        return 1;
>      }
>  
> -  /* Elided mutexes don't fail destroy, but this test is run with
> -     elision disabled so we can test them.  */
> -  e = pthread_mutex_destroy (m);
> -  if (e == 0)
> +  /* Elided mutexes don't fail destroy, thus only test this if we don't assume
> +     elision.  */
> +  if (assume_elided_mutex == false)
>      {
> -      printf ("mutex_destroy of self-locked mutex succeeded for %s\n", mas);
> -      return 1;
> -    }
> -  if (e != EBUSY)
> -    {
> -      printf ("mutex_destroy of self-locked mutex did not return EBUSY %s\n",
> -	      mas);
> -      return 1;
> +      e = pthread_mutex_destroy (m);
> +      if (e == 0)
> +	{
> +	  printf ("mutex_destroy of self-locked mutex succeeded for %s\n", mas);
> +	  return 1;
> +	}
> +      if (e != EBUSY)
> +	{
> +	  printf ("\
> +mutex_destroy of self-locked mutex did not return EBUSY %s\n",
> +		  mas);
> +	  return 1;
> +	}

OK.

>      }
>  
>    if (pthread_mutex_unlock (m) != 0)
> @@ -155,18 +184,22 @@ check_type (const char *mas, pthread_mutexattr_t *ma)
>      }
>  
>    /* Elided mutexes don't fail destroy.  */
> -  e = pthread_mutex_destroy (m);
> -  if (e == 0)
> +  if (assume_elided_mutex == false)
>      {
> -      printf ("mutex_destroy of self-trylocked mutex succeeded for %s\n", mas);
> -      return 1;
> -    }
> -  if (e != EBUSY)
> -    {
> -      printf ("\
> +      e = pthread_mutex_destroy (m);
> +      if (e == 0)
> +	{
> +	  printf ("mutex_destroy of self-trylocked mutex succeeded for %s\n",
> +		  mas);
> +	  return 1;
> +	}
> +      if (e != EBUSY)
> +	{
> +	  printf ("\
>  mutex_destroy of self-trylocked mutex did not return EBUSY %s\n",
> -	      mas);
> -      return 1;
> +		  mas);
> +	  return 1;
> +	}

OK.

>      }
>  
>    if (pthread_mutex_unlock (m) != 0)
> @@ -203,17 +236,21 @@ mutex_destroy of self-trylocked mutex did not return EBUSY %s\n",
>      }
>  
>    /* Elided mutexes don't fail destroy.  */
> -  e = pthread_mutex_destroy (m);
> -  if (e == 0)
> -    {
> -      printf ("mutex_destroy of condvar-used mutex succeeded for %s\n", mas);
> -      return 1;
> -    }
> -  if (e != EBUSY)
> +  if (assume_elided_mutex == false)
>      {
> -      printf ("\
> +      e = pthread_mutex_destroy (m);
> +      if (e == 0)
> +	{
> +	  printf ("mutex_destroy of condvar-used mutex succeeded for %s\n",
> +		  mas);
> +	  return 1;
> +	}
> +      if (e != EBUSY)
> +	{
> +	  printf ("\
>  mutex_destroy of condvar-used mutex did not return EBUSY for %s\n", mas);
> -      return 1;
> +	  return 1;
> +	}

OK.

>      }
>  
>    done = true;
> @@ -274,19 +311,22 @@ mutex_destroy of condvar-used mutex did not return EBUSY for %s\n", mas);
>      }
>  
>    /* Elided mutexes don't fail destroy.  */
> -  e = pthread_mutex_destroy (m);
> -  if (e == 0)
> -    {
> -      printf ("2nd mutex_destroy of condvar-used mutex succeeded for %s\n",
> -	      mas);
> -      return 1;
> -    }
> -  if (e != EBUSY)
> +  if (assume_elided_mutex == false)
>      {
> -      printf ("\
> +      e = pthread_mutex_destroy (m);
> +      if (e == 0)
> +	{
> +	  printf ("2nd mutex_destroy of condvar-used mutex succeeded for %s\n",
> +		  mas);
> +	  return 1;
> +	}
> +      if (e != EBUSY)
> +	{
> +	  printf ("\
>  2nd mutex_destroy of condvar-used mutex did not return EBUSY for %s\n",
> -	      mas);
> -      return 1;
> +		  mas);
> +	  return 1;
> +	}

OK.

>      }
>  
>    if (pthread_cancel (th) != 0)


-- 
Cheers,
Carlos.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]