Re: [Qemu-devel] [PATCH 05/16] iothread: release AioContext around aio_poll

[Paolo Bonzini]

On 03/02/2016 10:34, Stefan Hajnoczi wrote:
>>>>>>>>> +    g_usleep(500000);
>>>>> Sleep?
>>> 
>>> What about it? :)
> Sleep in a loop is inefficient but at least correct.
> 
> A sleep outside a loop is a race condition.  If the machine is
> heavily loaded, whatever you are waiting for may not happen in 500
> ms and the test case is unreliable.
> 
> What is the purpose of this sleep?

In the test the desired ordering of events is

main thread                             additional thread
---------------------------------------------------------------------
lock start_lock
start thread
                                        lock start_lock (waits)
acquire context
unlock start_lock
                                        lock start_lock (returns)
                                        unlock start_lock
aio_poll
   poll()                                                       <<<<
                                        event_notifier_set      <<<<

Comparing the test to QEMU, the roles of the threads are reversed. The
test's "main thread" is QEMU's dataplane thread, and the test's
additional thread is QEMU's main thread.

The sleep "ensures" that poll() blocks before event_notifier_set.  The
sleep represents how QEMU's main thread acquires the context rarely,
and not right after starting the dataplane thread.

Because this is a file descriptor source, there is really no
difference between the code's behavior, no matter if aio_poll starts
before or after the event_notifier_set.  The test passes even if you
remove the sleep.

Do you have any suggestion?  Just add a comment?

Paolo