Re: [Qemu-devel] Regression: virtio-pci: convert to ioeventfd callbacks

[Cornelia Huck]

On Tue, 28 Jun 2016 09:47:01 +0200
Peter Lieven <address@hidden> wrote:

> Am 28.06.2016 um 09:42 schrieb Cornelia Huck:
> > On Tue, 28 Jun 2016 10:03:21 +0300
> > "Michael S. Tsirkin" <address@hidden> wrote:
> >
> >> I notice cleanup is a bit weird:
> >>
> >>          virtio_queue_set_host_notifier_fd_handler(vq, false, false);
> >>          k->ioeventfd_assign(proxy, notifier, n, assign);
> >>          event_notifier_cleanup(notifier);
> >>
> >> I think virtio_queue_set_host_notifier_fd_handler should happen
> >> after ioeventfd_assign for symmetry with init?
> > Looking at the pre-rework code, ccw used the order now in common code,
> > while pci and mmio used the order you suggest.
> >
> > "Switch the handler back, then unassign the transport's ioeventfd
> > backing" made more sense to me (regardless of symmetry) - but we might
> > lose a notification?
> >
> > Peter: Can you check whether your problem goes away if you switch the
> > two lines around?
> >
> 
> The problem goes away, but its horribly slow. Maybe the lost notifications
> you were thinking off.

It points into that direction.

> 
> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
> index 1313760..7924a59 100644
> --- a/hw/virtio/virtio-bus.c
> +++ b/hw/virtio/virtio-bus.c
> @@ -176,9 +176,9 @@ static int set_host_notifier_internal(DeviceState *proxy, 
> VirtioBusState *bus,
>               return r;
>           }
>       } else {
> -        virtio_queue_set_host_notifier_fd_handler(vq, false, false);
>           k->ioeventfd_assign(proxy, notifier, n, assign);

The virtio_queue_...() should go here.

>           event_notifier_cleanup(notifier);
> +        virtio_queue_set_host_notifier_fd_handler(vq, false, false);
>       }
>       return r;
>   }
> 
> Peter
>