All of lore.kernel.org
 help / color / mirror / Atom feed
From: Didier Pallard <didier.pallard@6wind.com>
To: Victor Kaplansky <victork@redhat.com>, qemu-devel@nongnu.org
Cc: "Thibaut Collet" <thibaut.collet@6wind.com>,
	"Jean-Mickael Guerin" <jmg@6wind.com>,
	pbonzini@redhat.com,
	"Marc-André Lureau" <marcandre.lureau@gmail.com>,
	"Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [Qemu-devel] [RFC PATCH] vhost: fix lost interrupts from slow reacting back-end
Date: Tue, 12 Jan 2016 13:05:52 +0100	[thread overview]
Message-ID: <5694EC20.2040006@6wind.com> (raw)
In-Reply-To: <1452586854-21058-1-git-send-email-victork@redhat.com>

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 <victork@redhat.com>
> ---
>   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?

thanks
didier

-- 
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

  reply	other threads:[~2016-01-12 12:06 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-12  8:26 [Qemu-devel] [RFC PATCH] vhost: fix lost interrupts from slow reacting back-end Victor Kaplansky
2016-01-12 12:05 ` Didier Pallard [this message]
2016-01-13 15:32   ` Victor Kaplansky
2016-01-13 16:44     ` Didier Pallard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5694EC20.2040006@6wind.com \
    --to=didier.pallard@6wind.com \
    --cc=jmg@6wind.com \
    --cc=marcandre.lureau@gmail.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=thibaut.collet@6wind.com \
    --cc=victork@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.