Re: [Qemu-devel] [PATCH 1/3] Introduce threadlets

[Anthony Liguori <anthony@codemonkey.ws>]

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