* [Qemu-devel] [PATCH 1/2] virtio: add aio handler
2016-03-29 13:42 [Qemu-devel] [PATCH 0/2] dataplane: fix start/stop races Michael S. Tsirkin
@ 2016-03-29 13:42 ` Michael S. Tsirkin
2016-03-29 14:19 ` Paolo Bonzini
2016-03-29 13:42 ` [Qemu-devel] [PATCH 2/2] virtio-blk: use aio handler for data plane Michael S. Tsirkin
` (3 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2016-03-29 13:42 UTC (permalink / raw)
To: qemu-devel; +Cc: Cornelia Huck, pbonzini
In addition to handling IO in vcpu thread and
in io thread, blk dataplane introduces yet another mode:
handling it by aio.
This reuses the same handler as previous modes,
which triggers races as these were not designed to be reentrant.
As a temporary fix, add a separate handler just for aio, this will make
it possible to disable regular handlers when dataplane is active.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/hw/virtio/virtio.h | 4 ++++
hw/virtio/virtio.c | 36 ++++++++++++++++++++++++++++++++----
2 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 2b5b248..c032067 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -142,6 +142,9 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,
void (*handle_output)(VirtIODevice *,
VirtQueue *));
+void virtio_set_queue_aio(VirtQueue *vq,
+ void (*handle_output)(VirtIODevice *, VirtQueue *));
+
void virtio_del_queue(VirtIODevice *vdev, int n);
void *virtqueue_alloc_element(size_t sz, unsigned out_num, unsigned in_num);
@@ -253,6 +256,7 @@ void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign,
void virtio_queue_aio_set_host_notifier_handler(VirtQueue *vq, AioContext *ctx,
bool assign, bool set_handler);
void virtio_queue_notify_vq(VirtQueue *vq);
+void virtio_queue_notify_aio_vq(VirtQueue *vq);
void virtio_irq(VirtQueue *vq);
VirtQueue *virtio_vector_first_queue(VirtIODevice *vdev, uint16_t vector);
VirtQueue *virtio_vector_next_queue(VirtQueue *vq);
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 08275a9..182bc56 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -94,6 +94,7 @@ struct VirtQueue
uint16_t vector;
void (*handle_output)(VirtIODevice *vdev, VirtQueue *vq);
+ void (*handle_aio_output)(VirtIODevice *vdev, VirtQueue *vq);
VirtIODevice *vdev;
EventNotifier guest_notifier;
EventNotifier host_notifier;
@@ -1086,6 +1087,16 @@ void virtio_queue_set_align(VirtIODevice *vdev, int n, int align)
virtio_queue_update_rings(vdev, n);
}
+void virtio_queue_notify_aio_vq(VirtQueue *vq)
+{
+ if (vq->vring.desc && vq->handle_aio_output) {
+ VirtIODevice *vdev = vq->vdev;
+
+ trace_virtio_queue_notify(vdev, vq - vdev->vq, vq);
+ vq->handle_aio_output(vdev, vq);
+ }
+}
+
void virtio_queue_notify_vq(VirtQueue *vq)
{
if (vq->vring.desc && vq->handle_output) {
@@ -1141,10 +1152,19 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,
vdev->vq[i].vring.num_default = queue_size;
vdev->vq[i].vring.align = VIRTIO_PCI_VRING_ALIGN;
vdev->vq[i].handle_output = handle_output;
+ vdev->vq[i].handle_aio_output = NULL;
return &vdev->vq[i];
}
+void virtio_set_queue_aio(VirtQueue *vq,
+ void (*handle_output)(VirtIODevice *, VirtQueue *))
+{
+ assert(vq->handle_output);
+
+ vq->handle_aio_output = handle_output;
+}
+
void virtio_del_queue(VirtIODevice *vdev, int n)
{
if (n < 0 || n >= VIRTIO_QUEUE_MAX) {
@@ -1778,11 +1798,11 @@ EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq)
return &vq->guest_notifier;
}
-static void virtio_queue_host_notifier_read(EventNotifier *n)
+static void virtio_queue_host_notifier_aio_read(EventNotifier *n)
{
VirtQueue *vq = container_of(n, VirtQueue, host_notifier);
if (event_notifier_test_and_clear(n)) {
- virtio_queue_notify_vq(vq);
+ virtio_queue_notify_aio_vq(vq);
}
}
@@ -1791,14 +1811,22 @@ void virtio_queue_aio_set_host_notifier_handler(VirtQueue *vq, AioContext *ctx,
{
if (assign && set_handler) {
aio_set_event_notifier(ctx, &vq->host_notifier, true,
- virtio_queue_host_notifier_read);
+ virtio_queue_host_notifier_aio_read);
} else {
aio_set_event_notifier(ctx, &vq->host_notifier, true, NULL);
}
if (!assign) {
/* Test and clear notifier before after disabling event,
* in case poll callback didn't have time to run. */
- virtio_queue_host_notifier_read(&vq->host_notifier);
+ virtio_queue_host_notifier_aio_read(&vq->host_notifier);
+ }
+}
+
+static void virtio_queue_host_notifier_read(EventNotifier *n)
+{
+ VirtQueue *vq = container_of(n, VirtQueue, host_notifier);
+ if (event_notifier_test_and_clear(n)) {
+ virtio_queue_notify_vq(vq);
}
}
--
MST
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 2/2] virtio-blk: use aio handler for data plane
2016-03-29 13:42 [Qemu-devel] [PATCH 0/2] dataplane: fix start/stop races Michael S. Tsirkin
2016-03-29 13:42 ` [Qemu-devel] [PATCH 1/2] virtio: add aio handler Michael S. Tsirkin
@ 2016-03-29 13:42 ` Michael S. Tsirkin
2016-03-29 13:56 ` Paolo Bonzini
2016-03-29 14:05 ` Paolo Bonzini
2016-03-29 13:43 ` [Qemu-devel] [PATCH 0/2] dataplane: fix start/stop races Michael S. Tsirkin
` (2 subsequent siblings)
4 siblings, 2 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2016-03-29 13:42 UTC (permalink / raw)
To: qemu-devel
Cc: Cornelia Huck, pbonzini, Kevin Wolf, qemu-block, Stefan Hajnoczi
In addition to handling IO in vcpu thread and
in io thread, blk dataplane introduces yet another mode:
handling it by aio.
This reuses the same handler as previous modes,
which triggers races as these were not designed to be reentrant.
As a temporary fix, use a separate handler just for aio, and
disable regular handlers when dataplane is active.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/hw/virtio/virtio-blk.h | 2 ++
hw/block/dataplane/virtio-blk.c | 13 +++++++++++++
hw/block/virtio-blk.c | 28 ++++++++++++++++++----------
3 files changed, 33 insertions(+), 10 deletions(-)
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index ae84d92..df517ff 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -85,4 +85,6 @@ void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb);
void virtio_blk_submit_multireq(BlockBackend *blk, MultiReqBuffer *mrb);
+void virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq);
+
#endif
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 36f3d2b..7d1f3b2 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -184,6 +184,17 @@ void virtio_blk_data_plane_destroy(VirtIOBlockDataPlane *s)
g_free(s);
}
+static void virtio_blk_data_plane_handle_output(VirtIODevice *vdev,
+ VirtQueue *vq)
+{
+ VirtIOBlock *s = VIRTIO_BLK(vdev);
+
+ assert(s->dataplane);
+ assert(s->dataplane_started);
+
+ virtio_blk_handle_vq(s, vq);
+}
+
/* Context: QEMU global mutex held */
void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
{
@@ -226,6 +237,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
/* Get this show started by hooking up our callbacks */
aio_context_acquire(s->ctx);
+ virtio_set_queue_aio(s->vq, virtio_blk_data_plane_handle_output);
virtio_queue_aio_set_host_notifier_handler(s->vq, s->ctx, true, true);
aio_context_release(s->ctx);
return;
@@ -262,6 +274,7 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
/* Stop notifications for new requests from guest */
virtio_queue_aio_set_host_notifier_handler(s->vq, s->ctx, false, false);
+ virtio_set_queue_aio(s->vq, NULL);
/* Drain and switch bs back to the QEMU main loop */
blk_set_aio_context(s->conf->conf.blk, qemu_get_aio_context());
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index cb710f1..5717f09 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -577,20 +577,11 @@ void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)
}
}
-static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq)
+void virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq)
{
- VirtIOBlock *s = VIRTIO_BLK(vdev);
VirtIOBlockReq *req;
MultiReqBuffer mrb = {};
- /* Some guests kick before setting VIRTIO_CONFIG_S_DRIVER_OK so start
- * dataplane here instead of waiting for .set_status().
- */
- if (s->dataplane && !s->dataplane_started) {
- virtio_blk_data_plane_start(s->dataplane);
- return;
- }
-
blk_io_plug(s->blk);
while ((req = virtio_blk_get_request(s))) {
@@ -604,6 +595,23 @@ static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq)
blk_io_unplug(s->blk);
}
+static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq)
+{
+ VirtIOBlock *s = VIRTIO_BLK(vdev);
+
+ if (s->dataplane) {
+ /* Some guests kick before setting VIRTIO_CONFIG_S_DRIVER_OK so start
+ * dataplane here instead of waiting for .set_status().
+ */
+ if (!s->dataplane_started) {
+ virtio_blk_data_plane_start(s->dataplane);
+ }
+ return;
+ }
+
+ virtio_blk_handle_vq(s, vq);
+}
+
static void virtio_blk_dma_restart_bh(void *opaque)
{
VirtIOBlock *s = opaque;
--
MST
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] virtio-blk: use aio handler for data plane
2016-03-29 13:42 ` [Qemu-devel] [PATCH 2/2] virtio-blk: use aio handler for data plane Michael S. Tsirkin
@ 2016-03-29 13:56 ` Paolo Bonzini
2016-03-29 13:58 ` Michael S. Tsirkin
2016-03-29 14:05 ` Paolo Bonzini
1 sibling, 1 reply; 15+ messages in thread
From: Paolo Bonzini @ 2016-03-29 13:56 UTC (permalink / raw)
To: Michael S. Tsirkin, qemu-devel
Cc: Cornelia Huck, Kevin Wolf, qemu-block, Stefan Hajnoczi
On 29/03/2016 15:42, Michael S. Tsirkin wrote:
> @@ -262,6 +274,7 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
>
> /* Stop notifications for new requests from guest */
> virtio_queue_aio_set_host_notifier_handler(s->vq, s->ctx, false, false);
I think that this should have been ", true, false" even before your
patch; I'd prefer to fix it even if the problem is latent.
The patch looks good, and it might even be an API improvement
independent of Conny's patches. The reentrancy _is_ hard to understand,
and eliminating it makes the new API not just a hack.
In that case I would unify the new function with
virtio_queue_aio_set_host_notifier_handler. In other words
- virtio_queue_aio_set_host_notifier_handler(vq, ctx, NULL) is
the same as
virtio_set_queue_aio(s->vq, NULL);
virtio_queue_aio_set_host_notifier_handler(vq, ctx, true, false);
- virtio_queue_aio_set_host_notifier_handler(vq, ctx, fn) is the same as
virtio_queue_aio_set_host_notifier_handler(vq, ctx, true, true);
virtio_set_queue_aio(vq, fn);
Thanks,
Paolo
> + virtio_set_queue_aio(s->vq, NULL);
>
> /* Drain and switch bs back to the QEMU main loop */
> blk_set_aio_context(s->conf->conf.blk, qemu_get_aio_context());
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] virtio-blk: use aio handler for data plane
2016-03-29 13:56 ` Paolo Bonzini
@ 2016-03-29 13:58 ` Michael S. Tsirkin
2016-03-29 13:59 ` Paolo Bonzini
0 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2016-03-29 13:58 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Cornelia Huck, Kevin Wolf, qemu-block, qemu-devel,
Stefan Hajnoczi
On Tue, Mar 29, 2016 at 03:56:18PM +0200, Paolo Bonzini wrote:
>
>
> On 29/03/2016 15:42, Michael S. Tsirkin wrote:
> > @@ -262,6 +274,7 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
> >
> > /* Stop notifications for new requests from guest */
> > virtio_queue_aio_set_host_notifier_handler(s->vq, s->ctx, false, false);
>
> I think that this should have been ", true, false" even before your
> patch; I'd prefer to fix it even if the problem is latent.
Makes sense - post a patch?
> The patch looks good, and it might even be an API improvement
> independent of Conny's patches. The reentrancy _is_ hard to understand,
> and eliminating it makes the new API not just a hack.
>
> In that case I would unify the new function with
> virtio_queue_aio_set_host_notifier_handler. In other words
>
> - virtio_queue_aio_set_host_notifier_handler(vq, ctx, NULL) is
> the same as
>
> virtio_set_queue_aio(s->vq, NULL);
> virtio_queue_aio_set_host_notifier_handler(vq, ctx, true, false);
>
> - virtio_queue_aio_set_host_notifier_handler(vq, ctx, fn) is the same as
>
> virtio_queue_aio_set_host_notifier_handler(vq, ctx, true, true);
> virtio_set_queue_aio(vq, fn);
>
> Thanks,
>
> Paolo
In that case, we'll have to also change scsi to use the new API.
A bit more work, to be sure.
Does scsi have the same problem as blk?
> > + virtio_set_queue_aio(s->vq, NULL);
> >
> > /* Drain and switch bs back to the QEMU main loop */
> > blk_set_aio_context(s->conf->conf.blk, qemu_get_aio_context());
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] virtio-blk: use aio handler for data plane
2016-03-29 13:42 ` [Qemu-devel] [PATCH 2/2] virtio-blk: use aio handler for data plane Michael S. Tsirkin
2016-03-29 13:56 ` Paolo Bonzini
@ 2016-03-29 14:05 ` Paolo Bonzini
2016-03-29 14:09 ` Michael S. Tsirkin
1 sibling, 1 reply; 15+ messages in thread
From: Paolo Bonzini @ 2016-03-29 14:05 UTC (permalink / raw)
To: Michael S. Tsirkin, qemu-devel
Cc: Cornelia Huck, Kevin Wolf, Stefan Hajnoczi, qemu-block
On 29/03/2016 15:42, Michael S. Tsirkin wrote:
> + if (s->dataplane) {
> + /* Some guests kick before setting VIRTIO_CONFIG_S_DRIVER_OK so start
> + * dataplane here instead of waiting for .set_status().
> + */
> + if (!s->dataplane_started) {
> + virtio_blk_data_plane_start(s->dataplane);
> + }
> + return;
> + }
> +
> + virtio_blk_handle_vq(s, vq);
Another small comment, this can be written simply as
if (s->dataplane) {
virtio_blk_data_plane_start(s->dataplane);
} else {
virtio_blk_handle_vq(s, vq);
}
Paolo
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] virtio-blk: use aio handler for data plane
2016-03-29 14:05 ` Paolo Bonzini
@ 2016-03-29 14:09 ` Michael S. Tsirkin
2016-03-29 14:44 ` Paolo Bonzini
0 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2016-03-29 14:09 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Cornelia Huck, Kevin Wolf, Stefan Hajnoczi, qemu-devel,
qemu-block
On Tue, Mar 29, 2016 at 04:05:46PM +0200, Paolo Bonzini wrote:
>
>
> On 29/03/2016 15:42, Michael S. Tsirkin wrote:
> > + if (s->dataplane) {
> > + /* Some guests kick before setting VIRTIO_CONFIG_S_DRIVER_OK so start
> > + * dataplane here instead of waiting for .set_status().
> > + */
> > + if (!s->dataplane_started) {
> > + virtio_blk_data_plane_start(s->dataplane);
> > + }
> > + return;
> > + }
> > +
> > + virtio_blk_handle_vq(s, vq);
>
> Another small comment, this can be written simply as
>
> if (s->dataplane) {
> virtio_blk_data_plane_start(s->dataplane);
True, it's best not to poke at dataplane_started.
> } else {
> virtio_blk_handle_vq(s, vq);
> }
>
I prefer the return style I think, to stress the
fact that this is an unusual, unexpected case.
> Paolo
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] virtio-blk: use aio handler for data plane
2016-03-29 14:09 ` Michael S. Tsirkin
@ 2016-03-29 14:44 ` Paolo Bonzini
0 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2016-03-29 14:44 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Cornelia Huck, Kevin Wolf, Stefan Hajnoczi, qemu-devel,
qemu-block
On 29/03/2016 16:09, Michael S. Tsirkin wrote:
>> > Another small comment, this can be written simply as
>> >
>> > if (s->dataplane) {
>> > virtio_blk_data_plane_start(s->dataplane);
>
> True, it's best not to poke at dataplane_started.
>
> > } else {
> > virtio_blk_handle_vq(s, vq);
> > }
> >
>
> I prefer the return style I think, to stress the
> fact that this is an unusual, unexpected case.
We're getting dangerously close to personal preference, but I've noticed
virtio-scsi that you get a very common pattern of:
void virtio_scsi_handle_ctrl_vq(VirtIOSCSI *s, VirtQueue *vq)
{
... virtqueue_pop ...
}
static void virtio_scsi_data_plane_handle_ctrl(VirtIODevice *vdev,
VirtQueue *vq)
{
VirtIOSCSI *s = VIRTIO_SCSI(vdev);
assert(dataplane active and started);
virtio_scsi_handle_ctrl_vq(s, vq);
}
static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
{
VirtIOSCSI *s = VIRTIO_SCSI(vdev);
if (dataplane active) {
virtio_scsi_dataplane_start(s);
} else {
virtio_scsi_handle_ctrl_vq(s, vq);
}
}
so it's not really an unusual, unexpected case but a complete
separation between the dataplane case (handle_output starts
dataplane) and the non-dataplane case (handle_output just does
a cast and calls the actual workhorse).
Paolo
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] dataplane: fix start/stop races
2016-03-29 13:42 [Qemu-devel] [PATCH 0/2] dataplane: fix start/stop races Michael S. Tsirkin
2016-03-29 13:42 ` [Qemu-devel] [PATCH 1/2] virtio: add aio handler Michael S. Tsirkin
2016-03-29 13:42 ` [Qemu-devel] [PATCH 2/2] virtio-blk: use aio handler for data plane Michael S. Tsirkin
@ 2016-03-29 13:43 ` Michael S. Tsirkin
2016-03-29 15:06 ` Cornelia Huck
2016-03-29 16:31 ` Christian Borntraeger
4 siblings, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2016-03-29 13:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Cornelia Huck, pbonzini
On Tue, Mar 29, 2016 at 04:42:09PM +0300, Michael S. Tsirkin wrote:
> This works around races that data plane introduces
> simply by exiting immediately if we detect
> that dataplane is active.
>
> It's a small but ugly patch, it's only justification
> is that it's minimally intrusive, and that it clearly
> has no chance to break non data plane users.
>
> The idea is to rework it all post 2.6.
Testing status: lightly tested.
> Michael S. Tsirkin (2):
> virtio: add aio handler
> virtio-blk: use aio handler for data plane
>
> include/hw/virtio/virtio-blk.h | 2 ++
> include/hw/virtio/virtio.h | 4 ++++
> hw/block/dataplane/virtio-blk.c | 13 +++++++++++++
> hw/block/virtio-blk.c | 28 ++++++++++++++++++----------
> hw/virtio/virtio.c | 36 ++++++++++++++++++++++++++++++++----
> 5 files changed, 69 insertions(+), 14 deletions(-)
>
> --
> MST
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] dataplane: fix start/stop races
2016-03-29 13:42 [Qemu-devel] [PATCH 0/2] dataplane: fix start/stop races Michael S. Tsirkin
` (2 preceding siblings ...)
2016-03-29 13:43 ` [Qemu-devel] [PATCH 0/2] dataplane: fix start/stop races Michael S. Tsirkin
@ 2016-03-29 15:06 ` Cornelia Huck
2016-03-29 16:31 ` Christian Borntraeger
4 siblings, 0 replies; 15+ messages in thread
From: Cornelia Huck @ 2016-03-29 15:06 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: pbonzini, qemu-devel, Christian Borntraeger
On Tue, 29 Mar 2016 16:42:09 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> This works around races that data plane introduces
> simply by exiting immediately if we detect
> that dataplane is active.
>
> It's a small but ugly patch, it's only justification
> is that it's minimally intrusive, and that it clearly
> has no chance to break non data plane users.
>
> The idea is to rework it all post 2.6.
>
> Michael S. Tsirkin (2):
> virtio: add aio handler
> virtio-blk: use aio handler for data plane
>
> include/hw/virtio/virtio-blk.h | 2 ++
> include/hw/virtio/virtio.h | 4 ++++
> hw/block/dataplane/virtio-blk.c | 13 +++++++++++++
> hw/block/virtio-blk.c | 28 ++++++++++++++++++----------
> hw/virtio/virtio.c | 36 ++++++++++++++++++++++++++++++++----
> 5 files changed, 69 insertions(+), 14 deletions(-)
>
FWIW, this seems to survive an hour or so of reboot loops for me, but
that's only a small setup.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] dataplane: fix start/stop races
2016-03-29 13:42 [Qemu-devel] [PATCH 0/2] dataplane: fix start/stop races Michael S. Tsirkin
` (3 preceding siblings ...)
2016-03-29 15:06 ` Cornelia Huck
@ 2016-03-29 16:31 ` Christian Borntraeger
2016-03-30 3:04 ` tu bo
4 siblings, 1 reply; 15+ messages in thread
From: Christian Borntraeger @ 2016-03-29 16:31 UTC (permalink / raw)
To: Michael S. Tsirkin, qemu-devel; +Cc: Cornelia Huck, pbonzini, tubo
On 03/29/2016 03:42 PM, Michael S. Tsirkin wrote:
> This works around races that data plane introduces
> simply by exiting immediately if we detect
> that dataplane is active.
>
> It's a small but ugly patch, it's only justification
> is that it's minimally intrusive, and that it clearly
> has no chance to break non data plane users.
>
> The idea is to rework it all post 2.6.
>
> Michael S. Tsirkin (2):
> virtio: add aio handler
> virtio-blk: use aio handler for data plane
>
> include/hw/virtio/virtio-blk.h | 2 ++
> include/hw/virtio/virtio.h | 4 ++++
> hw/block/dataplane/virtio-blk.c | 13 +++++++++++++
> hw/block/virtio-blk.c | 28 ++++++++++++++++++----------
> hw/virtio/virtio.c | 36 ++++++++++++++++++++++++++++++++----
> 5 files changed, 69 insertions(+), 14 deletions(-)
>
This also seems to help on my setup.Tu Bo, would be good if you
can double check this patch set as well on your setup?
Christian
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] dataplane: fix start/stop races
2016-03-29 16:31 ` Christian Borntraeger
@ 2016-03-30 3:04 ` tu bo
2016-03-30 7:32 ` Paolo Bonzini
0 siblings, 1 reply; 15+ messages in thread
From: tu bo @ 2016-03-30 3:04 UTC (permalink / raw)
To: Christian Borntraeger, Michael S. Tsirkin, qemu-devel
Cc: Cornelia Huck, pbonzini, tubo
Hi Christian:
On 03/30/2016 12:31 AM, Christian Borntraeger wrote:
> On 03/29/2016 03:42 PM, Michael S. Tsirkin wrote:
>> This works around races that data plane introduces
>> simply by exiting immediately if we detect
>> that dataplane is active.
>>
>> It's a small but ugly patch, it's only justification
>> is that it's minimally intrusive, and that it clearly
>> has no chance to break non data plane users.
>>
>> The idea is to rework it all post 2.6.
>>
>> Michael S. Tsirkin (2):
>> virtio: add aio handler
>> virtio-blk: use aio handler for data plane
>>
>> include/hw/virtio/virtio-blk.h | 2 ++
>> include/hw/virtio/virtio.h | 4 ++++
>> hw/block/dataplane/virtio-blk.c | 13 +++++++++++++
>> hw/block/virtio-blk.c | 28 ++++++++++++++++++----------
>> hw/virtio/virtio.c | 36 ++++++++++++++++++++++++++++++++----
>> 5 files changed, 69 insertions(+), 14 deletions(-)
>>
>
> This also seems to help on my setup.Tu Bo, would be good if you
> can double check this patch set as well on your setup?
With qemu master + [PATCH 0/2] dataplane: fix start/stop races,
I did NOT see any crash, result is good in my box. thanks
>
> Christian
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] dataplane: fix start/stop races
2016-03-30 3:04 ` tu bo
@ 2016-03-30 7:32 ` Paolo Bonzini
0 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2016-03-30 7:32 UTC (permalink / raw)
To: tu bo, Christian Borntraeger, Michael S. Tsirkin, qemu-devel
Cc: Cornelia Huck, tubo
On 30/03/2016 05:04, tu bo wrote:
> Hi Christian:
>
> On 03/30/2016 12:31 AM, Christian Borntraeger wrote:
>> On 03/29/2016 03:42 PM, Michael S. Tsirkin wrote:
>>> This works around races that data plane introduces
>>> simply by exiting immediately if we detect
>>> that dataplane is active.
>>>
>>> It's a small but ugly patch, it's only justification
>>> is that it's minimally intrusive, and that it clearly
>>> has no chance to break non data plane users.
>>>
>>> The idea is to rework it all post 2.6.
>>>
>>> Michael S. Tsirkin (2):
>>> virtio: add aio handler
>>> virtio-blk: use aio handler for data plane
>>>
>>> include/hw/virtio/virtio-blk.h | 2 ++
>>> include/hw/virtio/virtio.h | 4 ++++
>>> hw/block/dataplane/virtio-blk.c | 13 +++++++++++++
>>> hw/block/virtio-blk.c | 28 ++++++++++++++++++----------
>>> hw/virtio/virtio.c | 36
>>> ++++++++++++++++++++++++++++++++----
>>> 5 files changed, 69 insertions(+), 14 deletions(-)
>>>
>>
>> This also seems to help on my setup.Tu Bo, would be good if you
>> can double check this patch set as well on your setup?
>
> With qemu master + [PATCH 0/2] dataplane: fix start/stop races,
>
> I did NOT see any crash, result is good in my box. thanks
Great, I will send a revised version of Michael's patches with
virtio-scsi too.
Paolo
^ permalink raw reply [flat|nested] 15+ messages in thread