Re: [Qemu-devel] [PATCH] cpus: ignore ESRCH in qemu_cpu_kick_thread()

[Paolo Bonzini]

On 15/01/19 17:34, Laurent Vivier wrote:
> On 08/01/2019 19:47, Laurent Vivier wrote:
>> On 08/01/2019 00:02, Paolo Bonzini wrote:
>>> On 02/01/19 15:16, Laurent Vivier wrote:
>>>> We can have a race condition between qemu_cpu_kick_thread() and
>>>> qemu_kvm_cpu_thread_fn() when we hotunplug a CPU. In this case,
>>>> qemu_cpu_kick_thread() can try to kick a thread that is exiting.
>>>> pthread_kill() returns an error and qemu is stopped by an exit(1).
>>>>
>>>>     qemu:qemu_cpu_kick_thread: No such process
>>>>
>>>> We can ignore safely this error.
>>>>
>>>> Signed-off-by: Laurent Vivier <address@hidden>
>>>> ---
>>>>   cpus.c | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/cpus.c b/cpus.c
>>>> index 0ddeeefc14..4717490bd0 100644
>>>> --- a/cpus.c
>>>> +++ b/cpus.c
>>>> @@ -1778,7 +1778,7 @@ static void qemu_cpu_kick_thread(CPUState *cpu)
>>>>       }
>>>>       cpu->thread_kicked = true;
>>>>       err = pthread_kill(cpu->thread->thread, SIG_IPI);
>>>> -    if (err) {
>>>> +    if (err && err != ESRCH) {
>>>>           fprintf(stderr, "qemu:%s: %s", __func__, strerror(err));
>>>>           exit(1);
>>>>       }
>>>>
>>>
>>> You could in principle be sending the signal to another thread, so the
>>> fix is a bit hackish.  However, I don't have a better idea that is not
>>> racy. :(
>>>
>>> The problem is that qemu_cpu_kick does not use any spinlock or mutex to
>>> synchronize against cpu_remove_sync's qemu_thread_join.  I think once
>>> the you reach qemu_cpu_kick in cpu_remove_sync (so if cpu->unplug) you
>>> do not need to reset cpu->thread_kicked anymore, but I don't think
>>> that's enough to fix it.
>>
>> Will you take the patch through one of your pull requests or should I
>> add it to the trivial-patches branch?
> 
> Paolo?

I queued it now, thanks.

Paolo