Re: [Qemu-devel] [RFC PATCH] vhost: fix lost interrupts from slow reacting back-end

[Didier Pallard <didier.pallard@6wind.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 <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?
>>
> 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