[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] linux-user: call cpu_copy under clone_lock
From: |
Alex Bennée |
Subject: |
Re: [Qemu-devel] [PATCH] linux-user: call cpu_copy under clone_lock |
Date: |
Wed, 04 Apr 2018 11:27:39 +0100 |
User-agent: |
mu4e 1.1.0; emacs 26.0.91 |
Max Filippov <address@hidden> writes:
> Hi Alex,
>
> On Tue, Apr 3, 2018 at 9:26 AM, Alex Bennée <address@hidden> wrote:
>> Max Filippov <address@hidden> 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