On Fri, Sep 25, 2015 at 18:21:27 +0200, Thomas Schwinge wrote: > On Thu, 26 Mar 2015 23:41:30 +0300, Ilya Verbin <iver...@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