Re: [dpdk-dev] [PATCH 2/3] vhost: simplify numa_realloc

[Yuanhan Liu <yuanhan.liu@linux.intel.com>]

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 <yuanhan.liu@linux.intel.com>
> > ---
> >  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