Re: [PATCH] Fix race in pthread_mutex_lock while promoting to PTHREAD_MUTEX_ELISION_NP [BZ #23275]

[Stefan Liebler <stli@linux.ibm.com>]

On 06/13/2018 11:36 AM, Florian Weimer wrote:
> On 06/13/2018 11:18 AM, Stefan Liebler wrote:
>> On 06/13/2018 10:41 AM, Florian Weimer wrote:
>>> On 06/12/2018 04:24 PM, Stefan Liebler wrote:
>>>> The new testcase tst-mutex10 is triggering the race on s390x and 
>>>> intel. Presumably also on power, but I don't have access to a power 
>>>> machine with lock-elision. At least the code for power is the same 
>>>> as on the other two architectures. Can somebody test it on power?
>>>
>>> I tried the test case on a machine with:
>>>
>>> Model:                 2.0 (pvr 004d 0200)
>>> Model name:            POWER8 (raw), altivec supported
>>>
>>> AT_HWCAP:        true_le archpmu vsx arch_2_06 dfp ic_snoop smt mmu 
>>> fpu altivec ppc64 ppc32
>>> AT_HWCAP2:       htm-nosc vcrypto tar isel ebb dscr htm arch_2_07
>>>
>>> Presumably, that should have lock elision support?
>> Unfortunately, I don't know.
>> @Tulio: Can you answer this question?
>>
>> To be sure, can you use gdb and step into pthread_mutex_lock in order 
>> to check if the elision path is used depending on 
>> __pthread_force_elision.
>> Ensure, that elision is enabled:
>> (gdb) set environment GLIBC_TUNABLES glibc.elision.enable=1
> 
> __pthread_force_elision is 1.
> 
>>> If I apply the test (and only the test) to commit 
>>> a745c837cb51c2efe8900740548cb68ec2a2f7ab, the resulting glibc does 
>>> not show a test failure.
> 
>> I assume, that nptl/Makefile contains:
>> tst-mutex10-ENV = GLIBC_TUNABLES=glibc.elision.enable=1
> 
> Yes it does.
> 
>> You can use the tst-mutex10 arguments --iterations (default is 
>> 1000000) and --threads (default is 3). Perhaps we have to increase 
>> those values for power. At least on my s390x/x86_64 machine, those 
>> default values do trigger a test failure as pthread_mutex_destroy is 
>> returning EBUSY.
> 
> I played around with various settings, but I could not get a test 
> failure.  Only a test timeout because the 50-second timeout is 
> eventually not large enough.  glibc is compiled with assertions enabled.
> 
> Thanks,
> Florian
> 

I've also retested tst-mutex10 without the remaining patch on multiple 
s390x machines. I've found out, that it depends on the used gcc (at 
least on s390x). On one machine, gcc 6 was used. With gcc 7 / 8, the 
type of the mutex was loaded from memory multiple times.
With gcc 6, it is loaded only one time and thus all threads are 
promoting the mutex to PTHREAD_MUTEX_ELISION_NP. Then you don't see a fail.

On x86_64, I've used gcc 8. Perhaps I can try other gcc versions.
On power, perhaps gcc does also not reload the type of the mutex. I 
don't know.

On s390x, I've build the whole patch and run the testsuite with various 
gcc versions. There was no fail in tst-mutex10.

Bye.
Stefan