From 1cbd94e834d58100579847d35e899768c384dae0 Mon Sep 17 00:00:00 2001
From: Julian Brown <julian@codesourcery.com>
Date: Fri, 13 Dec 2019 23:14:15 +0000
Subject: [PATCH] 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
---
libgomp/ChangeLog | 16 +++++++++++++
libgomp/libgomp.h | 2 ++
libgomp/oacc-mem.c | 11 ++++-----
libgomp/target.c | 59 +++++++++++++++++++++++++++++++++++-----------
4 files changed, 68 insertions(+), 20 deletions(-)
diff --git a/libgomp/ChangeLog b/libgomp/ChangeLog
index ce440394c95c..6b16bf34b179 100644
--- 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 9f4d04288719..36dcca283537 100644
--- 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 a809d0495a67..196b7e2a5202 100644
--- 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 1151debf2566..82ed38c01ec8 100644
--- 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);
--
GitLab