| From 9fb47ad317ad8cdda9960190d499ad6c3a9817f0 Mon Sep 17 00:00:00 2001 |
| From: Greg Kurz <groug@kaod.org> |
| Date: Thu, 19 Jan 2023 18:24:23 +0100 |
| Subject: [PATCH 18/20] Revert "vhost-user: Monitor slave channel in |
| vhost_user_read()" |
| MIME-Version: 1.0 |
| Content-Type: text/plain; charset=UTF-8 |
| Content-Transfer-Encoding: 8bit |
| |
| RH-Author: Laurent Vivier <lvivier@redhat.com> |
| RH-MergeRequest: 146: Fix vhost-user with dpdk |
| RH-Bugzilla: 2155173 |
| RH-Acked-by: Cindy Lu <lulu@redhat.com> |
| RH-Acked-by: Greg Kurz (RH) <gkurz@redhat.com> |
| RH-Acked-by: Eugenio PĂ©rez <eperezma@redhat.com> |
| RH-Commit: [1/2] c583a7f121ca9c93c9a2ad17bf0ccf5c1241dc99 (lvivier/qemu-kvm-centos) |
| |
| This reverts commit db8a3772e300c1a656331a92da0785d81667dc81. |
| |
| Motivation : this is breaking vhost-user with DPDK as reported in [0]. |
| |
| Received unexpected msg type. Expected 22 received 40 |
| Fail to update device iotlb |
| Received unexpected msg type. Expected 40 received 22 |
| Received unexpected msg type. Expected 22 received 11 |
| Fail to update device iotlb |
| Received unexpected msg type. Expected 11 received 22 |
| vhost VQ 1 ring restore failed: -71: Protocol error (71) |
| Received unexpected msg type. Expected 22 received 11 |
| Fail to update device iotlb |
| Received unexpected msg type. Expected 11 received 22 |
| vhost VQ 0 ring restore failed: -71: Protocol error (71) |
| unable to start vhost net: 71: falling back on userspace virtio |
| |
| The failing sequence that leads to the first error is : |
| - QEMU sends a VHOST_USER_GET_STATUS (40) request to DPDK on the master |
| socket |
| - QEMU starts a nested event loop in order to wait for the |
| VHOST_USER_GET_STATUS response and to be able to process messages from |
| the slave channel |
| - DPDK sends a couple of legitimate IOTLB miss messages on the slave |
| channel |
| - QEMU processes each IOTLB request and sends VHOST_USER_IOTLB_MSG (22) |
| updates on the master socket |
| - QEMU assumes to receive a response for the latest VHOST_USER_IOTLB_MSG |
| but it gets the response for the VHOST_USER_GET_STATUS instead |
| |
| The subsequent errors have the same root cause : the nested event loop |
| breaks the order by design. It lures QEMU to expect responses to the |
| latest message sent on the master socket to arrive first. |
| |
| Since this was only needed for DAX enablement which is still not merged |
| upstream, just drop the code for now. A working solution will have to |
| be merged later on. Likely protect the master socket with a mutex |
| and service the slave channel with a separate thread, as discussed with |
| Maxime in the mail thread below. |
| |
| [0] https://lore.kernel.org/qemu-devel/43145ede-89dc-280e-b953-6a2b436de395@redhat.com/ |
| |
| Reported-by: Yanghang Liu <yanghliu@redhat.com> |
| Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2155173 |
| Signed-off-by: Greg Kurz <groug@kaod.org> |
| Message-Id: <20230119172424.478268-2-groug@kaod.org> |
| Reviewed-by: Michael S. Tsirkin <mst@redhat.com> |
| Signed-off-by: Michael S. Tsirkin <mst@redhat.com> |
| Acked-by: Stefan Hajnoczi <stefanha@redhat.com> |
| Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com> |
| (cherry picked from commit f340a59d5a852d75ae34555723694c7e8eafbd0c) |
| Signed-off-by: Laurent Vivier <lvivier@redhat.com> |
| |
| hw/virtio/vhost-user.c | 35 +++-------------------------------- |
| 1 file changed, 3 insertions(+), 32 deletions(-) |
| |
| diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c |
| index 8f635844af..0ac00eb901 100644 |
| |
| |
| @@ -356,35 +356,6 @@ end: |
| return G_SOURCE_REMOVE; |
| } |
| |
| -static gboolean slave_read(QIOChannel *ioc, GIOCondition condition, |
| - gpointer opaque); |
| - |
| -/* |
| - * This updates the read handler to use a new event loop context. |
| - * Event sources are removed from the previous context : this ensures |
| - * that events detected in the previous context are purged. They will |
| - * be re-detected and processed in the new context. |
| - */ |
| -static void slave_update_read_handler(struct vhost_dev *dev, |
| - GMainContext *ctxt) |
| -{ |
| - struct vhost_user *u = dev->opaque; |
| - |
| - if (!u->slave_ioc) { |
| - return; |
| - } |
| - |
| - if (u->slave_src) { |
| - g_source_destroy(u->slave_src); |
| - g_source_unref(u->slave_src); |
| - } |
| - |
| - u->slave_src = qio_channel_add_watch_source(u->slave_ioc, |
| - G_IO_IN | G_IO_HUP, |
| - slave_read, dev, NULL, |
| - ctxt); |
| -} |
| - |
| static int vhost_user_read(struct vhost_dev *dev, VhostUserMsg *msg) |
| { |
| struct vhost_user *u = dev->opaque; |
| @@ -406,7 +377,6 @@ static int vhost_user_read(struct vhost_dev *dev, VhostUserMsg *msg) |
| * be prepared for re-entrancy. So we create a new one and switch chr |
| * to use it. |
| */ |
| - slave_update_read_handler(dev, ctxt); |
| qemu_chr_be_update_read_handlers(chr->chr, ctxt); |
| qemu_chr_fe_add_watch(chr, G_IO_IN | G_IO_HUP, vhost_user_read_cb, &data); |
| |
| @@ -418,7 +388,6 @@ static int vhost_user_read(struct vhost_dev *dev, VhostUserMsg *msg) |
| * context that have been processed by the nested loop are purged. |
| */ |
| qemu_chr_be_update_read_handlers(chr->chr, prev_ctxt); |
| - slave_update_read_handler(dev, NULL); |
| |
| g_main_loop_unref(loop); |
| g_main_context_unref(ctxt); |
| @@ -1802,7 +1771,9 @@ static int vhost_setup_slave_channel(struct vhost_dev *dev) |
| return -ECONNREFUSED; |
| } |
| u->slave_ioc = ioc; |
| - slave_update_read_handler(dev, NULL); |
| + u->slave_src = qio_channel_add_watch_source(u->slave_ioc, |
| + G_IO_IN | G_IO_HUP, |
| + slave_read, dev, NULL, NULL); |
| |
| if (reply_supported) { |
| msg.hdr.flags |= VHOST_USER_NEED_REPLY_MASK; |
| -- |
| 2.31.1 |
| |