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][BZ 18743] PowerPC: Fix a race condition when eliding a lock


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.

-- 
Tulio Magno


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