Max Filippov <jcmvb...@gmail.com> writes: > Hi Alex, > > On Tue, Apr 3, 2018 at 9:26 AM, Alex Bennée <alex.ben...@linaro.org> wrote: >> Max Filippov <jcmvb...@gmail.com> writes: >> >>> cpu_copy adds newly created CPU object to container/machine/unattached, >>> but does it w/o proper locking. As a result when multiple threads are >>> created rapidly QEMU may abort with the following message: >>> >>> GLib-CRITICAL **: g_hash_table_iter_next: assertion >>> 'ri->version == ri->hash_table->version' failed >>> >>> ERROR:qemu/qom/object.c:1663:object_get_canonical_path_component: >>> code should not be reached >>> >>> Move cpu_copy invocation under clone_lock to fix that. >> >> So my main concern is are we duplicating something already (should be?) >> handled by fork_start/fork_end? > > clone_lock already exists, it protects state in case of thread creation, > it just didn't protect enough of it. > > The work done by fork_start/fork_end appears to be heavier than > what's needed for thread creation, because fork_start stops all > other CPUs (to make sure that child process won't get locks owned > by threads that no longer exist in the child process), which is not > required for thread creation, hence thread creation uses clone_lock.
I'm wondering if it should be doing more. After all start/end_exclusive rely on the cpu list and that isn't updated on thread creation - and without that a bunch of other things fail like ld/st exclusive after your first new thread is spawned. This really needs some test cases to check. So while I think clone_lock fixes this immediate problem I suspect there is more to do for this case. > >> This serialises forks and ensures things like the cpu_list (which ~ a >> thread list for linux-user) are updated safely. -- Alex Bennée
