Re: [Qemu-devel] [PATCH v4 1/1] pci-host: add educational driver

[Jiri Slaby <jslaby@suse.cz>]

On 01/21/2015, 05:40 PM, Paolo Bonzini wrote:
> 
> 
> On 21/01/2015 17:28, Jiri Slaby wrote:
>> +        if (atomic_fetch_or(&edu->status, EDU_STATUS_COMPUTING) & EDU_STATUS_COMPUTING) {
> 
> Theoretically the other thread could see EDU_STATUS_COMPUTING here and
> not enter the condvar wait.  So...
> 
>> +            break;
>> +        }
>> +        qemu_mutex_lock(&edu->thr_mutex);
>> +        edu->fact = val;
>> +        qemu_cond_signal(&edu->thr_cond);
>> +        qemu_mutex_unlock(&edu->thr_mutex);
> 
> ... just one change:
> 
>    if (atomic_read(&edu->status) & EDU_STATUS_COMPUTING) {
>        break;
>    }
>    /* EDU_STATUS_COMPUTING cannot go 0->1 concurrently, because
>     * it is only set in this function and it is under the iothread
>     * mutex.
>     */
>    qemu_mutex_lock(&edu->thr_mutex);
>    edu->fact = val;
>    atomic_or(&edu->status), EDU_STATUS_COMPUTING);
>    qemu_cond_signal(&edu->thr_cond);
>    qemu_mutex_unlock(&edu->thr_mutex);
> 
> If you are okay with this change, I'll apply the patch to my tree.

Oh, I see. I will send v5 shortly as I found a bug in documentation too.

Thanks a lot for the review.

-- 
js
suse labs