[Qemu-devel] [PATCH] dataplane: fix hang introduced by AioContext transition

[Qemu-devel] [PATCH] dataplane: fix hang introduced by AioContext transition

[Paolo Bonzini]

The bug is that the EventNotifiers do have a NULL io_flush callback.
Because _none_ of the callbacks on the dataplane AioContext have such a
callback, aio_poll will simply do nothing.  Fixed by adding the callbacks:
the ioeventfd will always be polled (this can change in the future to
pause/resume the processing during live snapshots or similar operations);
the ioqueue will be polled if there are outstanding requests.

I must admit I have screwed up my testing somehow, because commit
2c20e71 does not work even if cherry-picked on top of 1.4.0, and this
patch fixes it there as well.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/dataplane/virtio-blk.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/hw/dataplane/virtio-blk.c b/hw/dataplane/virtio-blk.c
index aa9b040..24994fa 100644
--- a/hw/dataplane/virtio-blk.c
+++ b/hw/dataplane/virtio-blk.c
@@ -261,6 +261,11 @@ static int process_request(IOQueue *ioq, struct iovec iov[],
     }
 }
 
+static int flush_true(EventNotifier *e)
+{
+    return true;
+}
+
 static void handle_notify(EventNotifier *e)
 {
     VirtIOBlockDataPlane *s = container_of(e, VirtIOBlockDataPlane,
@@ -340,6 +345,14 @@ static void handle_notify(EventNotifier *e)
     }
 }
 
+static int flush_io(EventNotifier *e)
+{
+    VirtIOBlockDataPlane *s = container_of(e, VirtIOBlockDataPlane,
+                                           io_notifier);
+
+    return s->num_reqs > 0;
+}
+
 static void handle_io(EventNotifier *e)
 {
     VirtIOBlockDataPlane *s = container_of(e, VirtIOBlockDataPlane,
@@ -470,7 +483,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
         exit(1);
     }
     s->host_notifier = *virtio_queue_get_host_notifier(vq);
-    aio_set_event_notifier(s->ctx, &s->host_notifier, handle_notify, NULL);
+    aio_set_event_notifier(s->ctx, &s->host_notifier, handle_notify, flush_true);
 
     /* Set up ioqueue */
     ioq_init(&s->ioqueue, s->fd, REQ_MAX);
@@ -478,7 +491,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
         ioq_put_iocb(&s->ioqueue, &s->requests[i].iocb);
     }
     s->io_notifier = *ioq_get_notifier(&s->ioqueue);
-    aio_set_event_notifier(s->ctx, &s->io_notifier, handle_io, NULL);
+    aio_set_event_notifier(s->ctx, &s->io_notifier, handle_io, flush_io);
 
     s->started = true;
     trace_virtio_blk_data_plane_start(s);
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH] dataplane: fix hang introduced by AioContext transition

[Stefan Hajnoczi]

On Wed, Mar 13, 2013 at 03:14:15PM +0100, Paolo Bonzini wrote:
> The bug is that the EventNotifiers do have a NULL io_flush callback.
> Because _none_ of the callbacks on the dataplane AioContext have such a
> callback, aio_poll will simply do nothing.  Fixed by adding the callbacks:
> the ioeventfd will always be polled (this can change in the future to
> pause/resume the processing during live snapshots or similar operations);
> the ioqueue will be polled if there are outstanding requests.
> 
> I must admit I have screwed up my testing somehow, because commit
> 2c20e71 does not work even if cherry-picked on top of 1.4.0, and this
> patch fixes it there as well.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/dataplane/virtio-blk.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)

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

Stefan

Re: [Qemu-devel] [PATCH] dataplane: fix hang introduced by AioContext transition

[Jens Freimann]

On Wed, Mar 13, 2013 at 03:14:15PM +0100, Paolo Bonzini wrote:
> The bug is that the EventNotifiers do have a NULL io_flush callback.
> Because _none_ of the callbacks on the dataplane AioContext have such a
> callback, aio_poll will simply do nothing.  Fixed by adding the callbacks:
> the ioeventfd will always be polled (this can change in the future to
> pause/resume the processing during live snapshots or similar operations);
> the ioqueue will be polled if there are outstanding requests.

This also fixes the problem we were seeing on s390x. 

regards
Jens

> 
> I must admit I have screwed up my testing somehow, because commit
> 2c20e71 does not work even if cherry-picked on top of 1.4.0, and this
> patch fixes it there as well.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/dataplane/virtio-blk.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/dataplane/virtio-blk.c b/hw/dataplane/virtio-blk.c
> index aa9b040..24994fa 100644
> --- a/hw/dataplane/virtio-blk.c
> +++ b/hw/dataplane/virtio-blk.c
> @@ -261,6 +261,11 @@ static int process_request(IOQueue *ioq, struct iovec iov[],
>      }
>  }
> 
> +static int flush_true(EventNotifier *e)
> +{
> +    return true;
> +}
> +
>  static void handle_notify(EventNotifier *e)
>  {
>      VirtIOBlockDataPlane *s = container_of(e, VirtIOBlockDataPlane,
> @@ -340,6 +345,14 @@ static void handle_notify(EventNotifier *e)
>      }
>  }
> 
> +static int flush_io(EventNotifier *e)
> +{
> +    VirtIOBlockDataPlane *s = container_of(e, VirtIOBlockDataPlane,
> +                                           io_notifier);
> +
> +    return s->num_reqs > 0;
> +}
> +
>  static void handle_io(EventNotifier *e)
>  {
>      VirtIOBlockDataPlane *s = container_of(e, VirtIOBlockDataPlane,
> @@ -470,7 +483,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
>          exit(1);
>      }
>      s->host_notifier = *virtio_queue_get_host_notifier(vq);
> -    aio_set_event_notifier(s->ctx, &s->host_notifier, handle_notify, NULL);
> +    aio_set_event_notifier(s->ctx, &s->host_notifier, handle_notify, flush_true);
> 
>      /* Set up ioqueue */
>      ioq_init(&s->ioqueue, s->fd, REQ_MAX);
> @@ -478,7 +491,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
>          ioq_put_iocb(&s->ioqueue, &s->requests[i].iocb);
>      }
>      s->io_notifier = *ioq_get_notifier(&s->ioqueue);
> -    aio_set_event_notifier(s->ctx, &s->io_notifier, handle_io, NULL);
> +    aio_set_event_notifier(s->ctx, &s->io_notifier, handle_io, flush_io);
> 
>      s->started = true;
>      trace_virtio_blk_data_plane_start(s);
> -- 
> 1.8.1.4
> 
>