diff mbox

[4/8] virtio: add detach element for packed ring(1.1)

Message ID 1522846444-31725-5-git-send-email-wexu@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Wei Xu April 4, 2018, 12:54 p.m. UTC 
From: Wei Xu <wexu@redhat.com>

helper for packed ring

Signed-off-by: Wei Xu <wexu@redhat.com>
---
 hw/virtio/virtio.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

Comments

Jason Wang April 10, 2018, 7:32 a.m. UTC | #1 
On 2018年04月04日 20:54, wexu@redhat.com wrote:
> From: Wei Xu <wexu@redhat.com>
>
> helper for packed ring

It's odd and hard to review if you put detach patch first. I think this 
patch needs to be reordered after the implementation of pop/map.

Thanks

> Signed-off-by: Wei Xu <wexu@redhat.com>
> ---
>   hw/virtio/virtio.c | 21 +++++++++++++++++++--
>   1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 478df3d..fdee40f 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -561,6 +561,20 @@ static void virtqueue_unmap_sg(VirtQueue *vq, const VirtQueueElement *elem,
>                            elem->out_sg[i].iov_len);
>   }
>   
> +static void virtqueue_detach_element_split(VirtQueue *vq,
> +                            const VirtQueueElement *elem, unsigned int len)
> +{
> +    vq->inuse--;
> +    virtqueue_unmap_sg(vq, elem, len);
> +}
> +
> +static void virtqueue_detach_element_packed(VirtQueue *vq,
> +                            const VirtQueueElement *elem, unsigned int len)
> +{
> +    vq->inuse -= elem->count;
> +    virtqueue_unmap_sg(vq, elem, len);
> +}
> +
>   /* virtqueue_detach_element:
>    * @vq: The #VirtQueue
>    * @elem: The #VirtQueueElement
> @@ -573,8 +587,11 @@ static void virtqueue_unmap_sg(VirtQueue *vq, const VirtQueueElement *elem,
>   void virtqueue_detach_element(VirtQueue *vq, const VirtQueueElement *elem,
>                                 unsigned int len)
>   {
> -    vq->inuse--;
> -    virtqueue_unmap_sg(vq, elem, len);
> +    if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
> +        virtqueue_detach_element_packed(vq, elem, len);
> +    } else {
> +        virtqueue_detach_element_split(vq, elem, len);
> +    }
>   }
>   
>   /* virtqueue_unpop:
Wei Xu June 4, 2018, 1:34 a.m. UTC | #2 
On Tue, Apr 10, 2018 at 03:32:53PM +0800, Jason Wang wrote:
> 
> 
> On 2018年04月04日 20:54, wexu@redhat.com wrote:
> >From: Wei Xu <wexu@redhat.com>
> >
> >helper for packed ring
> 
> It's odd and hard to review if you put detach patch first. I think this
> patch needs to be reordered after the implementation of pop/map.

This patch is not necessary after sync to tiwei's v5, so we can skip it.

Wei

> 
> Thanks
> 
> >Signed-off-by: Wei Xu <wexu@redhat.com>
> >---
> >  hw/virtio/virtio.c | 21 +++++++++++++++++++--
> >  1 file changed, 19 insertions(+), 2 deletions(-)
> >
> >diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> >index 478df3d..fdee40f 100644
> >--- a/hw/virtio/virtio.c
> >+++ b/hw/virtio/virtio.c
> >@@ -561,6 +561,20 @@ static void virtqueue_unmap_sg(VirtQueue *vq, const VirtQueueElement *elem,
> >                           elem->out_sg[i].iov_len);
> >  }
> >+static void virtqueue_detach_element_split(VirtQueue *vq,
> >+                            const VirtQueueElement *elem, unsigned int len)
> >+{
> >+    vq->inuse--;
> >+    virtqueue_unmap_sg(vq, elem, len);
> >+}
> >+
> >+static void virtqueue_detach_element_packed(VirtQueue *vq,
> >+                            const VirtQueueElement *elem, unsigned int len)
> >+{
> >+    vq->inuse -= elem->count;
> >+    virtqueue_unmap_sg(vq, elem, len);
> >+}
> >+
> >  /* virtqueue_detach_element:
> >   * @vq: The #VirtQueue
> >   * @elem: The #VirtQueueElement
> >@@ -573,8 +587,11 @@ static void virtqueue_unmap_sg(VirtQueue *vq, const VirtQueueElement *elem,
> >  void virtqueue_detach_element(VirtQueue *vq, const VirtQueueElement *elem,
> >                                unsigned int len)
> >  {
> >-    vq->inuse--;
> >-    virtqueue_unmap_sg(vq, elem, len);
> >+    if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
> >+        virtqueue_detach_element_packed(vq, elem, len);
> >+    } else {
> >+        virtqueue_detach_element_split(vq, elem, len);
> >+    }
> >  }
> >  /* virtqueue_unpop:
>
Michael S. Tsirkin June 4, 2018, 1:54 a.m. UTC | #3 
On Mon, Jun 04, 2018 at 09:34:35AM +0800, Wei Xu wrote:
> On Tue, Apr 10, 2018 at 03:32:53PM +0800, Jason Wang wrote:
> > 
> > 
> > On 2018年04月04日 20:54, wexu@redhat.com wrote:
> > >From: Wei Xu <wexu@redhat.com>
> > >
> > >helper for packed ring
> > 
> > It's odd and hard to review if you put detach patch first. I think this
> > patch needs to be reordered after the implementation of pop/map.
> 
> This patch is not necessary after sync to tiwei's v5, so we can skip it.
> 
> Wei

I suspect we will need to bring detach back eventually but yes,
it can wait.

> > 
> > Thanks
> > 
> > >Signed-off-by: Wei Xu <wexu@redhat.com>
> > >---
> > >  hw/virtio/virtio.c | 21 +++++++++++++++++++--
> > >  1 file changed, 19 insertions(+), 2 deletions(-)
> > >
> > >diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > >index 478df3d..fdee40f 100644
> > >--- a/hw/virtio/virtio.c
> > >+++ b/hw/virtio/virtio.c
> > >@@ -561,6 +561,20 @@ static void virtqueue_unmap_sg(VirtQueue *vq, const VirtQueueElement *elem,
> > >                           elem->out_sg[i].iov_len);
> > >  }
> > >+static void virtqueue_detach_element_split(VirtQueue *vq,
> > >+                            const VirtQueueElement *elem, unsigned int len)
> > >+{
> > >+    vq->inuse--;
> > >+    virtqueue_unmap_sg(vq, elem, len);
> > >+}
> > >+
> > >+static void virtqueue_detach_element_packed(VirtQueue *vq,
> > >+                            const VirtQueueElement *elem, unsigned int len)
> > >+{
> > >+    vq->inuse -= elem->count;
> > >+    virtqueue_unmap_sg(vq, elem, len);
> > >+}
> > >+
> > >  /* virtqueue_detach_element:
> > >   * @vq: The #VirtQueue
> > >   * @elem: The #VirtQueueElement
> > >@@ -573,8 +587,11 @@ static void virtqueue_unmap_sg(VirtQueue *vq, const VirtQueueElement *elem,
> > >  void virtqueue_detach_element(VirtQueue *vq, const VirtQueueElement *elem,
> > >                                unsigned int len)
> > >  {
> > >-    vq->inuse--;
> > >-    virtqueue_unmap_sg(vq, elem, len);
> > >+    if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
> > >+        virtqueue_detach_element_packed(vq, elem, len);
> > >+    } else {
> > >+        virtqueue_detach_element_split(vq, elem, len);
> > >+    }
> > >  }
> > >  /* virtqueue_unpop:
> >
Wei Xu June 4, 2018, 9:40 a.m. UTC | #4 
On Mon, Jun 04, 2018 at 04:54:45AM +0300, Michael S. Tsirkin wrote:
> On Mon, Jun 04, 2018 at 09:34:35AM +0800, Wei Xu wrote:
> > On Tue, Apr 10, 2018 at 03:32:53PM +0800, Jason Wang wrote:
> > > 
> > > 
> > > On 2018年04月04日 20:54, wexu@redhat.com wrote:
> > > >From: Wei Xu <wexu@redhat.com>
> > > >
> > > >helper for packed ring
> > > 
> > > It's odd and hard to review if you put detach patch first. I think this
> > > patch needs to be reordered after the implementation of pop/map.
> > 
> > This patch is not necessary after sync to tiwei's v5, so we can skip it.
> > 
> > Wei
> 
> I suspect we will need to bring detach back eventually but yes,
> it can wait.

Sure, I reuse the code for 1.0 for now.

Wei

> 
> > > 
> > > Thanks
> > > 
> > > >Signed-off-by: Wei Xu <wexu@redhat.com>
> > > >---
> > > >  hw/virtio/virtio.c | 21 +++++++++++++++++++--
> > > >  1 file changed, 19 insertions(+), 2 deletions(-)
> > > >
> > > >diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > > >index 478df3d..fdee40f 100644
> > > >--- a/hw/virtio/virtio.c
> > > >+++ b/hw/virtio/virtio.c
> > > >@@ -561,6 +561,20 @@ static void virtqueue_unmap_sg(VirtQueue *vq, const VirtQueueElement *elem,
> > > >                           elem->out_sg[i].iov_len);
> > > >  }
> > > >+static void virtqueue_detach_element_split(VirtQueue *vq,
> > > >+                            const VirtQueueElement *elem, unsigned int len)
> > > >+{
> > > >+    vq->inuse--;
> > > >+    virtqueue_unmap_sg(vq, elem, len);
> > > >+}
> > > >+
> > > >+static void virtqueue_detach_element_packed(VirtQueue *vq,
> > > >+                            const VirtQueueElement *elem, unsigned int len)
> > > >+{
> > > >+    vq->inuse -= elem->count;
> > > >+    virtqueue_unmap_sg(vq, elem, len);
> > > >+}
> > > >+
> > > >  /* virtqueue_detach_element:
> > > >   * @vq: The #VirtQueue
> > > >   * @elem: The #VirtQueueElement
> > > >@@ -573,8 +587,11 @@ static void virtqueue_unmap_sg(VirtQueue *vq, const VirtQueueElement *elem,
> > > >  void virtqueue_detach_element(VirtQueue *vq, const VirtQueueElement *elem,
> > > >                                unsigned int len)
> > > >  {
> > > >-    vq->inuse--;
> > > >-    virtqueue_unmap_sg(vq, elem, len);
> > > >+    if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
> > > >+        virtqueue_detach_element_packed(vq, elem, len);
> > > >+    } else {
> > > >+        virtqueue_detach_element_split(vq, elem, len);
> > > >+    }
> > > >  }
> > > >  /* virtqueue_unpop:
> > > 
>
diff mbox

Patch

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 478df3d..fdee40f 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -561,6 +561,20 @@  static void virtqueue_unmap_sg(VirtQueue *vq, const VirtQueueElement *elem,
                          elem->out_sg[i].iov_len);
 }
 
+static void virtqueue_detach_element_split(VirtQueue *vq,
+                            const VirtQueueElement *elem, unsigned int len)
+{
+    vq->inuse--;
+    virtqueue_unmap_sg(vq, elem, len);
+}
+
+static void virtqueue_detach_element_packed(VirtQueue *vq,
+                            const VirtQueueElement *elem, unsigned int len)
+{
+    vq->inuse -= elem->count;
+    virtqueue_unmap_sg(vq, elem, len);
+}
+
 /* virtqueue_detach_element:
  * @vq: The #VirtQueue
  * @elem: The #VirtQueueElement
@@ -573,8 +587,11 @@  static void virtqueue_unmap_sg(VirtQueue *vq, const VirtQueueElement *elem,
 void virtqueue_detach_element(VirtQueue *vq, const VirtQueueElement *elem,
                               unsigned int len)
 {
-    vq->inuse--;
-    virtqueue_unmap_sg(vq, elem, len);
+    if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
+        virtqueue_detach_element_packed(vq, elem, len);
+    } else {
+        virtqueue_detach_element_split(vq, elem, len);
+    }
 }
 
 /* virtqueue_unpop: