Re: [PATCH] Async signal safe TLS accesses

[OndÅej BÃlka <neleai at seznam dot cz>]

On Sat, Nov 30, 2013 at 05:16:21PM -0200, Alexandre Oliva wrote:
> On Nov 29, 2013, "Carlos O'Donell" <carlos@redhat.com> wrote:
> 
> > The one page is wasted for the sake of simplicity of implementation
> > and maintenance. The more complex this code becomes the harder it 
> > is to review for AS-safety.
> 
> While I agree with the latter sentence in principle, I think this goes
> too far in complexity-avoidance, and I claim we already have a
> provably-safe allocation strategy that reuses malloc, does not incur
> such waste, and is very performant.
> 
> I've already alluded to it in my earlier email: using the existing
> malloc implementation with an arena that is only ever locked when
> signals are disabled.
> 

> Why is this AS-Safe?
> 
> The reasoning starts from the premise that our implementation of malloc
> is MT-Safe.  This is relevant because the main differences between
> MT-Safe and AS-Safety is that the intervening code out of signal
> handling runs on the same thread, and the interrupted function only gets
> control back when the handler is done.  Therefore, if we can show that
> this difference doesn't matter for a certain MT-Safe implementation,
> we've shown that it is also AS-Safe.
> 
> Here's an exhaustive list of situations in which the difference could
> matter:
> 
> 1. thread-local state is used without any form of locking.  This does
> not apply to our arena-based implementation of malloc: the only
> thread-local state is used to determine which arena to use.
> 
Actualy that is enough to get a unconditional cross-platform AS-safety
with penalty of around 4 cycles per invocation.
In my earlier proposal of async-safe wrapper I thought that performance
problems talk againist it. But when we use TLS anyway it suffices just
to get pointer and then it it just one load, two stores and a
comparison.

> 2. global state is used without any form of synchronization.  This would
> violate the premise that the code is MT-Safe, and it does not apply to
> our arena-based implementation of malloc.
> 
> 3. global state used without locking, but with atomics and busy waits.
> Busy waits can be a significant difference: while another thread whose
> completion would break the busy wait loop will eventually get there, a
> signal handler might wait indefinitely for a condition to hold, that
> would only hold if the interrupted thread would complete the interrupted
> operation.  The arena-based implementation of malloc uses atomics for
> statistics (no loops), for fastbin allocation and release (CAS loops,
> that just make sure the value we're replacing is the one we meant to
> replace; uses interrupted by signals would not cause the handler or the
> interruptee to misbehave or loop indefinitely, although the interruptee
> may get additional iterations if the handler changes the value).
> 
> 4. global state is used with recursive locking.  While this would
> exclude other threads from concurrent access to this global state, it
> wouldn't exclude the same thread within a signal handler.  Our
> arena-based implementaton of malloc does not use recursive locks, so
> this difference does not matter.
> 
> 5. global state is used with non-recursive locking.  This would exclude
> even the thread itself from concurrent access to the global state within
> a signal handler: we'd get a deadlock if the signal interrupted a thread
> that held the lock.
> 
> The only situations that matters for our arena-based malloc
> implementation are 3. and 5.  I've already covered 3., and there's no
> reason for concern there; 5. is easy to avoid: never take the lock with
> signals enabled.
> 
> Now, we don't want to disable signals around every heap operation; that
> might turn out to be quite expensive.  We can however easily avoid this
> penalty without sacrificing MT-Safety:
> 
> a) use trylock to lock a thread's preferred arena, and if that fails,
> fallback to the wasteful mmap-based implementation, regardless of size
>
That I wrote in thread 

[RFC][BZ #16159] Detecting that recursive mutex recursed.

You need to have reentrant trylock. This is possible in worst case by implementing that ourselves like

int
trylock (int &x)
{
  return atomic_compare_and_exchange_bool_acq (x, 0, 1);
}

void
unlock(int &x)
{ /* Always succeeds.  */
  atomic_compare_and_exchange_bool_acq (x, 1, 0);
}

But you need to document reentrancy requirements.

> b) use trylock as in (a), but if that fails, block all signals and use
> (create on demand) a (global?  per-thread?) fallback arena that's only
> ever locked with signals blocked.  This arena, if added to the list of
> arenas (avoiding its use as a fallback), would have to be added using
> atomic operations rather than taking the list-management lock.
> 
Does that matter when signals are error-path use case?

> c) variations on these themes, such as a fallback arena as currently
> used, 
> 
> 
> This suffices for this implementation to be AS-Safe, while using the
> existing malloc implementation.  The only care it needs to take, as was
> pointed out elsewhere, is to never use the main arena, for that could
> interfere with brk as used by a user-replaced malloc.
> 
> Should we want to extend the AS-Safe property to the malloc we expose to
> users, as opposed to limiting it to an internal API for more stringent
> environments, all we'd have to do is extend the arena-selection
> machinery to the same fallback machinery, and switch to atomics-based
> arena list mgmt or block signals while taking that lock.
> 
> -- 
> 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 Brazil Compiler Engineer