Fix potential race condition in OpenACC "exit data" operations

	PR libgomp/92881

	libgomp/
	* libgomp.h (gomp_remove_var_async): Add prototype.
	* oacc-mem.c (delete_copyout): Call gomp_remove_var_async instead of
	gomp_remove_var.
	* target.c (gomp_unref_tgt): Change return type to bool, indicating
	whether target_mem_desc was unmapped.
	(gomp_unref_tgt_void): New.
	(gomp_remove_var): Reimplement in terms of...
	(gomp_remove_var_internal): ...this new helper function.
	(gomp_remove_var_async): New, implemented using above helper function.
	(gomp_unmap_vars_internal): Use gomp_unref_tgt_void instead of
	gomp_unref_tgt.

Reviewed-by: Thomas Schwinge <thomas@codesourcery.com>
From-SVN: r279388

diff --git a/libgomp/ChangeLog b/libgomp/ChangeLog
index ce440394c95c8f6df69628cee2332c9e433d7895..6b16bf34b17905491a883d762187c3dee531dc83 100644 (file)
--- a/libgomp/ChangeLog
+++ b/libgomp/ChangeLog
@@ -1,3 +1,19 @@
+2019-12-13  Julian Brown  <julian@codesourcery.com>
+
+       PR libgomp/92881
+
+       * libgomp.h (gomp_remove_var_async): Add prototype.
+       * oacc-mem.c (delete_copyout): Call gomp_remove_var_async instead of
+       gomp_remove_var.
+       * target.c (gomp_unref_tgt): Change return type to bool, indicating
+       whether target_mem_desc was unmapped.
+       (gomp_unref_tgt_void): New.
+       (gomp_remove_var): Reimplement in terms of...
+       (gomp_remove_var_internal): ...this new helper function.
+       (gomp_remove_var_async): New, implemented using above helper function.
+       (gomp_unmap_vars_internal): Use gomp_unref_tgt_void instead of
+       gomp_unref_tgt.
+
 2019-12-13  Andrew Stubbs  <ams@codesourcery.com>
 
        * testsuite/libgomp.oacc-c-c++-common/acc_prof-init-1.c: Handle gcn.

diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
index 9f4d0428871919746fe11e35049e5b3856ee514f..36dcca2835379ea43ffd0c103ac63dc1e96281e0 100644 (file)
--- a/libgomp/libgomp.h
+++ b/libgomp/libgomp.h
@@ -1166,6 +1166,8 @@ extern bool gomp_fini_device (struct gomp_device_descr *);
 extern void gomp_free_memmap (struct splay_tree_s *);
 extern void gomp_unload_device (struct gomp_device_descr *);
 extern bool gomp_remove_var (struct gomp_device_descr *, splay_tree_key);
+extern void gomp_remove_var_async (struct gomp_device_descr *, splay_tree_key,
+                                  struct goacc_asyncqueue *);
 
 /* work.c */
 

diff --git a/libgomp/oacc-mem.c b/libgomp/oacc-mem.c
index a809d0495a6725c9640431421b8484bed7810ca5..196b7e2a52027f6e2eaf86b2ac560255dfef0b38 100644 (file)
--- a/libgomp/oacc-mem.c
+++ b/libgomp/oacc-mem.c
@@ -660,7 +660,6 @@ static void
 delete_copyout (unsigned f, void *h, size_t s, int async, const char *libfnname)
 {
   splay_tree_key n;
-  void *d;
   struct goacc_thread *thr = goacc_thread ();
   struct gomp_device_descr *acc_dev = thr->dev;
 
@@ -689,9 +688,6 @@ delete_copyout (unsigned f, void *h, size_t s, int async, const char *libfnname)
       gomp_fatal ("[%p,%d] is not mapped", (void *)h, (int)s);
     }
 
-  d = (void *) (n->tgt->tgt_start + n->tgt_offset
-               + (uintptr_t) h - n->host_start);
-
   if ((uintptr_t) h < n->host_start || (uintptr_t) h + s > n->host_end)
     {
       size_t host_size = n->host_end - n->host_start;
@@ -723,12 +719,15 @@ delete_copyout (unsigned f, void *h, size_t s, int async, const char *libfnname)
 
   if (n->refcount == 0)
     {
+      goacc_aq aq = get_goacc_asyncqueue (async);
+
       if (f & FLAG_COPYOUT)
        {
-         goacc_aq aq = get_goacc_asyncqueue (async);
+         void *d = (void *) (n->tgt->tgt_start + n->tgt_offset
+                             + (uintptr_t) h - n->host_start);
          gomp_copy_dev2host (acc_dev, aq, h, d, s);
        }
-      gomp_remove_var (acc_dev, n);
+      gomp_remove_var_async (acc_dev, n, aq);
     }
 
   gomp_mutex_unlock (&acc_dev->lock);

diff --git a/libgomp/target.c b/libgomp/target.c
index 1151debf25665bf7b79d68ecd75a24045b5b686e..82ed38c01ec8d165383644fd1aa2a4906ebde16c 100644 (file)
--- a/libgomp/target.c
+++ b/libgomp/target.c
@@ -1116,32 +1116,63 @@ gomp_unmap_tgt (struct target_mem_desc *tgt)
   free (tgt);
 }
 
-attribute_hidden bool
-gomp_remove_var (struct gomp_device_descr *devicep, splay_tree_key k)
+static bool
+gomp_unref_tgt (void *ptr)
 {
   bool is_tgt_unmapped = false;
-  splay_tree_remove (&devicep->mem_map, k);
-  if (k->link_key)
-    splay_tree_insert (&devicep->mem_map, (splay_tree_node) k->link_key);
-  if (k->tgt->refcount > 1)
-    k->tgt->refcount--;
+
+  struct target_mem_desc *tgt = (struct target_mem_desc *) ptr;
+
+  if (tgt->refcount > 1)
+    tgt->refcount--;
   else
     {
+      gomp_unmap_tgt (tgt);
       is_tgt_unmapped = true;
-      gomp_unmap_tgt (k->tgt);
     }
+
   return is_tgt_unmapped;
 }
 
 static void
-gomp_unref_tgt (void *ptr)
+gomp_unref_tgt_void (void *ptr)
 {
-  struct target_mem_desc *tgt = (struct target_mem_desc *) ptr;
+  (void) gomp_unref_tgt (ptr);
+}
 
-  if (tgt->refcount > 1)
-    tgt->refcount--;
+static inline __attribute__((always_inline)) bool
+gomp_remove_var_internal (struct gomp_device_descr *devicep, splay_tree_key k,
+                         struct goacc_asyncqueue *aq)
+{
+  bool is_tgt_unmapped = false;
+  splay_tree_remove (&devicep->mem_map, k);
+  if (k->link_key)
+    splay_tree_insert (&devicep->mem_map, (splay_tree_node) k->link_key);
+  if (aq)
+    devicep->openacc.async.queue_callback_func (aq, gomp_unref_tgt_void,
+                                               (void *) k->tgt);
   else
-    gomp_unmap_tgt (tgt);
+    is_tgt_unmapped = gomp_unref_tgt ((void *) k->tgt);
+  return is_tgt_unmapped;
+}
+
+attribute_hidden bool
+gomp_remove_var (struct gomp_device_descr *devicep, splay_tree_key k)
+{
+  return gomp_remove_var_internal (devicep, k, NULL);
+}
+
+/* Remove a variable asynchronously.  This actually removes the variable
+   mapping immediately, but retains the linked target_mem_desc until the
+   asynchronous operation has completed (as it may still refer to target
+   memory).  The device lock must be held before entry, and remains locked on
+   exit.  */
+
+attribute_hidden void
+gomp_remove_var_async (struct gomp_device_descr *devicep, splay_tree_key k,
+                      struct goacc_asyncqueue *aq)
+{
+  (void) gomp_remove_var_internal (devicep, k, aq);
 }
 
 /* Unmap variables described by TGT.  If DO_COPYFROM is true, copy relevant
@@ -1197,7 +1228,7 @@ gomp_unmap_vars_internal (struct target_mem_desc *tgt, bool do_copyfrom,
     }
 
   if (aq)
-    devicep->openacc.async.queue_callback_func (aq, gomp_unref_tgt,
+    devicep->openacc.async.queue_callback_func (aq, gomp_unref_tgt_void,
                                                (void *) tgt);
   else
     gomp_unref_tgt ((void *) tgt);