Re: ping x 7: [PATCH] [libgomp] make it possible to use OMP on both sides of a fork

[Jakub Jelinek <jakub@redhat.com>]

On Mon, Oct 13, 2014 at 10:16:19PM +0100, Nathaniel Smith wrote:
> Got total silence the last 4 times I posted this, and users have been
> bugging me about it offline, so trying again.
> 
> This patch fixes a showstopper problem preventing the transparent use
> of OpenMP in scientific libraries, esp. with Python. Specifically, it
> is currently not possible to use GNU OpenMP -- even in a limited,
> temporary manner -- in any program that uses (or might use) fork() for
> parallelism, even if the fork() and the use of OpenMP occur at totally
> different times. This limitation is unique to GNU OpenMP -- every
> competing OpenMP implementation already contains something like this
> patch. While technically not fully POSIX-compliant (because POSIX
> gives much much weaker guarantees around fork() than any real Unix),
> the approach used in this patch (a) performs only POSIX-compliant
> operations when the host program is itself fully POSIX-compliant, and
> (b) actually works perfectly reliably in practice on all commonly used
> platforms I'm aware of.

1) gomp_we_are_forked in your patch will attempt to free the pool
   of the thread that encounters it, which is racy; consider a program
   after fork calling pthread_create several times, each thread
   thusly created then ~ at the same time doing #pragma omp parallel
   and the initial thread too.  You really should clean up the pool
   data structure only in the initial thread and nowhere else;
   for native TLS (non-emulated, IE model) the best would be to have a flag
   in the gomp_thread_pool structure,
   struct gomp_thread *thr = gomp_thread ();
   if (thr && thr->thread_pool)
     thr->thread_pool->after_fork = true;
   should in that case be safe in the atfork child handler.
   For !HAVE_TLS or emulated TLS not sure if it is completely safe,
   it would call pthread_getspecific.  Perhaps just don't register
   atfork handler on those targets at all?
2) can you explain why are you removing the cleanups from
   gomp_free_pool_helper ?
3) you can call pthread_atfork many times (once for each pthread
   that creates a thread pool), that is undesirable, you want to do that
   only if the initial thread creates thread pool
4) the testcase is clearly not portable enough, should be probably limited
   to *-*-linux* only, fork etc. will likely not work on many targets.

In any case, even with the patch, are you aware that you'll leak megabytes
of thread stacks etc.?

	Jakub