This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH][BZ 18743] PowerPC: Fix a race condition when eliding a lock
- From: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- To: Tulio Magno Quites Machado Filho <tuliom at linux dot vnet dot ibm dot com>
- Cc: libc-alpha at sourceware dot org
- Date: Fri, 31 Jul 2015 18:59:33 -0300
- Subject: Re: [PATCH][BZ 18743] PowerPC: Fix a race condition when eliding a lock
- Authentication-results: sourceware.org; auth=none
- References: <1438274936-26493-1-git-send-email-tuliom at linux dot vnet dot ibm dot com> <55BA703D dot 7010303 at linaro dot org> <874mkl3wtq dot fsf at totoro dot lan> <55BB76FA dot 5040703 at linaro dot org> <87zj2c1ij1 dot fsf at totoro dot lan> <55BBD3E9 dot 8040005 at linaro dot org> <87oaisxaqu dot fsf at totoro dot lan>
On 31-07-2015 18:44, Tulio Magno Quites Machado Filho wrote:
> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>
>> On 31-07-2015 11:57, Tulio Magno Quites Machado Filho wrote:
>>> Let's say t1 is the first one to read rwlock->__data fields and they're all 0.
>>> t1 will set is_lock_free to 0.
>>
>> I believe you mean is_lock_free will be set to '1' if all fields are '0' (since
>> the lock will not be held).
>
> Yes! I'm sorry.
>
>> I see now a window where the pthread_rwlock_t internal structures might have
>> concurrent access and the transaction is never finished. However I do not
>> think this is a powerpc specific and thus I think it should be fixed in generic
>> algorithm instead. Something like this:
>>
>> ...
>>
>> I did not fix the x86_64 code, which would require some adjustments. I checked
>> on ppc64le and make nptl/tests passes.
>
> I didn't test your patch, but after reading it, I'm sure it does fix the
> problem too.
>
>> It would be good if you could create a
>> testcase to stress this issue (and I do see this could be quite hard since it is
>> very timing dependent).
>
> Yes.
> I tried, but I didn't like the idea of creating a testcase that would only
> provide intermittent failures. Especially because it's very difficult to
> reproduce this failure.
> Any suggestions?
>
>> The problem with this approach is it couple the elide macros with rdlock
>> fields, and I think the idea would make this header more generic for multiple
>> elide algorithms. Suggestions?
>
> And that's why I preferred to modify only the powerpc implementation.
> Both proposals are identical, in the sense that they move the memory access
> to the transaction.
>
> I'd appreciate if you could explain why you prefer to change the argument list
> of ELIDE_LOCK instead of the powerpc specific code.
>
Mainly because I see this issue is not powerpc specific and I think we should
also fix for x86 (that currently also uses the same mechanism) and for future
arches that also might potentially implement lock elision using this.