Re: [BZ#17090/17620/17621]: fix DTV race, assert, and DTV_SURPLUS Static TLS limit

[Alexandre Oliva <aoliva at redhat dot com>]

On Sep 29, 2016, Torvald Riegel <triegel@redhat.com> wrote:

> On Sun, 2016-09-25 at 19:09 -0300, Alexandre Oliva wrote:
>> On Sep 24, 2016, Torvald Riegel <triegel@redhat.com> wrote:
>> 
>> > On Fri, 2016-09-23 at 23:52 -0300, Alexandre Oliva wrote:
>> >> (if it decides to update it without
>> >> resizing, we also have a race, but both threads end up writing the same
>> >> final value, which AFAIK is not so much of a problem)
>> 
>> > Can this still happen after your patches?
>> 
>> After removing (again) one of the possibly-concurrent writes, I don't
>> see how it could still happen, since there's only one writer to a
>> thread's DTV at a time: at thread creation there's one, and while the
>> thread is active there's another.  But that's not the result of a
>> thorough global analysis from scratch, it's just my intuition based on
>> my limited understanding of the whole TLS architecture.  If you want a
>> thorough analysis, I'm afraid you'll have to resort to someone else,
>> probably someone who speaks the same language as you, and that reasons
>> at the same abstraction layer as you.  I'm not that person, as our many
>> attempts at conversations have shown.
>> 
>> > ISTM that all this should become a code comment, and not just be in an
>> > email on the list.
>> 
>> It's there in the patch.

> Apparently, I thought it wasn't.

> At the very least please include this sentence of yours (see above) in
> the code comments:
> "But that's not the result of a thorough global analysis from scratch,
> it's just my intuition based on my limited understanding of the whole
> TLS architecture."

> What I want to avoid is that somebody else tries to work on the code and
> believes in your comment as-is, when you already know that you haven't
> checked this completely.

I checked thoroughly what I wrote in the comment, namely, that there'd
be a DTV race if we did what the removed code used to do before.

What I meant is that I cannot affirm that there aren't any other DTV- or
TLS-related races, before or after the fix, because I haven't ever done
a thorough review of the TLS implementation, and I surely haven't done
one for purposes of posting the patch.

I just know enough about the code in general, and about concurrency
programming in general, to know that, if I remove that code, that was
unnecessary and that caused the race I had detected a year or so ago,
that specific race will be gone, and the remaining code will work the
way it did before the patch I installed last week.

What I know about these topics would allow me to make numerous other
claims, of course.  But what would the point be of my spelling any of
them out?  You wouldn't believe them anyway!  Even if I said I'd made a
thorough analysis, would you trust it?


>> > IIUC, that's were the assumption about dlopen vs. usage comes in that
>> > you included in the other patch.
>> 
>> Yeah, that too.
>> 
>> > If so, this is not a data race (nor a race condition).
>> 
>> It is a potential data race, yes,

> I'm saying that when making the assumption, which is essentially a
> precondition we (should) specify, this isn't a data race.  If violating
> the precondition, then that's already undefined behavior, but a fault of
> the caller.

Are you saying it ceases to be a data race just because some earlier
precondition failed to be observed?  I don't think I agree with that.
But I don't think I'm interested in being part of such a discussion with
you either.

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer