Re: [PATCH] Async signal safe TLS accesses

[Andrew Hunter <ahh at google dot com>]

On Thu, Dec 5, 2013 at 2:47 PM, OndÅej BÃlka <neleai@seznam.cz> wrote:
> On Thu, Dec 05, 2013 at 12:01:11PM -0800, Andrew Hunter wrote:
>>     TLS accesses from initial-exec variables are async-signal-safe.  Even
>> +
>> +  size_t pg = GLRO (dl_pagesize);
>> +  size_t padded_size;
>> +  if (boundary <= pg)
>> +    {
>> +      /* We'll get a pointer certainly aligned to boundary, so just
>> +      add one more boundary-sized chunk to hold the header.  */
>> +      padded_size = roundup (size, boundary) + boundary;
>> +    }
>> +  else
>> +    {
>> +      /* If we want K pages aligned to a J-page boundary, K+J+1 pages
>> +      contains at least one such region that isn't directly at the start
>> +      (so we can place the header.)  This is wasteful, but you're the one
>> +      who wanted 64K-aligned TLS.  */
>> +      padded_size = roundup (size, pg) + boundary + pg;
>> +    }
>> +
> Is special casing here really needed?
>

shrug, "needed" is hard to say, but given that it's written already I
don't see a reason to actively exclude it.  I don't think it
dramatically complicates things.

>
> snip
>
>> +      /* busywork */
>> +      free (malloc (128));
>
> that gets optimized away.
>

It does not:
0000000000401780 <spin>:
  401780:       48 83 ec 08             sub    $0x8,%rsp
  401784:       0f 1f 40 00             nopl   0x0(%rax)
  401788:       bf 80 00 00 00          mov    $0x80,%edi
  40178d:       e8 5e f9 ff ff          callq  4010f0 <malloc@plt>
  401792:       48 89 c7                mov    %rax,%rdi
  401795:       e8 06 f8 ff ff          callq  400fa0 <free@plt>
  40179a:       eb ec                   jmp    401788 <spin+0x8>
  40179c:       0f 1f 40 00             nopl   0x0(%rax)


It could be in principle and perhaps one should disable builtin malloc
in Makefile, but this actually works as written with my gcc (4.6.3).

> snip
>
>> +     pthread_sigmask in that we do not mask internal signals used for
>> +     cancellation and setxid handling. This disables asyncrhonous
>
> s/asynchronous
>

Fixed in next version of patch, thanks.