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

[Florian Weimer <fweimer@redhat.com>]

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