From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1MjAAw-0000yV-Rd
	for qemu-devel@nongnu.org; Thu, 03 Sep 2009 07:08:23 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1MjAAr-0000ty-J8
	for qemu-devel@nongnu.org; Thu, 03 Sep 2009 07:08:21 -0400
Received: from [199.232.76.173] (port=60341 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1MjAAr-0000tn-58
	for qemu-devel@nongnu.org; Thu, 03 Sep 2009 07:08:17 -0400
Date: Thu, 3 Sep 2009 08:07:50 -0300
From: Glauber Costa <glommer@redhat.com>
Message-ID: <20090903110750.GN30340@mothafucka.localdomain>
References: <1251939158-17153-1-git-send-email-glommer@redhat.com>
	<4A9F71E9.3090900@gnu.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4A9F71E9.3090900@gnu.org>
Subject: [Qemu-devel] Re: [RFC] queue_work proposal
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.gnu.org/pipermail/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: Paolo Bonzini <bonzini@gnu.org>
Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org

On Thu, Sep 03, 2009 at 09:36:09AM +0200, Paolo Bonzini wrote:
>
>> +    env->queued_total++;
>> +
>> +    if (env == qemu_get_current_env()) {
>> +        env->queued_total++;
>
> Why increment twice? (though queued_total is write only and queued_local  
> is unused, so...)
yeah, you got it =p

As I said, I just dumped whatever I had.

>
>> +        func(data);
>> +        return;
>> +    }
>> +
>> +    wii = qemu_mallocz(sizeof(*wii));
>> +    wii->func = func;
>> +    wii->data = data;
>> +    wii->wait = wait;
>> +    TAILQ_INSERT_TAIL(&env->queued_work, wii, entry);
>> +
>> +    qemu_thread_signal(env->thread, SIGUSR1);
>> +
>> +    while (wait&&  !wii->done) {
>> +        qemu_cond_wait(env->work_cond,&qemu_global_mutex);
>> +    }
>
> You need to lock qemu_global_mutex around this while statement, or to  
> add env->queue_mutex and include the TAILQ_INSERT_TAIL in the mutex.
Thanks for catching. The later is clearly preferred , IMHO, for scalability
purposes.