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
