From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34013) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aJOXB-0002Oc-2o for qemu-devel@nongnu.org; Wed, 13 Jan 2016 11:44:34 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aJOX6-0003rb-Ly for qemu-devel@nongnu.org; Wed, 13 Jan 2016 11:44:33 -0500 Received: from mail-wm0-x22b.google.com ([2a00:1450:400c:c09::22b]:38224) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aJOX6-0003rU-Bm for qemu-devel@nongnu.org; Wed, 13 Jan 2016 11:44:28 -0500 Received: by mail-wm0-x22b.google.com with SMTP id b14so381380544wmb.1 for ; Wed, 13 Jan 2016 08:44:27 -0800 (PST) Message-ID: <56967EDA.80407@6wind.com> Date: Wed, 13 Jan 2016 17:44:10 +0100 From: Didier Pallard MIME-Version: 1.0 References: <1452586854-21058-1-git-send-email-victork@redhat.com> <5694EC20.2040006@6wind.com> <20160113164618-mutt-send-email-victork@redhat.com> In-Reply-To: <20160113164618-mutt-send-email-victork@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH] vhost: fix lost interrupts from slow reacting back-end List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Victor Kaplansky Cc: "Michael S. Tsirkin" , Thibaut Collet , Jean-Mickael Guerin , qemu-devel@nongnu.org, =?windows-1252?Q?Marc-Andr=E9_Lureau?= , pbonzini@redhat.com On 01/13/2016 04:32 PM, Victor Kaplansky wrote: > On Tue, Jan 12, 2016 at 01:05:52PM +0100, Didier Pallard wrote: >> On 01/12/2016 09:26 AM, Victor Kaplansky wrote: >>> This RFC PATCH tries to solve the problem of lost interrupts >> >from a slow back-end. Didier could you test it? >>> Thanks, Victor >>> >>> When interrupts are unmasked, it could take some undefined time >>> to the back-end to start routing events to guest_notifier. Till >>> that the events will continue flow to masked_notifier, and some >>> interrupts could be lost. >>> >>> This patch tries to handle the above situation by testing and >>> cleaning both masked_notifier and guest_notifier in >>> guest_notifier read handler. >>> >>> Signed-off-by: Victor Kaplansky >>> --- >>> include/hw/virtio/virtio.h | 1 + >>> hw/virtio/vhost.c | 3 +++ >>> hw/virtio/virtio.c | 14 ++++++++++++++ >>> 3 files changed, 18 insertions(+) >>> >>> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h >>> index 205fadf2..f52b0b6a 100644 >>> --- a/include/hw/virtio/virtio.h >>> +++ b/include/hw/virtio/virtio.h >>> @@ -240,6 +240,7 @@ VirtQueue *virtio_get_queue(VirtIODevice *vdev, int n); >>> uint16_t virtio_get_queue_index(VirtQueue *vq); >>> int virtio_queue_get_id(VirtQueue *vq); >>> EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq); >>> +void virtio_queue_set_masked_guest_notifier(VirtQueue *vq, EventNotifier *n); >>> void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign, >>> bool with_irqfd); >>> EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq); >>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c >>> index de29968a..51ce1532 100644 >>> --- a/hw/virtio/vhost.c >>> +++ b/hw/virtio/vhost.c >>> @@ -854,6 +854,9 @@ static int vhost_virtqueue_start(struct vhost_dev *dev, >>> /* Clear and discard previous events if any. */ >>> event_notifier_test_and_clear(&vq->masked_notifier); >>> + /* Set masked guest_notifier. */ >>> + virtio_queue_set_masked_guest_notifier(vvq, &vq->masked_notifier); >>> + >>> return 0; >>> fail_kick: >>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c >>> index bd6b4df9..d9095c51 100644 >>> --- a/hw/virtio/virtio.c >>> +++ b/hw/virtio/virtio.c >>> @@ -89,6 +89,7 @@ struct VirtQueue >>> VirtIODevice *vdev; >>> EventNotifier guest_notifier; >>> EventNotifier host_notifier; >>> + EventNotifier *masked_guest_notifier; >>> QLIST_ENTRY(VirtQueue) node; >>> }; >>> @@ -1622,6 +1623,14 @@ static void virtio_queue_guest_notifier_read(EventNotifier *n) >>> if (event_notifier_test_and_clear(n)) { >>> virtio_irq(vq); >>> } >>> + /* It could take some time to the backend to switch to >>> + * sending to unmasked evenfd, so we have to test masked >>> + * notifier too. */ >>> + if (vq->masked_guest_notifier) { >>> + if (event_notifier_test_and_clear(vq->masked_guest_notifier)) { >>> + virtio_irq(vq); >>> + } >>> + } >>> } >>> void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign, >>> @@ -1645,6 +1654,11 @@ EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq) >>> return &vq->guest_notifier; >>> } >>> +void virtio_queue_set_masked_guest_notifier(VirtQueue *vq, EventNotifier *n) >>> +{ >>> + vq->masked_guest_notifier = n; >>> +} >>> + >>> static void virtio_queue_host_notifier_read(EventNotifier *n) >>> { >>> VirtQueue *vq = container_of(n, VirtQueue, host_notifier); >> Hi viktor, >> >> i'm wondering how this patch works. >> virtio_queue_guest_notifier_read is only used in >> virtio_queue_set_guest_notifier_fd_handler. >> and it is only used if with_irq is not set: >> if (assign && !with_irqfd) { >> event_notifier_set_handler(&vq->guest_notifier, >> virtio_queue_guest_notifier_read); >> } else { >> event_notifier_set_handler(&vq->guest_notifier, NULL); >> } >> else null handler is set in guest_notifier. >> And from my understanding, virtio-pci in kvm mode uses irqfd, so when are >> we entering the virtio_queue_guest_notifier_read? >> did you also change the qemu configuration? >> > Hmm, right. So, probably it would be better to take your version > of the fix as a temporarily solution which just disables the > ability to mask interrupts when virtio-net-pci is backed by > vhost-user. > > Well, it does not completely disable the ability to mask interrupts: interrupt masking is directly done by qemu (that set/unset eventfd in kvm to unmask/mask interrupts) rather than by vhost-user backend. This allows to be sure that interrupts are correctly masked[unmasked] on return of virtio_pci_vq_vector_mask[unmask] function, (which is not the case when a message is sent through the vhost-user linux socket) but my patch was only tested with a single platform configuration. I don't know if it behaves well with non pci buses, for example. -- Didier PALLARD 6WIND Software Engineer Tel: +33 1 39 30 92 46 Mob: +33 6 49 11 40 14 Fax: +33 1 39 30 92 11 didier.pallard@6wind.com www.6wind.com