On Thu, Dec 20, 2012 at 02:03:06PM +0100, Joerg-Cyril.Hoehle@t-systems.com wrote:
what's up with this patch?
I wrote:
SetEvent is one of those calls with a high probability that the Linux scheduler will switch to another thread, e.g. the one receiving the event. As the critical section is still hold, it'll prevent the receiver from invoking mmdevapi, e.g. GetCurrentPadding, without two more thread switches (forth and back). We don't need that.
Now that This->event is constant between IAC_Start and IAC_Release, thanks to my "SetEventHandle is allowed once only" patch, it's safe to move SetEvent(This->event) outside of the CS.
Unlike the rejected patch about a CreateTimerQueue race condition internal to Wine (and only relevant in lock-less mode), this one is between the driver and the client app that blocks when calling e.g. GetCurrentPadding, what almost all apps will do, including our DSound http://source.winehq.org/source/dlls/dsound/mixer.c#L679
I don't have any problems with this patch, but I don't know our synchronization methods well enough to know if this is correct. Is it true that SetEvent() causes a context switch? I see it eventually resolves down to a write() in <server/thread.c:send_thread_wakeup()>, but I don't know what that implies.
Julliard, any thoughts?
Andrew