diff mbox

virtio/vhost: drop unnecessary VHOST_SET_VRING call

Message ID 1439374791-25879-1-git-send-email-marcel@redhat.com
State New
Headers show

Commit Message

Marcel Apfelbaum Aug. 12, 2015, 10:19 a.m. UTC
No need to send VHOST_SET_VRING_CALL to backend
before the negotiation with the guest is finished.

Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
 hw/virtio/vhost.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

Comments

Michael S. Tsirkin Aug. 12, 2015, 10:34 a.m. UTC | #1
On Wed, Aug 12, 2015 at 01:19:51PM +0300, Marcel Apfelbaum wrote:
> No need to send VHOST_SET_VRING_CALL to backend
> before the negotiation with the guest is finished.
> 
> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>

Well - we do need to set it to the masked notifier initially to avoid
losing events.  You can't just drop it - need to move this call
somewhere else.

> ---
>  hw/virtio/vhost.c | 13 +------------
>  1 file changed, 1 insertion(+), 12 deletions(-)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 2712c6f..b448542 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -875,24 +875,13 @@ static void vhost_eventfd_del(MemoryListener *listener,
>  static int vhost_virtqueue_init(struct vhost_dev *dev,
>                                  struct vhost_virtqueue *vq, int n)
>  {
> -    struct vhost_vring_file file = {
> -        .index = n,
> -    };
>      int r = event_notifier_init(&vq->masked_notifier, 0);
> +
>      if (r < 0) {
>          return r;
>      }
>  
> -    file.fd = event_notifier_get_fd(&vq->masked_notifier);
> -    r = dev->vhost_ops->vhost_call(dev, VHOST_SET_VRING_CALL, &file);
> -    if (r) {
> -        r = -errno;
> -        goto fail_call;
> -    }
>      return 0;
> -fail_call:
> -    event_notifier_cleanup(&vq->masked_notifier);
> -    return r;
>  }
>  
>  static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq)
> -- 
> 2.1.0
Marcel Apfelbaum Aug. 12, 2015, 11:10 a.m. UTC | #2
On 08/12/2015 01:34 PM, Michael S. Tsirkin wrote:
> On Wed, Aug 12, 2015 at 01:19:51PM +0300, Marcel Apfelbaum wrote:
>> No need to send VHOST_SET_VRING_CALL to backend
>> before the negotiation with the guest is finished.
>>
>> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
>
> Well - we do need to set it to the masked notifier initially to avoid
> losing events.  You can't just drop it - need to move this call
> somewhere else.
What do we need to set?
I just dropped the call to VHOST_SET_VRING_CALL.

Thanks,
Marcel

>
>> ---
>>   hw/virtio/vhost.c | 13 +------------
>>   1 file changed, 1 insertion(+), 12 deletions(-)
>>
>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>> index 2712c6f..b448542 100644
>> --- a/hw/virtio/vhost.c
>> +++ b/hw/virtio/vhost.c
>> @@ -875,24 +875,13 @@ static void vhost_eventfd_del(MemoryListener *listener,
>>   static int vhost_virtqueue_init(struct vhost_dev *dev,
>>                                   struct vhost_virtqueue *vq, int n)
>>   {
>> -    struct vhost_vring_file file = {
>> -        .index = n,
>> -    };
>>       int r = event_notifier_init(&vq->masked_notifier, 0);
>> +
>>       if (r < 0) {
>>           return r;
>>       }
>>
>> -    file.fd = event_notifier_get_fd(&vq->masked_notifier);
>> -    r = dev->vhost_ops->vhost_call(dev, VHOST_SET_VRING_CALL, &file);
>> -    if (r) {
>> -        r = -errno;
>> -        goto fail_call;
>> -    }
>>       return 0;
>> -fail_call:
>> -    event_notifier_cleanup(&vq->masked_notifier);
>> -    return r;
>>   }
>>
>>   static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq)
>> --
>> 2.1.0
Michael S. Tsirkin Aug. 12, 2015, 11:15 a.m. UTC | #3
On Wed, Aug 12, 2015 at 02:10:56PM +0300, Marcel Apfelbaum wrote:
> On 08/12/2015 01:34 PM, Michael S. Tsirkin wrote:
> >On Wed, Aug 12, 2015 at 01:19:51PM +0300, Marcel Apfelbaum wrote:
> >>No need to send VHOST_SET_VRING_CALL to backend
> >>before the negotiation with the guest is finished.
> >>
> >>Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
> >
> >Well - we do need to set it to the masked notifier initially to avoid
> >losing events.  You can't just drop it - need to move this call
> >somewhere else.
> What do we need to set?
> I just dropped the call to VHOST_SET_VRING_CALL.
> 
> Thanks,
> Marcel

We use two eventfds: masked and unmasked one.
We switch dynamically dependent on msi mask value.
Code assumes we start out masked, so we need to match that.


> >
> >>---
> >>  hw/virtio/vhost.c | 13 +------------
> >>  1 file changed, 1 insertion(+), 12 deletions(-)
> >>
> >>diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> >>index 2712c6f..b448542 100644
> >>--- a/hw/virtio/vhost.c
> >>+++ b/hw/virtio/vhost.c
> >>@@ -875,24 +875,13 @@ static void vhost_eventfd_del(MemoryListener *listener,
> >>  static int vhost_virtqueue_init(struct vhost_dev *dev,
> >>                                  struct vhost_virtqueue *vq, int n)
> >>  {
> >>-    struct vhost_vring_file file = {
> >>-        .index = n,
> >>-    };
> >>      int r = event_notifier_init(&vq->masked_notifier, 0);
> >>+
> >>      if (r < 0) {
> >>          return r;
> >>      }
> >>
> >>-    file.fd = event_notifier_get_fd(&vq->masked_notifier);
> >>-    r = dev->vhost_ops->vhost_call(dev, VHOST_SET_VRING_CALL, &file);
> >>-    if (r) {
> >>-        r = -errno;
> >>-        goto fail_call;
> >>-    }
> >>      return 0;
> >>-fail_call:
> >>-    event_notifier_cleanup(&vq->masked_notifier);
> >>-    return r;
> >>  }
> >>
> >>  static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq)
> >>--
> >>2.1.0
diff mbox

Patch

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 2712c6f..b448542 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -875,24 +875,13 @@  static void vhost_eventfd_del(MemoryListener *listener,
 static int vhost_virtqueue_init(struct vhost_dev *dev,
                                 struct vhost_virtqueue *vq, int n)
 {
-    struct vhost_vring_file file = {
-        .index = n,
-    };
     int r = event_notifier_init(&vq->masked_notifier, 0);
+
     if (r < 0) {
         return r;
     }
 
-    file.fd = event_notifier_get_fd(&vq->masked_notifier);
-    r = dev->vhost_ops->vhost_call(dev, VHOST_SET_VRING_CALL, &file);
-    if (r) {
-        r = -errno;
-        goto fail_call;
-    }
     return 0;
-fail_call:
-    event_notifier_cleanup(&vq->masked_notifier);
-    return r;
 }
 
 static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq)