From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=56587 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1P8YTd-0008NV-Rb
	for qemu-devel@nongnu.org; Wed, 20 Oct 2010 09:13:10 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <anthony@codemonkey.ws>) id 1P8YTb-0001o0-3w
	for qemu-devel@nongnu.org; Wed, 20 Oct 2010 09:13:09 -0400
Received: from mail-qw0-f45.google.com ([209.85.216.45]:36205)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <anthony@codemonkey.ws>) id 1P8YTb-0001nq-1q
	for qemu-devel@nongnu.org; Wed, 20 Oct 2010 09:13:07 -0400
Received: by qwh5 with SMTP id 5so2437677qwh.4
	for <qemu-devel@nongnu.org>; Wed, 20 Oct 2010 06:13:06 -0700 (PDT)
Message-ID: <4CBEEADF.3050801@codemonkey.ws>
Date: Wed, 20 Oct 2010 08:13:03 -0500
From: Anthony Liguori <anthony@codemonkey.ws>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [PATCH 1/3] Introduce threadlets
References: <20101019173946.16514.62027.stgit@localhost6.localdomain6>
	<20101019174245.16514.14542.stgit@localhost6.localdomain6>
	<20101019183644.GI15844@balbir.in.ibm.com>
	<4CBE0F5F.3020204@codemonkey.ws>
	<20101020022217.GL15844@balbir.in.ibm.com>
In-Reply-To: <20101020022217.GL15844@balbir.in.ibm.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
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: balbir@linux.vnet.ibm.com
Cc: Arun R Bharadwaj <arun@linux.vnet.ibm.com>, qemu-devel@nongnu.org

On 10/19/2010 09:22 PM, Balbir Singh wrote:
>
> OK, here is a situation that can happen
>
> T1                              T2
> ---                             ---
> threadlet                       submit_threadletwork_to_queue
> (sees condition as no work)     mutex_lock
> qemu_cond_timedwait             add_work
> ...                             mutex_unlock
>
> T3
> --
> cancel_threadlet_work_on_queue
> mutex_lock (grabs it) before T1 can
> cancels the work
>
>
>                                  qemu_cond_signal
>
> T1
> --
> Grabs mutex_lock (from within cond_timedwait)
> Now there is no work to do, the condition
> has changed before the thread wakes up
>
>
> The man page also states
>
> "however, if predictable scheduling behavior is required, then that
> mutex shall be locked by the thread calling pthread_cond_broadcast()
> or pthread_cond_signal()"
>    

The scenario you're describing is a spurious wakeup.  Any code that uses 
conditions ought to handle spurious wakeups.  The typical idiom for this is:

while (no_work_available()) {
    pthread_cond_wait(cond, lock);
}

So yes, pthread_cond_timedwait() will return but the while loop 
condition will be checked first.  In the scenario you describe, we'll go 
immediately back to sleep and the assert will not be triggered.

As I mentioned originally, in the absence of performance data, code 
readability trumps premature optimization.  I think the code is a lot 
more readable if the signaling point is outside of the mutex.

Regards,

Anthony Liguori