From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 8BBE28D8F for ; Tue, 22 Dec 2015 07:51:30 +0100 (CET) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga101.jf.intel.com with ESMTP; 21 Dec 2015 22:51:11 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,463,1444719600"; d="scan'208";a="876541381" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.66.49]) by orsmga002.jf.intel.com with ESMTP; 21 Dec 2015 22:51:10 -0800 Date: Tue, 22 Dec 2015 14:52:24 +0800 From: Yuanhan Liu To: "Xie, Huawei" Message-ID: <20151222065224.GN18863@yliu-dev.sh.intel.com> References: <1450422247-6814-1-git-send-email-yuanhan.liu@linux.intel.com> <1450422247-6814-2-git-send-email-yuanhan.liu@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH 2/3] vhost: simplify numa_realloc X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 22 Dec 2015 06:51:30 -0000 On Tue, Dec 22, 2015 at 06:46:32AM +0000, Xie, Huawei wrote: > On 12/18/2015 3:03 PM, Yuanhan Liu wrote: > > We could first check if we need realloc vq or not, if so, > > reallocate it. We then do similar to vhost dev realloc. > > > > This could get rid of the tons of repeated "if (realloc_dev)" > > and "if (realloc_vq)" statements, therefore, makes code > > a bit more readable. > > > > Signed-off-by: Yuanhan Liu > > --- > > lib/librte_vhost/virtio-net.c | 77 ++++++++++++++++++++----------------------- > > 1 file changed, 36 insertions(+), 41 deletions(-) > > > > diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c > > index 2f83438..31ca4f7 100644 > > --- a/lib/librte_vhost/virtio-net.c > > +++ b/lib/librte_vhost/virtio-net.c > > @@ -441,64 +441,59 @@ static struct virtio_net* > > numa_realloc(struct virtio_net *dev, int index) > > { > > int oldnode, newnode; > > - struct virtio_net *old_dev, *new_dev = NULL; > > - struct vhost_virtqueue *old_vq, *new_vq = NULL; > > + struct virtio_net *old_dev; > > + struct vhost_virtqueue *old_vq, *vq; > > int ret; > > - int realloc_dev = 0, realloc_vq = 0; > > > > old_dev = dev; > > - old_vq = dev->virtqueue[index]; > > + vq = old_vq = dev->virtqueue[index]; > > > > - ret = get_mempolicy(&newnode, NULL, 0, old_vq->desc, > > - MPOL_F_NODE | MPOL_F_ADDR); > > - ret = ret | get_mempolicy(&oldnode, NULL, 0, old_dev, > > + ret = get_mempolicy(&newnode, NULL, 0, old_vq->desc, > > MPOL_F_NODE | MPOL_F_ADDR); > > + > > + /* check if we need to reallocate vq */ > > + ret = get_mempolicy(&oldnode, NULL, 0, old_vq, MPOL_F_NODE | MPOL_F_ADDR); > > Why remove the ret = ret | ? Both get_mempolicy could fail. Right, will fix it. > > > if (ret) { > > RTE_LOG(ERR, VHOST_CONFIG, > > - "Unable to get vring desc or dev numa information.\n"); > > + "Unable to get vq numa information.\n"); > > return dev; > > } > > - if (oldnode != newnode) > > - realloc_dev = 1; > > + if (oldnode != newnode) { > > + RTE_LOG(INFO, VHOST_CONFIG, > > + "reallocate vq from %d to %d node\n", oldnode, newnode); > > + vq = rte_malloc_socket(NULL, sizeof(*vq), 0, newnode); > > + if (!vq) > > + return dev; > > + > > + memcpy(vq, old_vq, sizeof(*vq)); > > + rte_free(old_vq); > > + } > > > > - ret = get_mempolicy(&oldnode, NULL, 0, old_vq, > > - MPOL_F_NODE | MPOL_F_ADDR); > > + /* check if we need to reallocate dev */ > > + ret = get_mempolicy(&oldnode, NULL, 0, old_dev, MPOL_F_NODE | MPOL_F_ADDR); > > if (ret) { > > RTE_LOG(ERR, VHOST_CONFIG, > > - "Unable to get vq numa information.\n"); > > - return dev; > > + "Unable to get vring desc or dev numa information.\n"); > > + goto out; > > } > > Why vring desc in the err message? Oops, no idea why I did that. Will fix it. --yliu