From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17163 invoked by alias); 24 Apr 2015 17:13:29 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 17149 invoked by uid 89); 24 Apr 2015 17:13:28 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 24 Apr 2015 17:13:25 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=SVR-IES-FEM-01.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1YlhAG-0004Ye-IO from Julian_Brown@mentor.com ; Fri, 24 Apr 2015 10:13:21 -0700 Received: from octopus (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.3.224.2; Fri, 24 Apr 2015 18:13:18 +0100 Date: Fri, 24 Apr 2015 17:13:00 -0000 From: Julian Brown To: Thomas Schwinge CC: , Jakub Jelinek Subject: Re: [PATCH] Tidy up locking for libgomp OpenACC entry points Message-ID: <20150424181308.099d8a26@octopus> In-Reply-To: <87egnavltt.fsf@schwinge.name> References: <20150422194243.115f26fa@octopus> <87egnavltt.fsf@schwinge.name> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="MP_/RH7NNEkuVzLEJhM./8rbYUI" X-IsSubscribed: yes X-SW-Source: 2015-04/txt/msg01519.txt.bz2 --MP_/RH7NNEkuVzLEJhM./8rbYUI Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline Content-length: 6651 On Thu, 23 Apr 2015 18:41:34 +0200 Thomas Schwinge wrote: > Hi! >=20 > On Wed, 22 Apr 2015 19:42:43 +0100, Julian Brown > wrote: > > This patch is an attempt to fix some potential race conditions with > > accesses to shared data structures from multiple concurrent threads > > in libgomp's OpenACC entry points. The main change is to move > > locking out of lookup_host and lookup_dev in oacc-mem.c and into > > their callers (which can then hold the locks for the whole > > operation that they are performing). >=20 > Yeah, that makes sense I guess. >=20 > > Also missing locking has been added for gomp_acc_insert_pointer. > >=20 > > Tests look OK (with offloading to NVidia PTX). >=20 > How did you test to get some confidence in the locking being > sufficient? Merely by running the existing tests and via inspection, sadly. I'm not sure how much value we'd get from implementing an exhaustive threading testsuite at this stage: I guess testcases will be easier to come by in the future if/when people start to use e.g. OpenMP and OpenACC together. > Going further (separate patch?), a few more comments: >=20 > Is it OK that oacc-init.c:cached_base_dev is accessed without locking? >=20 > Generally, we have to keep in mind that the same device may be > accessed in parallel through both OpenACC and OpenMP interfaces. For > this, for example, in oacc-init.c, even though acc_device_lock is > held, is it OK to call gomp_init_device(D) without D->lock being > locked? (Compare to target.c code.) >=20 > Please document what exactly oacc-init.c:acc_device_lock is to guard. > I'm not sure I'm understanding this correctly. I've attached a follow-on patch that documents the purpose of acc_device_lock -- and also fixes some places that should have been holding the lock, but were not. I've also added locking (with dev->lock) when calling gomp_init_device and gomp_fini_device from the OpenACC initialisation/finalisation code. > Should oacc-init.c:acc_shutdown_1 release goacc_thread_lock before any > gomp_fatal calls? (That seems to be the general policy in libgomp.) I added this to the first patch. > > --- a/libgomp/oacc-mem.c > > +++ b/libgomp/oacc-mem.c >=20 > > @@ -120,25 +116,32 @@ acc_free (void *d) > > { > > splay_tree_key k; > > struct goacc_thread *thr =3D goacc_thread (); > > + struct gomp_device_descr *acc_dev =3D thr->dev; > >=20=20 > > if (!d) > > return; > >=20=20 > > assert (thr && thr->dev); > >=20=20 > > + 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 =3D lookup_dev (thr->dev->openacc.data_environ, d, 1))) > > - { > > - void *offset; > > + if ((k =3D lookup_dev (acc_dev->openacc.data_environ, d, 1))) > > + { > > + void *offset; > > + > > + offset =3D d - k->tgt->tgt_start + k->tgt_offset; > >=20=20 > > - offset =3D d - k->tgt->tgt_start + k->tgt_offset; > > + gomp_mutex_unlock (&acc_dev->lock); > >=20=20 > > - acc_unmap_data ((void *)(k->host_start + offset)); > > - } > > + acc_unmap_data ((void *)(k->host_start + offset)); > > + } > > + else > > + gomp_mutex_unlock (&acc_dev->lock); >=20 > Does it make sense to make the unlock unconditional, and move the > acc_unmap_data after it, guarded by =C2=BBif (k)=C2=AB? I've left this one -- just a stylistic tweak, but I think it's fine as-is. > > - thr->dev->free_func (thr->dev->target_id, d); > > + acc_dev->free_func (acc_dev->target_id, d); > > } > >=20=20 > > void > > @@ -178,16 +181,24 @@ acc_deviceptr (void *h) > > goacc_lazy_initialize (); > >=20=20 > > struct goacc_thread *thr =3D goacc_thread (); > > + struct gomp_device_descr *dev =3D thr->dev; > > + > > + gomp_mutex_lock (&dev->lock); > >=20=20 > > - n =3D lookup_host (thr->dev, h, 1); > > + n =3D lookup_host (dev, h, 1); > >=20=20 > > if (!n) > > - return NULL; > > + { > > + gomp_mutex_unlock (&dev->lock); > > + return NULL; > > + } > >=20=20 > > offset =3D h - n->host_start; > >=20=20 > > d =3D n->tgt->tgt_start + n->tgt_offset + offset; > >=20=20 > > + gomp_mutex_unlock (&dev->lock); > > + > > return d; > > } >=20 > Do we need to retain the lock while working with n? If not, the > unlock could be placed right after the lookup_host, unconditionally. > I'm confused -- it's commonly being done (retained) in target.c code, > but not in the tgt_fn lookup in target.c:GOMP_target. I think the difference can be explained as follows: a given mapping (splay_key_tree_s) is essentially immutable after it is created (apart from the refcounts). Thus it can be safely accessed *so long as we know it will not be deallocated*. Now, in some parts of target.c, we have an "active" target_mem_desc, corresponding to a set of host-target mappings. So long as we are holding that target_mem_desc (e.g. as we are in GOMP_target_data), we know that none of the associated mappings' refcounts will fall to zero: so, we can access them (read only) safely without explicitly holding the lock. But, that's *not* the case for e.g. acc_deviceptr: that can be called at any point, in particular partway through deallocation of a set of mappings. So we need the lock for that. I didn't write the code of course, so E&OE :-). > Should external functions (dev2host_func) be called without any locks > held (to avoid issues with any call-backs); so move the unlocking > before that? I fixed this in the first patch. Re-tested (libgomp), results look OK. OK to apply now? Julian ChangeLog libgomp/ * 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. ChangeLog libgomp/ * oacc-init.c (acc_device_lock): Add explanatory comment. (resolve_device): Add comment about locking requirement. (acc_init_1, acc_shutdown_1): Likewise. Add locking around gomp_init_device and gomp_fini_device calls. (acc_get_num_devices, acc_set_device_type, acc_get_device_type) (acc_get_device_num, acc_set_device_num): Add locking around resolve_device and gomp_init_device calls. --MP_/RH7NNEkuVzLEJhM./8rbYUI Content-Type: text/x-patch Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="acc-device-lock-tweaks-1.diff" Content-length: 3703 commit 5009a4a24f15150976a9ad717f81c3436a082cea Author: Julian Brown Date: Fri Apr 24 03:49:14 2015 -0700 Fix usage of acc_device_lock diff --git a/libgomp/oacc-init.c b/libgomp/oacc-init.c index 9d47a23..503f8b8 100644 --- a/libgomp/oacc-init.c +++ b/libgomp/oacc-init.c @@ -36,6 +36,9 @@ #include #include +/* This lock is used to protect access to cached_base_dev, dispatchers and + the (abstract) initialisation state of attached offloading devices. */ + static gomp_mutex_t acc_device_lock; /* A cached version of the dispatcher for the global "current" accelerator type, @@ -106,6 +109,8 @@ name_of_acc_device_t (enum acc_device_t type) } } +/* ACC_DEVICE_LOCK should be held before calling this function. */ + static struct gomp_device_descr * resolve_device (acc_device_t d) { @@ -166,7 +171,8 @@ resolve_device (acc_device_t d) /* This is called when plugins have been initialized, and serves to call (indirectly) the target's device_init hook. Calling multiple times without - an intervening acc_shutdown_1 call is an error. */ + an intervening acc_shutdown_1 call is an error. ACC_DEVICE_LOCK should be + held before calling this function. */ static struct gomp_device_descr * acc_init_1 (acc_device_t d) @@ -183,14 +189,21 @@ acc_init_1 (acc_device_t d) acc_dev = &base_dev[goacc_device_num]; + gomp_mutex_lock (&acc_dev->lock); if (acc_dev->is_initialized) - gomp_fatal ("device already active"); + { + gomp_mutex_unlock (&acc_dev->lock); + gomp_fatal ("device already active"); + } gomp_init_device (acc_dev); + gomp_mutex_unlock (&acc_dev->lock); return base_dev; } +/* ACC_DEVICE_LOCK should be held before calling this function. */ + static void acc_shutdown_1 (acc_device_t d) { @@ -249,11 +262,13 @@ acc_shutdown_1 (acc_device_t d) for (i = 0; i < ndevs; i++) { struct gomp_device_descr *acc_dev = &base_dev[i]; + gomp_mutex_lock (&acc_dev->lock); if (acc_dev->is_initialized) { devices_active = true; gomp_fini_device (acc_dev); } + gomp_mutex_unlock (&acc_dev->lock); } if (!devices_active) @@ -410,7 +425,10 @@ acc_get_num_devices (acc_device_t d) gomp_init_targets_once (); + gomp_mutex_lock (&acc_device_lock); acc_dev = resolve_device (d); + gomp_mutex_unlock (&acc_device_lock); + if (!acc_dev) return 0; @@ -441,8 +459,10 @@ acc_set_device_type (acc_device_t d) cached_base_dev = base_dev = resolve_device (d); acc_dev = &base_dev[goacc_device_num]; + gomp_mutex_lock (&acc_dev->lock); if (!acc_dev->is_initialized) gomp_init_device (acc_dev); + gomp_mutex_unlock (&acc_dev->lock); gomp_mutex_unlock (&acc_device_lock); @@ -473,7 +493,9 @@ acc_get_device_type (void) { gomp_init_targets_once (); + gomp_mutex_lock (&acc_device_lock); dev = resolve_device (acc_device_default); + gomp_mutex_unlock (&acc_device_lock); res = acc_device_type (dev->type); } @@ -497,7 +519,9 @@ acc_get_device_num (acc_device_t d) if (!cached_base_dev) gomp_init_targets_once (); + gomp_mutex_lock (&acc_device_lock); dev = resolve_device (d); + gomp_mutex_unlock (&acc_device_lock); if (!dev) gomp_fatal ("device %s not supported", name_of_acc_device_t (d)); @@ -539,8 +563,10 @@ acc_set_device_num (int ord, acc_device_t d) acc_dev = &base_dev[ord]; + gomp_mutex_lock (&acc_dev->lock); if (!acc_dev->is_initialized) gomp_init_device (acc_dev); + gomp_mutex_unlock (&acc_dev->lock); gomp_mutex_unlock (&acc_device_lock); --MP_/RH7NNEkuVzLEJhM./8rbYUI Content-Type: text/x-patch Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="more-locking-4.diff" Content-length: 12124 commit 8ae37f2c789e3a61fe4c62101fc13ea56cc1ee9f Author: Julian Brown Date: Tue Apr 21 12:42:17 2015 -0700 More locking in oacc-mem.c diff --git a/libgomp/oacc-init.c b/libgomp/oacc-init.c index 03db8ee..9d47a23 100644 --- a/libgomp/oacc-init.c +++ b/libgomp/oacc-init.c @@ -218,11 +218,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 89ef5fc..90d43eb 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 --MP_/RH7NNEkuVzLEJhM./8rbYUI--