On 06/08/2010 06:30 PM, Johannes Sixt wrote:
Am 07.06.2010 15:38, schrieb Paolo Bonzini:@@ -172,9 +172,10 @@ int pthread_cond_broadcast(pthread_cond_t *cond) * As in pthread_cond_signal, access to cond->waiters and * cond->was_broadcast is locked via the external mutex. */ - - if ((cond->was_broadcast = cond->waiters> 0)) { + if (cond->waiters> 0) { BOOLEAN result; + cond->was_broadcast = cond->waiters> 1; +It is possible that you set was_broadcast to 1 here, while another thread still sees was_broadcast == 0 in cond_wait.
That still cannot happen, because pthread_cond_wait will be locked on the semaphore until the ReleaseSemaphore. The only race that exists is between broadcast/signal's ReleaseSemaphore and wait's WaitForSingleObject. This is benign, and exists before my patch. But in all cases the code before ReleaseSemaphore is serialized WRT to the code after wait's WaitForSingleObject.
That said, as long as this series buys performance only at the expense of clarity, I'm rather opposed to it because we do not call cond_wait and cond_broadcast in time-critical paths.
Yes, it is less clear indeed. I tried to compensate with comments but that was not enough apparently. As I said I did this patch for another project where condvars are used in time-critical paths; if you do not want to keep it, that's not a problem.
Paolo -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html