This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: libgomp nvptx plugin: rework initialisation and support the proposed load/unload hooks (was: Merge current set of OpenACC changes from gomp-4_0-branch)
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Ilya Verbin <iverbin at gmail dot com>
- Cc: Julian Brown <julian at codesourcery dot com>, Thomas Schwinge <thomas at codesourcery dot com>, gcc-patches at gcc dot gnu dot org, Kirill Yukhin <kirill dot yukhin at gmail dot com>
- Date: Wed, 1 Apr 2015 15:20:25 +0200
- Subject: Re: libgomp nvptx plugin: rework initialisation and support the proposed load/unload hooks (was: Merge current set of OpenACC changes from gomp-4_0-branch)
- Authentication-results: sourceware.org; auth=none
- References: <20150323194439 dot GA12972 at msticlxl57 dot ims dot intel dot com> <20150326120919 dot GZ1746 at tucnak dot redhat dot com> <20150326204130 dot GA65474 at msticlxl57 dot ims dot intel dot com> <20150330164202 dot GH1746 at tucnak dot redhat dot com> <20150331125206 dot GC64930 at msticlxl57 dot ims dot intel dot com> <20150331130758 dot GA19273 at tucnak dot redhat dot com> <20150331161036 dot GA623 at msticlxl57 dot ims dot intel dot com> <20150331235328 dot GC623 at msticlxl57 dot ims dot intel dot com> <20150401052147 dot GG19273 at tucnak dot redhat dot com> <20150401131405 dot GD623 at msticlxl57 dot ims dot intel dot com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
On Wed, Apr 01, 2015 at 04:14:05PM +0300, Ilya Verbin wrote:
> On Wed, Apr 01, 2015 at 07:21:47 +0200, Jakub Jelinek wrote:
> > On Wed, Apr 01, 2015 at 02:53:28AM +0300, Ilya Verbin wrote:
> > > +/* Similar to gomp_fatal, but release mutexes before. */
> > > +
> > > +static void
> > > +gomp_fatal_unlock (const char *fmt, ...)
> > > +{
> > > + int i;
> > > + va_list list;
> > > +
> > > + for (i = 0; i < num_devices; i++)
> > > + gomp_mutex_unlock (&devices[i].lock);
> >
> > This is wrong. Calling gomp_mutex_unlock on a lock that you don't have
> > locked is undefined behavior.
> > You really should unlock it in the caller which should be aware which 0/1/2
> > locks it holds.
>
> I was worried about the following scenario:
> 1. Thread 1 in GOMP_target locks device 1.
> 2. Thread 2 in GOMP_target locks device 2 and calls gomp_fatal.
> 3. GOMP_offload_unregister will wait for device 1, even device 2 is unlocked.
How is that different from
1. Thread 1 in GOMP_target locks device 1.
2. Thread 2 calls exit.
? I mean when you unlock the device and register locks if you own them
before gomp_fatal.
> Anyway, it was a bad idea to unlock mutexes from non-owner thread.
>
> Here is patch, which unlocks proper mutexes in the caller, as you suggested.
> make check-target-libgomp passed.
LGTM with proper ChangeLog entry.
Jakub