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

["Xie, Huawei" <huawei.xie@intel.com>]

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.

>  	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?
-huawei
[...]