From e38fdba42b1380d32845b3891a3e0deb4b653e1a Mon Sep 17 00:00:00 2001 From: Julian Brown Date: Wed, 6 May 2015 16:45:49 +0000 Subject: [PATCH] oacc-init.c (acc_shutdown_1): Call gomp_mutex_unlock for goacc_thread_lock on error paths. * oacc-init.c (acc_shutdown_1): Call gomp_mutex_unlock for goacc_thread_lock on error paths. * oacc-mem.c (lookup_host): Remove locking from function. Note locking requirement for caller in function comment. (lookup_dev): Likewise. (acc_free, acc_deviceptr, acc_hostptr, acc_is_present) (acc_map_data, acc_unmap_data, present_create_copy, delete_copyout) (update_dev_host, gomp_acc_insert_pointer, gomp_acc_remove_pointer): Add locking. From-SVN: r222860 --- libgomp/ChangeLog | 12 ++++ libgomp/oacc-init.c | 10 ++- libgomp/oacc-mem.c | 162 +++++++++++++++++++++++++++++++++----------- 3 files changed, 141 insertions(+), 43 deletions(-) diff --git a/libgomp/ChangeLog b/libgomp/ChangeLog index 15785ce1601..d943df20aa9 100644 --- a/libgomp/ChangeLog +++ b/libgomp/ChangeLog @@ -1,3 +1,15 @@ +2015-05-06 Julian Brown + + * oacc-init.c (acc_shutdown_1): Call gomp_mutex_unlock for + goacc_thread_lock on error paths. + * oacc-mem.c (lookup_host): Remove locking from function. Note + locking requirement for caller in function comment. + (lookup_dev): Likewise. + (acc_free, acc_deviceptr, acc_hostptr, acc_is_present) + (acc_map_data, acc_unmap_data, present_create_copy, delete_copyout) + (update_dev_host, gomp_acc_insert_pointer, gomp_acc_remove_pointer): + Add locking. + 2015-05-05 Thomas Schwinge PR testsuite/65205 diff --git a/libgomp/oacc-init.c b/libgomp/oacc-init.c index dc40fb6ffe1..10722980fe8 100644 --- a/libgomp/oacc-init.c +++ b/libgomp/oacc-init.c @@ -217,11 +217,17 @@ acc_shutdown_1 (acc_device_t d) /* This would mean the user is shutting down OpenACC in the middle of an "acc data" pragma. Likely not intentional. */ if (walk->mapped_data) - gomp_fatal ("shutdown in 'acc data' region"); + { + gomp_mutex_unlock (&goacc_thread_lock); + gomp_fatal ("shutdown in 'acc data' region"); + } /* Similarly, if this happens then user code has done something weird. */ if (walk->saved_bound_dev) - gomp_fatal ("shutdown during host fallback"); + { + gomp_mutex_unlock (&goacc_thread_lock); + gomp_fatal ("shutdown during host fallback"); + } if (walk->dev) { diff --git a/libgomp/oacc-mem.c b/libgomp/oacc-mem.c index 89ef5fcd887..90d43eb2b8a 100644 --- a/libgomp/oacc-mem.c +++ b/libgomp/oacc-mem.c @@ -35,7 +35,8 @@ #include #include -/* Return block containing [H->S), or NULL if not contained. */ +/* Return block containing [H->S), or NULL if not contained. The device lock + for DEV must be locked on entry, and remains locked on exit. */ static splay_tree_key lookup_host (struct gomp_device_descr *dev, void *h, size_t s) @@ -46,9 +47,7 @@ lookup_host (struct gomp_device_descr *dev, void *h, size_t s) node.host_start = (uintptr_t) h; node.host_end = (uintptr_t) h + s; - gomp_mutex_lock (&dev->lock); key = splay_tree_lookup (&dev->mem_map, &node); - gomp_mutex_unlock (&dev->lock); return key; } @@ -56,7 +55,8 @@ lookup_host (struct gomp_device_descr *dev, void *h, size_t s) /* Return block containing [D->S), or NULL if not contained. The list isn't ordered by device address, so we have to iterate over the whole array. This is not expected to be a common - operation. */ + operation. The device lock associated with TGT must be locked on entry, and + remains locked on exit. */ static splay_tree_key lookup_dev (struct target_mem_desc *tgt, void *d, size_t s) @@ -67,16 +67,12 @@ lookup_dev (struct target_mem_desc *tgt, void *d, size_t s) if (!tgt) return NULL; - gomp_mutex_lock (&tgt->device_descr->lock); - for (t = tgt; t != NULL; t = t->prev) { if (t->tgt_start <= (uintptr_t) d && t->tgt_end >= (uintptr_t) d + s) break; } - gomp_mutex_unlock (&tgt->device_descr->lock); - if (!t) return NULL; @@ -119,26 +115,35 @@ void acc_free (void *d) { splay_tree_key k; - struct goacc_thread *thr = goacc_thread (); if (!d) return; + struct goacc_thread *thr = goacc_thread (); + assert (thr && thr->dev); + struct gomp_device_descr *acc_dev = thr->dev; + + gomp_mutex_lock (&acc_dev->lock); + /* We don't have to call lazy open here, as the ptr value must have been returned by acc_malloc. It's not permitted to pass NULL in (unless you got that null from acc_malloc). */ - if ((k = lookup_dev (thr->dev->openacc.data_environ, d, 1))) - { - void *offset; + if ((k = lookup_dev (acc_dev->openacc.data_environ, d, 1))) + { + void *offset; - offset = d - k->tgt->tgt_start + k->tgt_offset; + offset = d - k->tgt->tgt_start + k->tgt_offset; - acc_unmap_data ((void *)(k->host_start + offset)); - } + gomp_mutex_unlock (&acc_dev->lock); - thr->dev->free_func (thr->dev->target_id, d); + acc_unmap_data ((void *)(k->host_start + offset)); + } + else + gomp_mutex_unlock (&acc_dev->lock); + + acc_dev->free_func (acc_dev->target_id, d); } void @@ -178,16 +183,24 @@ acc_deviceptr (void *h) goacc_lazy_initialize (); struct goacc_thread *thr = goacc_thread (); + struct gomp_device_descr *dev = thr->dev; + + gomp_mutex_lock (&dev->lock); - n = lookup_host (thr->dev, h, 1); + n = lookup_host (dev, h, 1); if (!n) - return NULL; + { + gomp_mutex_unlock (&dev->lock); + return NULL; + } offset = h - n->host_start; d = n->tgt->tgt_start + n->tgt_offset + offset; + gomp_mutex_unlock (&dev->lock); + return d; } @@ -204,16 +217,24 @@ acc_hostptr (void *d) goacc_lazy_initialize (); struct goacc_thread *thr = goacc_thread (); + struct gomp_device_descr *acc_dev = thr->dev; - n = lookup_dev (thr->dev->openacc.data_environ, d, 1); + gomp_mutex_lock (&acc_dev->lock); + + n = lookup_dev (acc_dev->openacc.data_environ, d, 1); if (!n) - return NULL; + { + gomp_mutex_unlock (&acc_dev->lock); + return NULL; + } offset = d - n->tgt->tgt_start + n->tgt_offset; h = n->host_start + offset; + gomp_mutex_unlock (&acc_dev->lock); + return h; } @@ -232,6 +253,8 @@ acc_is_present (void *h, size_t s) struct goacc_thread *thr = goacc_thread (); struct gomp_device_descr *acc_dev = thr->dev; + gomp_mutex_lock (&acc_dev->lock); + n = lookup_host (acc_dev, h, s); if (n && ((uintptr_t)h < n->host_start @@ -239,6 +262,8 @@ acc_is_present (void *h, size_t s) || s > n->host_end - n->host_start)) n = NULL; + gomp_mutex_unlock (&acc_dev->lock); + return n != NULL; } @@ -274,20 +299,32 @@ acc_map_data (void *h, void *d, size_t s) gomp_fatal ("[%p,+%d]->[%p,+%d] is a bad map", (void *)h, (int)s, (void *)d, (int)s); + gomp_mutex_lock (&acc_dev->lock); + if (lookup_host (acc_dev, h, s)) - gomp_fatal ("host address [%p, +%d] is already mapped", (void *)h, - (int)s); + { + gomp_mutex_unlock (&acc_dev->lock); + gomp_fatal ("host address [%p, +%d] is already mapped", (void *)h, + (int)s); + } if (lookup_dev (thr->dev->openacc.data_environ, d, s)) - gomp_fatal ("device address [%p, +%d] is already mapped", (void *)d, - (int)s); + { + gomp_mutex_unlock (&acc_dev->lock); + gomp_fatal ("device address [%p, +%d] is already mapped", (void *)d, + (int)s); + } + + gomp_mutex_unlock (&acc_dev->lock); tgt = gomp_map_vars (acc_dev, mapnum, &hostaddrs, &devaddrs, &sizes, &kinds, true, false); } + gomp_mutex_lock (&acc_dev->lock); tgt->prev = acc_dev->openacc.data_environ; acc_dev->openacc.data_environ = tgt; + gomp_mutex_unlock (&acc_dev->lock); } void @@ -299,17 +336,26 @@ acc_unmap_data (void *h) /* No need to call lazy open, as the address must have been mapped. */ size_t host_size; + + gomp_mutex_lock (&acc_dev->lock); + splay_tree_key n = lookup_host (acc_dev, h, 1); struct target_mem_desc *t; if (!n) - gomp_fatal ("%p is not a mapped block", (void *)h); + { + gomp_mutex_unlock (&acc_dev->lock); + gomp_fatal ("%p is not a mapped block", (void *)h); + } host_size = n->host_end - n->host_start; if (n->host_start != (uintptr_t) h) - gomp_fatal ("[%p,%d] surrounds1 %p", - (void *) n->host_start, (int) host_size, (void *) h); + { + gomp_mutex_unlock (&acc_dev->lock); + gomp_fatal ("[%p,%d] surrounds %p", + (void *) n->host_start, (int) host_size, (void *) h); + } t = n->tgt; @@ -323,8 +369,6 @@ acc_unmap_data (void *h) t->tgt_end = 0; t->to_free = 0; - gomp_mutex_lock (&acc_dev->lock); - for (tp = NULL, t = acc_dev->openacc.data_environ; t != NULL; tp = t, t = t->prev) if (n->tgt == t) @@ -336,10 +380,10 @@ acc_unmap_data (void *h) break; } - - gomp_mutex_unlock (&acc_dev->lock); } + gomp_mutex_unlock (&acc_dev->lock); + gomp_unmap_vars (t, true); } @@ -361,6 +405,8 @@ present_create_copy (unsigned f, void *h, size_t s) struct goacc_thread *thr = goacc_thread (); struct gomp_device_descr *acc_dev = thr->dev; + gomp_mutex_lock (&acc_dev->lock); + n = lookup_host (acc_dev, h, s); if (n) { @@ -368,13 +414,22 @@ present_create_copy (unsigned f, void *h, size_t s) d = (void *) (n->tgt->tgt_start + n->tgt_offset); if (!(f & FLAG_PRESENT)) - gomp_fatal ("[%p,+%d] already mapped to [%p,+%d]", - (void *)h, (int)s, (void *)d, (int)s); + { + gomp_mutex_unlock (&acc_dev->lock); + gomp_fatal ("[%p,+%d] already mapped to [%p,+%d]", + (void *)h, (int)s, (void *)d, (int)s); + } if ((h + s) > (void *)n->host_end) - gomp_fatal ("[%p,+%d] not mapped", (void *)h, (int)s); + { + gomp_mutex_unlock (&acc_dev->lock); + gomp_fatal ("[%p,+%d] not mapped", (void *)h, (int)s); + } + + gomp_mutex_unlock (&acc_dev->lock); } else if (!(f & FLAG_CREATE)) { + gomp_mutex_unlock (&acc_dev->lock); gomp_fatal ("[%p,+%d] not mapped", (void *)h, (int)s); } else @@ -389,6 +444,8 @@ present_create_copy (unsigned f, void *h, size_t s) else kinds = GOMP_MAP_ALLOC; + gomp_mutex_unlock (&acc_dev->lock); + tgt = gomp_map_vars (acc_dev, mapnum, &hostaddrs, NULL, &s, &kinds, true, false); @@ -439,21 +496,31 @@ delete_copyout (unsigned f, void *h, size_t s) struct goacc_thread *thr = goacc_thread (); struct gomp_device_descr *acc_dev = thr->dev; + gomp_mutex_lock (&acc_dev->lock); + n = lookup_host (acc_dev, h, s); /* No need to call lazy open, as the data must already have been mapped. */ if (!n) - gomp_fatal ("[%p,%d] is not mapped", (void *)h, (int)s); + { + gomp_mutex_unlock (&acc_dev->lock); + gomp_fatal ("[%p,%d] is not mapped", (void *)h, (int)s); + } d = (void *) (n->tgt->tgt_start + n->tgt_offset); host_size = n->host_end - n->host_start; if (n->host_start != (uintptr_t) h || host_size != s) - gomp_fatal ("[%p,%d] surrounds2 [%p,+%d]", - (void *) n->host_start, (int) host_size, (void *) h, (int) s); + { + gomp_mutex_unlock (&acc_dev->lock); + gomp_fatal ("[%p,%d] surrounds2 [%p,+%d]", + (void *) n->host_start, (int) host_size, (void *) h, (int) s); + } + + gomp_mutex_unlock (&acc_dev->lock); if (f & FLAG_COPYOUT) acc_dev->dev2host_func (acc_dev->target_id, h, d, s); @@ -482,16 +549,23 @@ update_dev_host (int is_dev, void *h, size_t s) struct goacc_thread *thr = goacc_thread (); struct gomp_device_descr *acc_dev = thr->dev; + gomp_mutex_lock (&acc_dev->lock); + n = lookup_host (acc_dev, h, s); /* No need to call lazy open, as the data must already have been mapped. */ if (!n) - gomp_fatal ("[%p,%d] is not mapped", h, (int)s); + { + gomp_mutex_unlock (&acc_dev->lock); + gomp_fatal ("[%p,%d] is not mapped", h, (int)s); + } d = (void *) (n->tgt->tgt_start + n->tgt_offset); + gomp_mutex_unlock (&acc_dev->lock); + if (is_dev) acc_dev->host2dev_func (acc_dev->target_id, d, h, s); else @@ -522,8 +596,11 @@ gomp_acc_insert_pointer (size_t mapnum, void **hostaddrs, size_t *sizes, tgt = gomp_map_vars (acc_dev, mapnum, hostaddrs, NULL, sizes, kinds, true, false); gomp_debug (0, " %s: mappings prepared\n", __FUNCTION__); + + gomp_mutex_lock (&acc_dev->lock); tgt->prev = acc_dev->openacc.data_environ; acc_dev->openacc.data_environ = tgt; + gomp_mutex_unlock (&acc_dev->lock); } void @@ -535,10 +612,15 @@ gomp_acc_remove_pointer (void *h, bool force_copyfrom, int async, int mapnum) struct target_mem_desc *t; int minrefs = (mapnum == 1) ? 2 : 3; + gomp_mutex_lock (&acc_dev->lock); + n = lookup_host (acc_dev, h, 1); if (!n) - gomp_fatal ("%p is not a mapped block", (void *)h); + { + gomp_mutex_unlock (&acc_dev->lock); + gomp_fatal ("%p is not a mapped block", (void *)h); + } gomp_debug (0, " %s: restore mappings\n", __FUNCTION__); @@ -546,8 +628,6 @@ gomp_acc_remove_pointer (void *h, bool force_copyfrom, int async, int mapnum) struct target_mem_desc *tp; - gomp_mutex_lock (&acc_dev->lock); - if (t->refcount == minrefs) { /* This is the last reference, so pull the descriptor off the -- 2.30.2