[Qemu-devel] [PATCH v2] main-loop: fix qemu_notify_event for aio_notify optimization

[Qemu-devel] [PATCH v2] main-loop: fix qemu_notify_event for aio_notify optimization

[Paolo Bonzini]

aio_notify can be optimized away, and in fact almost always will.  However,
qemu_notify_event is used in places where this is incorrect---most notably,
when handling SIGTERM.  When aio_notify is optimized away, it is possible that
QEMU enters a blocking ppoll immediately afterwards and stays there, without
reaching main_loop_should_exit().

Fix this by using a bottom half.  The bottom half can be optimized too, but
scheduling it is enough for the ppoll not to block.  The hang is thus avoided.

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 main-loop.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/main-loop.c b/main-loop.c
index 82875a4..3997043 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -114,6 +114,14 @@ static int qemu_signal_init(void)
 #endif
 
 static AioContext *qemu_aio_context;
+static QEMUBH *qemu_notify_bh;
+
+static void notify_event_cb(void *opaque)
+{
+    /* No need to do anything; this bottom half is only used to
+     * kick the kernel out of ppoll/poll/WaitForMultipleObjects.
+     */
+}
 
 AioContext *qemu_get_aio_context(void)
 {
@@ -125,7 +133,7 @@ void qemu_notify_event(void)
     if (!qemu_aio_context) {
         return;
     }
-    aio_notify(qemu_aio_context);
+    qemu_bh_schedule(qemu_notify_bh);
 }
 
 static GArray *gpollfds;
@@ -144,6 +152,7 @@ int qemu_init_main_loop(Error **errp)
     }
 
     qemu_aio_context = aio_context_new(&local_error);
+    qemu_notify_bh = qemu_bh_new(notify_event_cb, NULL);
     if (!qemu_aio_context) {
         error_propagate(errp, local_error);
         return -EMFILE;
-- 
2.4.3

Re: [Qemu-devel] [PATCH v2] main-loop: fix qemu_notify_event for aio_notify optimization

[Stefan Hajnoczi]

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

On Fri, Jul 24, 2015 at 01:42:55PM +0200, Paolo Bonzini wrote:
> aio_notify can be optimized away, and in fact almost always will.  However,
> qemu_notify_event is used in places where this is incorrect---most notably,
> when handling SIGTERM.  When aio_notify is optimized away, it is possible that
> QEMU enters a blocking ppoll immediately afterwards and stays there, without
> reaching main_loop_should_exit().
> 
> Fix this by using a bottom half.  The bottom half can be optimized too, but
> scheduling it is enough for the ppoll not to block.  The hang is thus avoided.
> 
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  main-loop.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

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

Re: [Qemu-devel] [PATCH v2] main-loop: fix qemu_notify_event for aio_notify optimization

[Peter Maydell]

On 24 July 2015 at 12:42, Paolo Bonzini <pbonzini@redhat.com> wrote:
> aio_notify can be optimized away, and in fact almost always will.  However,
> qemu_notify_event is used in places where this is incorrect---most notably,
> when handling SIGTERM.  When aio_notify is optimized away, it is possible that
> QEMU enters a blocking ppoll immediately afterwards and stays there, without
> reaching main_loop_should_exit().
>
> Fix this by using a bottom half.  The bottom half can be optimized too, but
> scheduling it is enough for the ppoll not to block.  The hang is thus avoided.
>
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Applied to master, thanks.

-- PMM