Re: [Qemu-devel] [PATCH v2] AioContext: fix broken placement of event_notifier_test_and_clear

[Stefan Hajnoczi]

On Mon, Jul 20, 2015 at 07:27:11AM +0200, Paolo Bonzini wrote:
> event_notifier_test_and_clear must be called before processing events.
> Otherwise, an aio_poll could "eat" the notification before the main
> I/O thread invokes ppoll().  The main I/O thread then never wakes up.
> This is an example of what could happen:
> 
>    i/o thread         vcpu thread                   worker thread
>    ---------------------------------------------------------------------
>    lock_iothread
>    notify_me = 1
>    ...
>    unlock_iothread
>                       lock_iothread
>                       notify_me = 3
>                       ppoll
>                       notify_me = 1
>                                                      bh->scheduled = 1
>                                                      event_notifier_set
>                       event_notifier_test_and_clear
>    ppoll
>    *** hang ***
> 
> In contrast with the previous bug, there wasn't really much debugging
> to do here.  "Tracing" with qemu_clock_get_ns shows pretty much the
> same behavior as in the previous patch, so the only wait to find the
> bug is staring at the code.
> 
> One could also use a formal model, of course.  The included one shows
> this with three processes: notifier corresponds to a QEMU thread pool
> worker, temporary_waiter to a VCPU thread that invokes aio_poll(),
> waiter to the main I/O thread.  I would be happy to say that the
> formal model found the bug for me, but actually I wrote it after the
> fact.
> 
> This patch is a bit of a big hammer.  Fam is looking into optimizations.
> 
> Reported-by: Richard W. M. Jones <address@hidden>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>         v1->v2: only do event_notifier_test_and_clear once for Win32
> 
>  aio-posix.c                 |   2 +
>  aio-win32.c                 |   7 ++-
>  async.c                     |   8 ++-
>  docs/aio_notify_bug.promela | 140 
> ++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 153 insertions(+), 4 deletions(-)
>  create mode 100644 docs/aio_notify_bug.promela

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

pgpOi9xUsbvid.pgp
Description: PGP signature