On Thu, Aug 31, 2023 at 9:14 AM Laszlo Ersek <ler...@redhat.com> wrote:
> On 8/30/23 15:40, Laszlo Ersek wrote: > > Cc: "Michael S. Tsirkin" <m...@redhat.com> (supporter:vhost) > > Cc: Eugenio Perez Martin <epere...@redhat.com> > > Cc: German Maglione <gmagli...@redhat.com> > > Cc: Liu Jiang <ge...@linux.alibaba.com> > > Cc: Sergio Lopez Pascual <s...@redhat.com> > > Cc: Stefano Garzarella <sgarz...@redhat.com> > > Signed-off-by: Laszlo Ersek <ler...@redhat.com> > > Reviewed-by: Stefano Garzarella <sgarz...@redhat.com> > > --- > > > > Notes: > > v2: > > > > - pick up Stefano's R-b > > > > hw/virtio/vhost-user.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > This has been > > Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org> > > under the (identical) v1 posting: > > cd0604a1-6826-ac6c-6c47-dcb6def64b28@linaro.org">http://mid.mail-archive.com/cd0604a1-6826-ac6c-6c47-dcb6def64b28@linaro.org > > Thanks, Phil! (and sorry that I posted v2 too quickly -- I forgot that > sometimes reviewers split a review over multiple days.) > > Laszlo > > > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > > index 8dcf049d422b..b4b677c1ce66 100644 > > --- a/hw/virtio/vhost-user.c > > +++ b/hw/virtio/vhost-user.c > > @@ -398,7 +398,7 @@ static int vhost_user_write(struct vhost_dev *dev, > VhostUserMsg *msg, > > * operations such as configuring device memory mappings or issuing > device > > * resets, which affect the whole device instead of individual VQs, > > * vhost-user messages should only be sent once. > > - * > > + * > > * Devices with multiple vhost_devs are given an associated > dev->vq_index > > * so per_device requests are only sent if vq_index is 0. > > */ > > > > Thanks for the series! I had a timeout problem with a virtio device I am developing, and I was not sure yet what was going on. Your description of the problem seemed to fit mine, in my case the driver sent a command through the data plane right after the feature negotiation that reached the backend too soon. Adding delays alleviated the issue, so it already hinted me to a race condition. I tested using this patch series and according to my experiments, it really lowers the chances to get the deadlock. Sadly, I do still get the issue sometimes, though (not frequently)... However, I think probably the solution comes not from the Qemu side, but from the rust-vmm vhost-user-backend crate. I am looking for that solution on my side. But that does not invalidate this patch, which I think is a necessary improvement, and in my tests it really helps a lot with the described issue. Therefore: Tested-by: Albert Esteve <aest...@redhat.com> BR, Albert
