From 1cbd94e834d58100579847d35e899768c384dae0 Mon Sep 17 00:00:00 2001 From: Julian Brown 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 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 ce440394c95..6b16bf34b17 100644 --- a/libgomp/ChangeLog +++ b/libgomp/ChangeLog @@ -1,3 +1,19 @@ +2019-12-13 Julian Brown + + 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 * testsuite/libgomp.oacc-c-c++-common/acc_prof-init-1.c: Handle gcn. diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h index 9f4d0428871..36dcca28353 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 a809d0495a6..196b7e2a520 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 1151debf256..82ed38c01ec 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); -- 2.30.2