Re: [Libunwind-devel] Updated fast trace patch with initial performance results

[Lassi Tuura]

Hi,

> I've looked at the committed patch, and I think there may be a problem ;-(
> 
> You are using pthread_setspecific() to set the thread-local trace cache,
> but pthread_setspecific may call calloc.
> 
> I believe that may cause a deadlock if you are unwinding from a signal,
> or from malloc/calloc itself.

Ah, thanks for noticing that. As per your other message, I don't think I ever 
exceeded the limit of 32 TLS items to trigger this.

> Fortunately, all Linux/x86_64 systems support __thread, which doesn't suffer
> from the same problem.
> 
> Is there a particular reason you didn't use __thread?
> If not, I'll send a patch.

All clear from me to use __thread. I originally used it, but changed because 
include/libunwind_i.h says:

   38 #ifdef HAVE___THREAD
   39   /* For now, turn off per-thread caching.  It uses up too much TLS
   40      memory per thread even when the thread never uses libunwind at
   41      all.  */
   42 # undef HAVE___THREAD
   43 #endif

I meant to ask if someone could expand on that comment? I thought it wouldn't 
apply to the fast trace, which could use __thread, but I am not familiar enough 
with __thread implementations out there to know what triggered that comment so 
I backed off.

Did that comment mean that merely linking in libunwind causes too high 
per-thread cost when using __thread? Or how it is used (for address space cache 
implementation inside libunwind, ia64-only) ends up gobbling too much memory, 
and using __thread is ok elsewhere in the code?

Only src/ia64/Gscript.c and src/mi/Gset_caching_policy.c use HAVE___THREAD. 
There are no __thread uses that aren't protected by HAVE___THREAD.

Anyway that #undef sort of stops use of __thread, unless we go ahead and use it 
anyway, without checking whether it's supported?

Thanks for the review.

Regards,
Lassi