From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Yang, Zhiyong" Subject: Re: [PATCH] vhost: do deep copy while reallocate vq Date: Wed, 17 Jan 2018 01:36:58 +0000 Message-ID: References: <1516015939-11266-1-git-send-email-junjie.j.chen@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" To: "Chen, Junjie J" , "yliu@fridaylinux.org" , "maxime.coquelin@redhat.com" Return-path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id E441E1B29B for ; Wed, 17 Jan 2018 02:37:04 +0100 (CET) In-Reply-To: Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > -----Original Message----- > From: Chen, Junjie J > Sent: Tuesday, January 16, 2018 3:39 PM > To: Yang, Zhiyong ; yliu@fridaylinux.org; > maxime.coquelin@redhat.com > Cc: dev@dpdk.org > Subject: RE: [dpdk-dev] [PATCH] vhost: do deep copy while reallocate vq >=20 > Hi > > > > > @@ -227,6 +227,7 @@ vhost_user_set_vring_num(struct virtio_net > > *dev, > > > > > "zero copy is force disabled\n"); > > > > > dev->dequeue_zero_copy =3D 0; > > > > > } > > > > > + TAILQ_INIT(&vq->zmbuf_list); > > > > > } > > > > > > > > > > vq->shadow_used_ring =3D rte_malloc(NULL, @@ -261,6 > +262,9 > > @@ > > > > > numa_realloc(struct virtio_net *dev, int index) > > > > > int oldnode, newnode; > > > > > struct virtio_net *old_dev; > > > > > struct vhost_virtqueue *old_vq, *vq; > > > > > + struct zcopy_mbuf *new_zmbuf; > > > > > + struct vring_used_elem *new_shadow_used_ring; > > > > > + struct batch_copy_elem *new_batch_copy_elems; > > > > > int ret; > > > > > > > > > > old_dev =3D dev; > > > > > @@ -285,6 +289,33 @@ numa_realloc(struct virtio_net *dev, int > > index) > > > > > return dev; > > > > > > > > > > memcpy(vq, old_vq, sizeof(*vq)); > > > > > + TAILQ_INIT(&vq->zmbuf_list); > > > > > + > > > > > + new_zmbuf =3D rte_malloc_socket(NULL, vq- > >zmbuf_size * > > > > > + sizeof(struct zcopy_mbuf), 0, newnode); > > > > > + if (new_zmbuf) { > > > > > + rte_free(vq->zmbufs); > > > > > + vq->zmbufs =3D new_zmbuf; > > > > > + } > > > > > > > > You need to consider how to handle the case ( rte_malloc_socket > > > > return NULL). > > > > > > If it failed to allocate new_zmbuf, it uses old zmbufs, so as to > > > keep vhost alive. > > > > It sounds reasonable, another question is, for the 3 blocks of memory > > being allocated, If some succeed , others fails, Does it mean that > > the code will run on different socket? What's the perf impact if it ha= ppens. >=20 > The original code doesn't do deep copy and thus access memory on differen= t > socket, this patch is to mitigate this situation. It does access remote m= emory > when one of above allocation failed. >=20 > I saw some performance improvement (24.8Gbits/s -> 26.1Gbit/s) on my dev > machine when only reallocate for zmbufs, while I didn't see significant > performance difference when allocating vring_used_elem and > batch_copy_elem. Great,=20 Reviewed-by: Zhiyong Yang =20 Thanks Zhiyong