| Message ID | 20241115080312.3184-1-zuoboqun@baidu.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] vhost_net: fix assertion triggered by batch of host notifiers processing | expand |
On Fri, Nov 15, 2024 at 04:03:12PM +0800, Zuo boqun wrote: >From: zuoboqun <zuoboqun@baidu.com> > >When the backend of vhost_net restarts during the vm is running, vhost_net >is stopped and started. The virtio_device_grab_ioeventfd() fucntion in >vhost_net_enable_notifiers() will result in a call to >virtio_bus_set_host_notifier()(assign=false). > >And now virtio_device_grab_ioeventfd() is batched in a single transaction >with virtio_bus_set_host_notifier()(assign=true). > >This triggers the following assertion: > >kvm_mem_ioeventfd_del: error deleting ioeventfd: Bad file descriptor > >This patch moves virtio_device_grab_ioeventfd() out of the batch to fix >this problem. > >To be noted that the for loop to release ioeventfd should start from i+1, >not i, because the i-th ioeventfd has already been released in >vhost_dev_disable_notifiers_nvqs(). > >Fixes: 6166799f6 ("vhost_net: configure all host notifiers in a single MR transaction") >Signed-off-by: Zuo Boqun <zuoboqun@baidu.com> >Reported-by: Gao Shiyuan <gaoshiyuan@baidu.com> > >--- > >v1->v2: > *To explain why the for loop to release ioeventfd starts from i+1: > 1) add a comment in the code > 2) describe it in the commit message Thanks for that! LGTM, but I don't know vhost-net enough so I'd wait for a comment from Michael and Jason. Acked-by: Stefano Garzarella <sgarzare@redhat.com> >--- > hw/net/vhost_net.c | 35 ++++++++++++++++++++++++----------- > 1 file changed, 24 insertions(+), 11 deletions(-) > >diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c >index 997aab0557..891f235a0a 100644 >--- a/hw/net/vhost_net.c >+++ b/hw/net/vhost_net.c >@@ -229,9 +229,24 @@ static int vhost_net_enable_notifiers(VirtIODevice *dev, > int nvhosts = data_queue_pairs + cvq; > struct vhost_net *net; > struct vhost_dev *hdev; >- int r, i, j; >+ int r, i, j, k; > NetClientState *peer; > >+ /* >+ * We will pass the notifiers to the kernel, make sure that QEMU >+ * doesn't interfere. >+ */ >+ for (i = 0; i < nvhosts; i++) { >+ r = virtio_device_grab_ioeventfd(dev); >+ if (r < 0) { >+ error_report("vhost %d binding does not support host notifiers", i); >+ for (k = 0; k < i; k++) { >+ virtio_device_release_ioeventfd(dev); >+ } >+ return r; >+ } >+ } >+ > /* > * Batch all the host notifiers in a single transaction to avoid > * quadratic time complexity in address_space_update_ioeventfds(). >@@ -247,16 +262,6 @@ static int vhost_net_enable_notifiers(VirtIODevice *dev, > > net = get_vhost_net(peer); > hdev = &net->dev; >- /* >- * We will pass the notifiers to the kernel, make sure that QEMU >- * doesn't interfere. >- */ >- r = virtio_device_grab_ioeventfd(dev); >- if (r < 0) { >- error_report("binding does not support host notifiers"); >- memory_region_transaction_commit(); >- goto fail_nvhosts; >- } > > for (j = 0; j < hdev->nvqs; j++) { > r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), >@@ -277,6 +282,14 @@ static int vhost_net_enable_notifiers(VirtIODevice *dev, > return 0; > fail_nvhosts: > vhost_net_disable_notifiers_nvhosts(dev, ncs, data_queue_pairs, i); >+ /* >+ * This for loop starts from i+1, not i, because the i-th ioeventfd >+ * has already been released in vhost_dev_disable_notifiers_nvqs(). >+ */ >+ for (k = i + 1; k < nvhosts; k++) { >+ virtio_device_release_ioeventfd(dev); >+ } >+ > return r; > } > >-- >2.42.0.windows.2 >
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index 997aab0557..891f235a0a 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -229,9 +229,24 @@ static int vhost_net_enable_notifiers(VirtIODevice *dev, int nvhosts = data_queue_pairs + cvq; struct vhost_net *net; struct vhost_dev *hdev; - int r, i, j; + int r, i, j, k; NetClientState *peer; + /* + * We will pass the notifiers to the kernel, make sure that QEMU + * doesn't interfere. + */ + for (i = 0; i < nvhosts; i++) { + r = virtio_device_grab_ioeventfd(dev); + if (r < 0) { + error_report("vhost %d binding does not support host notifiers", i); + for (k = 0; k < i; k++) { + virtio_device_release_ioeventfd(dev); + } + return r; + } + } + /* * Batch all the host notifiers in a single transaction to avoid * quadratic time complexity in address_space_update_ioeventfds(). @@ -247,16 +262,6 @@ static int vhost_net_enable_notifiers(VirtIODevice *dev, net = get_vhost_net(peer); hdev = &net->dev; - /* - * We will pass the notifiers to the kernel, make sure that QEMU - * doesn't interfere. - */ - r = virtio_device_grab_ioeventfd(dev); - if (r < 0) { - error_report("binding does not support host notifiers"); - memory_region_transaction_commit(); - goto fail_nvhosts; - } for (j = 0; j < hdev->nvqs; j++) { r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), @@ -277,6 +282,14 @@ static int vhost_net_enable_notifiers(VirtIODevice *dev, return 0; fail_nvhosts: vhost_net_disable_notifiers_nvhosts(dev, ncs, data_queue_pairs, i); + /* + * This for loop starts from i+1, not i, because the i-th ioeventfd + * has already been released in vhost_dev_disable_notifiers_nvqs(). + */ + for (k = i + 1; k < nvhosts; k++) { + virtio_device_release_ioeventfd(dev); + } + return r; }