[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH V7 01/19] cpus: protect queued_work_* with w
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [RFC PATCH V7 01/19] cpus: protect queued_work_* with work_mutex. |
Date: |
Mon, 10 Aug 2015 18:06:02 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 |
On 10/08/2015 18:04, Frederic Konrad wrote:
> On 10/08/2015 17:59, Paolo Bonzini wrote:
>>
>> On 10/08/2015 17:26, address@hidden wrote:
>>> + qemu_mutex_lock(&cpu->work_mutex);
>>> while ((wi = cpu->queued_work_first)) {
>>> cpu->queued_work_first = wi->next;
>>> + qemu_mutex_unlock(&cpu->work_mutex);
>>> wi->func(wi->data);
>>> + qemu_mutex_lock(&cpu->work_mutex);
>>> wi->done = true;
>> This should be atomic_mb_set
>
> Isn't that protected by the mutex?
This use is not protected by the mutex:
@@ -853,6 +855,7 @@ void run_on_cpu(CPUState *cpu, void (*func)(void *data),
void *data)
cpu->queued_work_last = &wi;
wi.next = NULL;
wi.done = false;
+ qemu_mutex_unlock(&cpu->work_mutex);
qemu_cpu_kick(cpu);
while (!wi.done) {
Paolo
Or maybe it's used somewhere else?
>>
>>> if (wi->free) {
>>> g_free(wi);
>>> }
>>> }
>>> cpu->queued_work_last = NULL;
>> ... and I'm a bit afraid of leaving the state of the list inconsistent,
>> so I'd move this after the cpu->queued_work_first assignment. Otherwise
>> the patch looks good, I'm queuing it for 2.5.
>>
>> Paolo
>>
>>> + qemu_mutex_unlock(&cpu->work_mutex);
>>> +
>
[Qemu-devel] [RFC PATCH V7 03/19] cpus: introduce async_run_safe_work_on_cpu., fred . konrad, 2015/08/10
[Qemu-devel] [RFC PATCH V7 05/19] remove unused spinlock., fred . konrad, 2015/08/10
[Qemu-devel] [RFC PATCH V7 06/19] add support for spin lock on POSIX systems exclusively, fred . konrad, 2015/08/10
[Qemu-devel] [RFC PATCH V7 04/19] replace spinlock by QemuMutex., fred . konrad, 2015/08/10