From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=54502 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1OPC2y-0004Rh-KX
	for qemu-devel@nongnu.org; Thu, 17 Jun 2010 06:10:09 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69)
	(envelope-from <pbonzini@redhat.com>) id 1OPC2x-0007E6-FH
	for qemu-devel@nongnu.org; Thu, 17 Jun 2010 06:10:08 -0400
Received: from mx1.redhat.com ([209.132.183.28]:12406)
	by eggs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <pbonzini@redhat.com>) id 1OPC2x-0007Dh-3v
	for qemu-devel@nongnu.org; Thu, 17 Jun 2010 06:10:07 -0400
Message-ID: <4C19F477.6080709@redhat.com>
Date: Thu, 17 Jun 2010 12:09:59 +0200
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <20100616115404.10988.62371.stgit@localhost.localdomain>
	<20100616115656.10988.96529.stgit@localhost.localdomain>
	<4C18C4C8.8090901@redhat.com> <20100617085325.GA2849@in.ibm.com>
In-Reply-To: <20100617085325.GA2849@in.ibm.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: [Qemu-devel] Re: [PATCH V4 2/3] qemu: Generic task offloading
	framework:	threadlets
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/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: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: ego@in.ibm.com
Cc: Anthony Liguori <aliguori@linux.vnet.ibm.com>, Avi Kivity <avi@redhat.com>, Qemu-development List <qemu-devel@nongnu.org>, Corentin Chary <corentin.chary@gmail.com>, "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>

>>> +        while (QTAILQ_EMPTY(&(queue->request_list))&&
>>> +               (ret != ETIMEDOUT)) {
>>> +            ret = qemu_cond_timedwait(&(queue->cond),
>>> +					&(queue->lock), 10*100000);
>>> +        }
>>
>> Using qemu_cond_timedwait is a hack for not properly broadcasting the
>> condvar in flush_threadlet_queue.
>
> I think Anthony answered this one.

I think he said that the code has been changed so I am right? :)

>>> +/**
>>> + * flush_threadlet_queue: Wait till completion of all the submitted tasks
>>> + * @queue: Queue containing the tasks we're waiting on.
>>> + */
>>> +void flush_threadlet_queue(ThreadletQueue *queue)
>>> +{
>>> +    qemu_mutex_lock(&queue->lock);
>>> +    queue->exit = 1;
>>> +
>>> +    qemu_barrier_init(&queue->barr, queue->cur_threads + 1);
>>> +    qemu_mutex_unlock(&queue->lock);
>>> +
>>> +    qemu_barrier_wait(&queue->barr);
>>
>> Can be implemented just as well with queue->cond and a loop waiting for
>> queue->cur_threads == 0.  This would remove the need to implement barriers
>> in qemu-threads (especially for Win32).  Anyway whoever will contribute
>> Win32 qemu-threads can do it, since it's not hard.
>
> That was the other option I had considered before going for barriers,
> for no particular reason. Now, considering that barriers are not
> welcome, I will implement this method.

I guess we decided flush isn't really useful at all.  Might as well 
leave it out of v5 and implement it later, so the barrier and 
complicated exit condition are now unnecessary.

Thanks,

Paolo