Re: [PR18457] Don't require rtld lock to compute DTV addr for static TLS

["Carlos O'Donell" <carlos at redhat dot com>]

On 06/05/2015 12:23 AM, Alexandre Oliva wrote:
> On Jun  4, 2015, "Carlos O'Donell" <carlos@redhat.com> wrote:
> 
>> I agree with Siddhesh and Torvald, it's not a good enough solution.
> 
> Again, good enough for what?

The solution does not meet my standards for fixing atomicity issues.
It lacks proper documentation on the synchronization changes being made.

I've asked Siddhesh to look into this issue and document some of the
expectations and perhaps resolve some of the broader issues as well.
We have received real customer reports of at least 3 kinds of P&C
related failures in this code. So we are going to expand the nature
of the fix and see if there isn't an overall cleaner solution.

> It is undisputably enough to fix the performance regression that in
> certain cases introduced new deadlocks to the pile of other possible
> deadlocks that variants of the testcase could exercise.  It fixes a
> small problem, and it's a trivial patch.

I agree.
 
> Sure, it doesn't fix a much larger preexisting problem, that's being
> misrepresented as a TLS issue, but that is actually a far more pervasive
> dynamic loader problem.  I have no interest or time in tackling that.

I appreciate your honesty in expressing what you are able to do and what
you want to do.

> Sure, if you guys want to leave the regression unfixed until someone
> figures out some way to fix a larger but mostly unrelated problem, it's
> your choice.

Siddhesh and I will be working on it right now since more than one of the
problems are impacting real customer applications.

> But if this larger fix does not contain the very change I'm proposing,
> tls_get_addr on variables assigned static TLS will remain much slower
> than needed, because there's absolutely no need to take a lock when it
> is already decided whether the module should use static or dynamic TLS.

I agree there is no need to take the lock, but your patch needs to be
expanded in more detail to make it crystal clear exactly what
synchronization is being done and why.

> So what is this simple regression fix waiting for to get installed,
> again?

As of today only Fedora is using your previous patches which reveal these
older bugs, and until we branch for 2.22, this patch can wait while we
look into a more holistic fix.

Cheers,
Carlos.