From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Wang, Zhihong" Subject: Re: [PATCH v3 4/5] vhost: batch update used ring Date: Thu, 25 Aug 2016 05:19:07 +0000 Message-ID: <8F6C2BD409508844A0EFC19955BE094110776248@SHSMSX103.ccr.corp.intel.com> References: <1471585430-125925-1-git-send-email-zhihong.wang@intel.com> <1471585430-125925-5-git-send-email-zhihong.wang@intel.com> <20160825034800.GW30752@yliu-dev.sh.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" , "maxime.coquelin@redhat.com" To: Yuanhan Liu Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 1A35358F3 for ; Thu, 25 Aug 2016 07:19:10 +0200 (CEST) In-Reply-To: <20160825034800.GW30752@yliu-dev.sh.intel.com> Content-Language: en-US List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > -----Original Message----- > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com] > Sent: Thursday, August 25, 2016 11:48 AM > To: Wang, Zhihong > Cc: dev@dpdk.org; maxime.coquelin@redhat.com > Subject: Re: [PATCH v3 4/5] vhost: batch update used ring >=20 > 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 > ... > > diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-ne= t.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 =3D 0; i < dev->virt_qp_nb; i++) > > - rte_free(dev->virtqueue[i * VIRTIO_QNUM]); > > + for (i =3D 0; i < dev->virt_qp_nb; i++) { > > + vq =3D 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 =3D get_device(vid); > > if (dev =3D=3D NULL) > > return -1; > > > > /* State->index refers to the queue index. The txq is 1, rxq is 0. */ > > - dev->virtqueue[state->index]->size =3D state->num; > > + vq =3D dev->virtqueue[state->index]; > > + vq->size =3D state->num; > > + vq->shadow_used_ring =3D rte_malloc("", > > + vq->size * sizeof(struct vring_used_elem), > > + RTE_CACHE_LINE_SIZE); >=20 > Few notes here: >=20 > - I think the typical way to not specific a string type is using NULL, > but not "". >=20 > - You should check the return value of rte_malloc: it could fail. >=20 > - 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. >=20 > The right way is to free it on get_vring_base(). Good catch! Thanks. >=20 > --yliu