From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:45968)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <jirislaby@gmail.com>) id 1YDyOR-0001k5-FB
	for qemu-devel@nongnu.org; Wed, 21 Jan 2015 11:44:36 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <jirislaby@gmail.com>) id 1YDyON-0004zL-Hi
	for qemu-devel@nongnu.org; Wed, 21 Jan 2015 11:44:35 -0500
Received: from mail-we0-x22e.google.com ([2a00:1450:400c:c03::22e]:61121)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <jirislaby@gmail.com>) id 1YDyON-0004z2-BC
	for qemu-devel@nongnu.org; Wed, 21 Jan 2015 11:44:31 -0500
Received: by mail-we0-f174.google.com with SMTP id x3so10649476wes.5
	for <qemu-devel@nongnu.org>; Wed, 21 Jan 2015 08:44:30 -0800 (PST)
Sender: Jiri Slaby <jirislaby@gmail.com>
Message-ID: <54BFD76C.80005@suse.cz>
Date: Wed, 21 Jan 2015 17:44:28 +0100
From: Jiri Slaby <jslaby@suse.cz>
MIME-Version: 1.0
References: <1421857693-12014-1-git-send-email-jslaby@suse.cz>
	<54BFD677.2070905@redhat.com>
In-Reply-To: <54BFD677.2070905@redhat.com>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v4 1/1] pci-host: add educational driver
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Paolo Bonzini <pbonzini@redhat.com>, qemu-devel@nongnu.org

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