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.
Oops, on looking at this email I see I attached an old patch.. This
one avoids a segfault on trying to lock team->task_lock when there
is no team. This one really has been bootstrapped and regression
tested successfully.
PR libgomp/51376
* task.c (GOMP_taskwait): Don't access task->children outside of
task_lock mutex region.
(GOMP_task): Likewise.
Index: libgomp/task.c
===================================================================
--- libgomp/task.c (revision 181902)
+++ libgomp/task.c (working copy)
@@ -116,10 +116,11 @@ GOMP_task (void (*fn) (void *), void *da
}
else
fn (data);
- if (task.children)
+ if (team != NULL)
{
gomp_mutex_lock (&team->task_lock);
- gomp_clear_parent (task.children);
+ if (task.children != NULL)
+ gomp_clear_parent (task.children);
gomp_mutex_unlock (&team->task_lock);
}
gomp_end_task ();
@@ -290,8 +291,9 @@ GOMP_taskwait (void)
struct gomp_task *child_task = NULL;
struct gomp_task *to_free = NULL;
- if (task == NULL || task->children == NULL)
+ if (task == NULL || team == NULL)
return;
+
gomp_mutex_lock (&team->task_lock);
while (1)
{
--
Alan Modra
Australia Development Lab, IBM
