* [Qemu-devel] [PATCH] dataplane: fix hang introduced by AioContext transition
@ 2013-03-13 14:14 Paolo Bonzini
2013-03-14 9:57 ` Stefan Hajnoczi
2013-03-14 13:04 ` Jens Freimann
0 siblings, 2 replies; 3+ messages in thread
From: Paolo Bonzini @ 2013-03-13 14:14 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha
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
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH] dataplane: fix hang introduced by AioContext transition
2013-03-13 14:14 [Qemu-devel] [PATCH] dataplane: fix hang introduced by AioContext transition Paolo Bonzini
@ 2013-03-14 9:57 ` Stefan Hajnoczi
2013-03-14 13:04 ` Jens Freimann
1 sibling, 0 replies; 3+ messages in thread
From: Stefan Hajnoczi @ 2013-03-14 9:57 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kwolf, qemu-devel, stefanha
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] dataplane: fix hang introduced by AioContext transition
2013-03-13 14:14 [Qemu-devel] [PATCH] dataplane: fix hang introduced by AioContext transition Paolo Bonzini
2013-03-14 9:57 ` Stefan Hajnoczi
@ 2013-03-14 13:04 ` Jens Freimann
1 sibling, 0 replies; 3+ messages in thread
From: Jens Freimann @ 2013-03-14 13:04 UTC (permalink / raw)
To: qemu-devel; +Cc: Cornelia Huck, Paolo Bonzini, dingel, borntraeger
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
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-03-14 13:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-13 14:14 [Qemu-devel] [PATCH] dataplane: fix hang introduced by AioContext transition Paolo Bonzini
2013-03-14 9:57 ` Stefan Hajnoczi
2013-03-14 13:04 ` Jens Freimann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).