> -----Original Message----- > From: Yuanhan Liu [mailto:yuanhan.liu at linux.intel.com] > Sent: Thursday, August 25, 2016 11:48 AM > To: Wang, Zhihong <zhihong.wang at intel.com> > Cc: dev at dpdk.org; maxime.coquelin at redhat.com > Subject: Re: [PATCH v3 4/5] vhost: batch update used ring > > On Fri, Aug 19, 2016 at 01:43:49AM -0400, Zhihong Wang wrote: > > This patch enables batch update of the used ring for better efficiency. > > > > Signed-off-by: Zhihong Wang <zhihong.wang at intel.com> > ... > > diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c > > index 1785695..87d09fa 100644 > > --- a/lib/librte_vhost/virtio-net.c > > +++ b/lib/librte_vhost/virtio-net.c > > @@ -152,10 +152,14 @@ cleanup_device(struct virtio_net *dev, int > destroy) > > static void > > free_device(struct virtio_net *dev) > > { > > + struct vhost_virtqueue *vq; > > uint32_t i; > > > > - for (i = 0; i < dev->virt_qp_nb; i++) > > - rte_free(dev->virtqueue[i * VIRTIO_QNUM]); > > + for (i = 0; i < dev->virt_qp_nb; i++) { > > + vq = dev->virtqueue[i * VIRTIO_QNUM]; > > + rte_free(vq->shadow_used_ring); > > + rte_free(vq); > > + } > > rte_free(dev); > > } > > @@ -418,13 +422,18 @@ int > > vhost_set_vring_num(int vid, struct vhost_vring_state *state) > > { > > struct virtio_net *dev; > > + struct vhost_virtqueue *vq; > > > > dev = get_device(vid); > > if (dev == NULL) > > return -1; > > > > /* State->index refers to the queue index. The txq is 1, rxq is 0. */ > > - dev->virtqueue[state->index]->size = state->num; > > + vq = dev->virtqueue[state->index]; > > + vq->size = state->num; > > + vq->shadow_used_ring = rte_malloc("", > > + vq->size * sizeof(struct vring_used_elem), > > + RTE_CACHE_LINE_SIZE); > > Few notes here: > > - I think the typical way to not specific a string type is using NULL, > but not "". > > - You should check the return value of rte_malloc: it could fail. > > - Note that free_device() is invoked only when the vhost-user connection > is broken (say the guest is halt). However, vhost_set_vring_num() could > be invoked many times for a connection, say when you restart testpmd > many times. This would lead to memory leak. > > The right way is to free it on get_vring_base().
Good catch! Thanks. > > --yliu
