Correct mutex type check for disable lock elision via pthread_mutexattr_settype call

Message ID lhdtdc$ek0$1@ger.gmane.org
State Committed
Headers

Commit Message

Stefan Liebler April 1, 2014, 8:29 a.m. UTC
  Hi,

if glibc is build with --enable-lock-elision=yes,
all default mutexes are elided (only for supported architectures).
Someone can disable the elision for one specific mutex with
a call to pthread_mutexattr_settype.
Currently you can specify either PTHREAD_MUTEX_NORMAL or 
PTHREAD_MUTEX_DEFAULT, because they are both defined to 0.
The function checks for type PTHREAD_MUTEX_DEFAULT
and then disables elision.

For correctness, the function should check against PTHREAD_MUTEX_NORMAL!
According to POSIX, PTHREAD_MUTEX_NORMAL shall deadlock if a thread 
tries to relock a mutex without first unlocking it.
In case of PTHREAD_MUTEX_DEFAULT, the behavior is undefined.
Thus the mutex can be elided with PTHREAD_MUTEX_DEFAULT, but not with 
PTHREAD_MUTEX_NORMAL.
The same can be read in the lock elision implementation guidelines
(https://sourceware.org/glibc/wiki/LockElisionGuide).

Bye.

---
2014-04-01  Stefan Liebler  <stli@linux.vnet.ibm.com>

	* nptl/pthread_mutexattr_settype.c
	(__pthread_mutexattr_settype):
	Disable lock elision for PTHREAD_MUTEX_NORMAL.
---
  

Comments

Stefan Liebler April 8, 2014, 12:08 p.m. UTC | #1
ping!
On 04/01/2014 10:29 AM, Stefan Liebler wrote:
> Hi,
>
> if glibc is build with --enable-lock-elision=yes,
> all default mutexes are elided (only for supported architectures).
> Someone can disable the elision for one specific mutex with
> a call to pthread_mutexattr_settype.
> Currently you can specify either PTHREAD_MUTEX_NORMAL or
> PTHREAD_MUTEX_DEFAULT, because they are both defined to 0.
> The function checks for type PTHREAD_MUTEX_DEFAULT
> and then disables elision.
>
> For correctness, the function should check against PTHREAD_MUTEX_NORMAL!
> According to POSIX, PTHREAD_MUTEX_NORMAL shall deadlock if a thread
> tries to relock a mutex without first unlocking it.
> In case of PTHREAD_MUTEX_DEFAULT, the behavior is undefined.
> Thus the mutex can be elided with PTHREAD_MUTEX_DEFAULT, but not with
> PTHREAD_MUTEX_NORMAL.
> The same can be read in the lock elision implementation guidelines
> (https://sourceware.org/glibc/wiki/LockElisionGuide).
>
> Bye.
>
> ---
> 2014-04-01  Stefan Liebler  <stli@linux.vnet.ibm.com>
>
>      * nptl/pthread_mutexattr_settype.c
>      (__pthread_mutexattr_settype):
>      Disable lock elision for PTHREAD_MUTEX_NORMAL.
> ---
  
Stefan Liebler April 14, 2014, 7:06 a.m. UTC | #2
ping.

ok to commit?

On 04/08/2014 02:08 PM, Stefan Liebler wrote:
> ping!
> On 04/01/2014 10:29 AM, Stefan Liebler wrote:
>> Hi,
>>
>> if glibc is build with --enable-lock-elision=yes,
>> all default mutexes are elided (only for supported architectures).
>> Someone can disable the elision for one specific mutex with
>> a call to pthread_mutexattr_settype.
>> Currently you can specify either PTHREAD_MUTEX_NORMAL or
>> PTHREAD_MUTEX_DEFAULT, because they are both defined to 0.
>> The function checks for type PTHREAD_MUTEX_DEFAULT
>> and then disables elision.
>>
>> For correctness, the function should check against PTHREAD_MUTEX_NORMAL!
>> According to POSIX, PTHREAD_MUTEX_NORMAL shall deadlock if a thread
>> tries to relock a mutex without first unlocking it.
>> In case of PTHREAD_MUTEX_DEFAULT, the behavior is undefined.
>> Thus the mutex can be elided with PTHREAD_MUTEX_DEFAULT, but not with
>> PTHREAD_MUTEX_NORMAL.
>> The same can be read in the lock elision implementation guidelines
>> (https://sourceware.org/glibc/wiki/LockElisionGuide).
>>
>> Bye.
>>
>> ---
>> 2014-04-01  Stefan Liebler  <stli@linux.vnet.ibm.com>
>>
>>      * nptl/pthread_mutexattr_settype.c
>>      (__pthread_mutexattr_settype):
>>      Disable lock elision for PTHREAD_MUTEX_NORMAL.
>> ---
>
>
>
  
Stefan Liebler April 23, 2014, 11:30 a.m. UTC | #3
ping
On 04/14/2014 09:06 AM, Stefan Liebler wrote:
> ping.
>
> ok to commit?
>
> On 04/08/2014 02:08 PM, Stefan Liebler wrote:
>> ping!
>> On 04/01/2014 10:29 AM, Stefan Liebler wrote:
>>> Hi,
>>>
>>> if glibc is build with --enable-lock-elision=yes,
>>> all default mutexes are elided (only for supported architectures).
>>> Someone can disable the elision for one specific mutex with
>>> a call to pthread_mutexattr_settype.
>>> Currently you can specify either PTHREAD_MUTEX_NORMAL or
>>> PTHREAD_MUTEX_DEFAULT, because they are both defined to 0.
>>> The function checks for type PTHREAD_MUTEX_DEFAULT
>>> and then disables elision.
>>>
>>> For correctness, the function should check against PTHREAD_MUTEX_NORMAL!
>>> According to POSIX, PTHREAD_MUTEX_NORMAL shall deadlock if a thread
>>> tries to relock a mutex without first unlocking it.
>>> In case of PTHREAD_MUTEX_DEFAULT, the behavior is undefined.
>>> Thus the mutex can be elided with PTHREAD_MUTEX_DEFAULT, but not with
>>> PTHREAD_MUTEX_NORMAL.
>>> The same can be read in the lock elision implementation guidelines
>>> (https://sourceware.org/glibc/wiki/LockElisionGuide).
>>>
>>> Bye.
>>>
>>> ---
>>> 2014-04-01  Stefan Liebler  <stli@linux.vnet.ibm.com>
>>>
>>>      * nptl/pthread_mutexattr_settype.c
>>>      (__pthread_mutexattr_settype):
>>>      Disable lock elision for PTHREAD_MUTEX_NORMAL.
>>> ---
>>
>>
>>
>
>
>
  
Stefan Liebler April 30, 2014, 2:33 p.m. UTC | #4
ping
On 04/23/2014 01:30 PM, Stefan Liebler wrote:
> ping
> On 04/14/2014 09:06 AM, Stefan Liebler wrote:
>> ping.
>>
>> ok to commit?
>>
>> On 04/08/2014 02:08 PM, Stefan Liebler wrote:
>>> ping!
>>> On 04/01/2014 10:29 AM, Stefan Liebler wrote:
>>>> Hi,
>>>>
>>>> if glibc is build with --enable-lock-elision=yes,
>>>> all default mutexes are elided (only for supported architectures).
>>>> Someone can disable the elision for one specific mutex with
>>>> a call to pthread_mutexattr_settype.
>>>> Currently you can specify either PTHREAD_MUTEX_NORMAL or
>>>> PTHREAD_MUTEX_DEFAULT, because they are both defined to 0.
>>>> The function checks for type PTHREAD_MUTEX_DEFAULT
>>>> and then disables elision.
>>>>
>>>> For correctness, the function should check against
>>>> PTHREAD_MUTEX_NORMAL!
>>>> According to POSIX, PTHREAD_MUTEX_NORMAL shall deadlock if a thread
>>>> tries to relock a mutex without first unlocking it.
>>>> In case of PTHREAD_MUTEX_DEFAULT, the behavior is undefined.
>>>> Thus the mutex can be elided with PTHREAD_MUTEX_DEFAULT, but not with
>>>> PTHREAD_MUTEX_NORMAL.
>>>> The same can be read in the lock elision implementation guidelines
>>>> (https://sourceware.org/glibc/wiki/LockElisionGuide).
>>>>
>>>> Bye.
>>>>
>>>> ---
>>>> 2014-04-01  Stefan Liebler  <stli@linux.vnet.ibm.com>
>>>>
>>>>      * nptl/pthread_mutexattr_settype.c
>>>>      (__pthread_mutexattr_settype):
>>>>      Disable lock elision for PTHREAD_MUTEX_NORMAL.
>>>> ---
>>>
>>>
>>>
>>
>>
>>
>
>
>
  
Ondrej Bilka May 23, 2014, 1:51 p.m. UTC | #5
On Tue, Apr 01, 2014 at 10:29:32AM +0200, Stefan Liebler wrote:
> Hi,
> 
> if glibc is build with --enable-lock-elision=yes,
> all default mutexes are elided (only for supported architectures).
> Someone can disable the elision for one specific mutex with
> a call to pthread_mutexattr_settype.
> Currently you can specify either PTHREAD_MUTEX_NORMAL or
> PTHREAD_MUTEX_DEFAULT, because they are both defined to 0.
> The function checks for type PTHREAD_MUTEX_DEFAULT
> and then disables elision.
> 
> For correctness, the function should check against PTHREAD_MUTEX_NORMAL!
> According to POSIX, PTHREAD_MUTEX_NORMAL shall deadlock if a thread
> tries to relock a mutex without first unlocking it.
> In case of PTHREAD_MUTEX_DEFAULT, the behavior is undefined.
> Thus the mutex can be elided with PTHREAD_MUTEX_DEFAULT, but not
> with PTHREAD_MUTEX_NORMAL.
> The same can be read in the lock elision implementation guidelines
> (https://sourceware.org/glibc/wiki/LockElisionGuide).
> 
> Bye.
> 
> ---
> 2014-04-01  Stefan Liebler  <stli@linux.vnet.ibm.com>
> 
> 	* nptl/pthread_mutexattr_settype.c
> 	(__pthread_mutexattr_settype):
> 	Disable lock elision for PTHREAD_MUTEX_NORMAL.
> ---

> diff --git a/nptl/pthread_mutexattr_settype.c b/nptl/pthread_mutexattr_settype.c
> index 0e91292..cf648cf 100644
> --- a/nptl/pthread_mutexattr_settype.c
> +++ b/nptl/pthread_mutexattr_settype.c
> @@ -32,7 +32,7 @@ __pthread_mutexattr_settype (attr, kind)
>  
>    /* Cannot distinguish between DEFAULT and NORMAL. So any settype
>       call disables elision for now.  */
> -  if (kind == PTHREAD_MUTEX_DEFAULT)
> +  if (kind == PTHREAD_MUTEX_NORMAL)
>      kind |= PTHREAD_MUTEX_NO_ELISION_NP;
>  
>    iattr = (struct pthread_mutexattr *) attr;

Looks ok.
  

Patch

diff --git a/nptl/pthread_mutexattr_settype.c b/nptl/pthread_mutexattr_settype.c
index 0e91292..cf648cf 100644
--- a/nptl/pthread_mutexattr_settype.c
+++ b/nptl/pthread_mutexattr_settype.c
@@ -32,7 +32,7 @@  __pthread_mutexattr_settype (attr, kind)
 
   /* Cannot distinguish between DEFAULT and NORMAL. So any settype
      call disables elision for now.  */
-  if (kind == PTHREAD_MUTEX_DEFAULT)
+  if (kind == PTHREAD_MUTEX_NORMAL)
     kind |= PTHREAD_MUTEX_NO_ELISION_NP;
 
   iattr = (struct pthread_mutexattr *) attr;