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

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1728 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]