[committed] libgomp: Fix occassional hangs with taskwait nowait depend

Message ID Yo31gLzfZg1hR5Nb@tucnak
State Committed
Headers
Series [committed] libgomp: Fix occassional hangs with taskwait nowait depend |

Commit Message

Jakub Jelinek May 25, 2022, 9:23 a.m. UTC
  Hi!

Richi reported occassional hangs with taskwait-depend-nowait-1.*
tests and I've finally manged to reproduce.  The problem is if
taskwait depend without nowait is encountered soon after
taskwait depend nowait and the former depends on the latter and there
is no other work to do, the taskwait depend without nowait is put
to sleep, but the empty_task optimization in
gomp_task_run_post_handle_dependers wouldn't wake it up in that
case.  gomp_task_run_post_handle_dependers normally does some wakeups
because it schedules more work (another task), which is not the
case of empty_task, but we need to do the wakeups that would be done
upon task completion so that we awake sleeping threads when the
last child is done.
So, the taskwait-depend-nowait-1.* testcase is fixed with the
else if (__builtin_expect (task->parent_depends_on, 0) part of
the patch.
The new testcase can hang on another problem, if the empty task
is the last task of a taskgroup, we need to use atomic store
like elsewhere to decrease the counter to 0, and wake up taskgroup
end if needed.
Yet another spot which can sleep is normal taskwait (without depend),
but I believe nothing needs to be done for that - in that case we
await solely until the children's queue has no tasks, tasks still
waiting for dependencies aren't accounted in that, but the reason
is that if taskwait should wait for something, there needs to be at least
one active child doing something (in the children queue), which then
possibly awakes some of its siblings when the dependencies are met,
or in the empty task case awakes further dependencies, but in any
case the child that finished is still handled as active child and
will awake taskwait at the end if there is nothing further to
do.
Last sleeping case are barriers, but that is handled by ++ret and
awaking the barrier.

Bootstrapped/regtested on x86_64-linux and i686-linux, committed
to trunk.

2022-05-25  Jakub Jelinek  <jakub@redhat.com>

	* task.c (gomp_task_run_post_handle_dependers): If empty_task
	is the last task taskwait depend depends on, wake it up.
	Similarly if it is the last child of a taskgroup, use atomic
	store instead of decrement and awak taskgroup wait if any.
	* testsuite/libgomp.c-c++-common/taskwait-depend-nowait-2.c: New test.


	Jakub
  

Patch

--- libgomp/task.c.jj	2022-05-23 22:38:26.381094885 +0200
+++ libgomp/task.c	2022-05-24 18:23:03.054074341 +0200
@@ -1382,10 +1382,30 @@  gomp_task_run_post_handle_dependers (str
 	{
 	  if (!parent)
 	    task->parent = NULL;
+	  else if (__builtin_expect (task->parent_depends_on, 0)
+		   && --parent->taskwait->n_depend == 0
+		   && parent->taskwait->in_depend_wait)
+	    {
+	      parent->taskwait->in_depend_wait = false;
+	      gomp_sem_post (&parent->taskwait->taskwait_sem);
+	    }
 	  if (gomp_task_run_post_handle_depend (task, team))
 	    ++ret;
 	  if (taskgroup)
-	    taskgroup->num_children--;
+	    {
+	      if (taskgroup->num_children > 1)
+		--taskgroup->num_children;
+	      else
+		{
+		  __atomic_store_n (&taskgroup->num_children, 0,
+				    MEMMODEL_RELEASE);
+		  if (taskgroup->in_taskgroup_wait)
+		    {
+		      taskgroup->in_taskgroup_wait = false;
+		      gomp_sem_post (&taskgroup->taskgroup_sem);
+		    }
+		}
+	    }
 	  gomp_finish_task (task);
 	  free (task);
 	  continue;
--- libgomp/testsuite/libgomp.c-c++-common/taskwait-depend-nowait-2.c.jj	2022-05-25 09:56:20.131618294 +0200
+++ libgomp/testsuite/libgomp.c-c++-common/taskwait-depend-nowait-2.c	2022-05-25 10:50:36.781833445 +0200
@@ -0,0 +1,48 @@ 
+#include <stdlib.h>
+#include <unistd.h>
+
+int
+main ()
+{
+  int a[48], b = 1;
+  #pragma omp parallel num_threads (4)
+  {
+    #pragma omp barrier
+    #pragma omp single
+    {
+      int i;
+      for (i = 0; i < 48; ++i)
+	#pragma omp task depend(in: a) shared(a)
+	  a[i] = i;
+      for (i = 0; i < 32; ++i)
+	{
+	  #pragma omp taskwait depend(inout: a) nowait
+	}
+      #pragma omp taskwait
+      for (i = 0; i < 48; ++i)
+	if (a[i] != i)
+	  abort ();
+      for (i = 0; i < 48; ++i)
+	#pragma omp task depend(in: a) shared(a)
+	  a[i] = 2 * i + 1;
+      #pragma omp taskgroup
+      {
+	#pragma omp taskwait depend(inoutset: a) nowait
+	#pragma omp taskgroup
+	{
+	  #pragma omp taskwait depend(inoutset: a) nowait
+	}
+      }
+      for (i = 0; i < 48; ++i)
+	if (a[i] != 2 * i + 1)
+	  abort ();
+      #pragma omp task depend(in: a) shared(a)
+      usleep (5000);
+      #pragma omp taskgroup
+      {
+	#pragma omp taskwait depend(inout: a) nowait
+      }
+    }
+  }
+  return 0;
+}