This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: libgomp: Guard all offload_images/num_offload_images access by register_lock (was: libgomp nvptx plugin: rework initialisation and support the proposed load/unload hooks)
- From: Ilya Verbin <iverbin at gmail dot com>
- To: Thomas Schwinge <thomas at codesourcery dot com>
- Cc: Jakub Jelinek <jakub at redhat dot com>, Julian Brown <julian at codesourcery dot com>, gcc-patches at gcc dot gnu dot org, Kirill Yukhin <kirill dot yukhin at gmail dot com>
- Date: Fri, 25 Sep 2015 19:49:50 +0300
- Subject: Re: libgomp: Guard all offload_images/num_offload_images access by register_lock (was: libgomp nvptx plugin: rework initialisation and support the proposed load/unload hooks)
- Authentication-results: sourceware.org; auth=none
- References: <20150204150545 dot 2628ad35 at octopus> <20150224112951 dot 363b71fd at octopus> <87a902ib93 dot fsf at schwinge dot name> <20150226172511 dot GA49293 at msticlxl57 dot ims dot intel dot com> <20150306140113 dot GB26588 at msticlxl57 dot ims dot intel dot com> <20150309144555 dot 3a078f48 at octopus> <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> <877fnewjug dot fsf at kepler dot schwinge dot homeip dot net>
On Fri, Sep 25, 2015 at 18:21:27 +0200, Thomas Schwinge wrote:
> On Thu, 26 Mar 2015 23:41:30 +0300, Ilya Verbin <iverbin@gmail.com> wrote:
> > On Thu, Mar 26, 2015 at 13:09:19 +0100, Jakub Jelinek wrote:
> > > the current code is majorly broken. As I've said earlier, e.g. the lack
> > > of mutex guarding gomp_target_init (which is using pthread_once guaranteed
> > > to be run just once) vs. concurrent GOMP_offload_register calls
> > > (if those are run from ctors, then I guess something like dl_load_lock
> > > ensures at least on glibc that multiple GOMP_offload_register calls aren't
> > > performed at the same time) in accessing/reallocating offload_images
> > > and num_offload_images and the lack of support to register further
> > > images after the gomp_target_init call (if you dlopen further shared
> > > libraries) is really bad. And it would be really nice to support the
> > > unloading.
>
> > Here is the latest patch for libgomp and mic plugin.
>
> What about the scenario where one thread is inside
> GOMP_offload_register_ver/GOMP_offload_register (say, due to opening a
> shared library with such a mkoffload-generated constructor) and is
> modifying offload_images with register_lock held, and another thread is
> inside a GOMP_target* construct -> gomp_init_device and is accessing
> offload_images without register_lock held? Or, why isn't that a
> reachable scenario?
>
> Would the following patch (untested) do the right thing (locking added to
> gomp_init_device and gomp_unload_device)? We can then also remove the
> is_register_lock parameter from gomp_load_image_to_device, and simplify
> the code.
Looks like you're right, and this scenario is possible.
-- Ilya