PR libgomp/51376 fix

[Alan Modra]

On Fri, Dec 02, 2011 at 08:10:11PM +1030, Alan Modra wrote:
> On Thu, Dec 01, 2011 at 12:36:08PM +0100, Jakub Jelinek wrote:
> > On Thu, Dec 01, 2011 at 09:58:08PM +1030, Alan Modra wrote:
> > > The GOMP_task change fixes a similar potential problem.  Bootstrapped
> > > and regression tested powerpc-linux.  OK to apply?
> > > 
> > > 	PR libgomp/51376
> > > 	* task.c (GOMP_taskwait): Don't access task->children outside of
> > > 	task_lock mutex region.
> > > 	(GOMP_task): Likewise.
> > 
> > Can't this be solved just by adding a barrier?  The access to the var
> > outside of the lock has been quite intentional, to avoid locking in the
> > common case where there are no children.
> 
> No, I tried that and the task-6.C testcase still failed although not
> quite as readily.  I was using
> 
> if (task == NULL
>     || __atomic_load_n (&task->children, MEMMODEL_ACQUIRE) == 0)
> 
> You need a release in the child as well as the acquire to ensure
> proper synchronisation, and there's a window for failure between the
> child clearing task->children and performing a release as part of the
> mutex unlock.

Perhaps I misunderstood your question.  I suppose you could solve the
problem by adding a barrier, if by that you meant the acquire barrier
in GOMP_taskwait as I tried before I fully understood the problem,
plus the corresponding release barrier in gomp_barrier_handle_tasks.
ie. replace
		      parent->children = NULL;
with
		      __atomic_write_n (&parent->children, NULL,
					MEMMODEL_RELEASE);

That should work, but of course will slow down what I imagine is a
common multi-thread case where child threads don't complete before the
parent gets around to a taskwait.  Do you really want to do that?

-- 
Alan Modra
Australia Development Lab, IBM