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

[Stefan Hajnoczi]

On Wed, Feb 03, 2016 at 10:52:38AM +0100, Paolo Bonzini wrote:
> 
> 
> 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?

How about removing the sleep and adding a comment explaining that the
event_notifier_set() could happen either before or during poll()?

Stefan

signature.asc
Description: PGP signature