[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 05/16] iothread: release AioContext around aio_p
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH 05/16] iothread: release AioContext around aio_poll |
Date: |
Fri, 5 Feb 2016 17:51:49 +0000 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
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