Re: [dpdk-dev] [PATCH v4 5/7] vhost: improve NUMA reallocation

[Maxime Coquelin <maxime.coquelin@redhat.com>]

On 6/18/21 6:34 AM, Xia, Chenbo wrote:
> Hi Maxime,
> 
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Thursday, June 17, 2021 11:38 PM
>> To: dev@dpdk.org; david.marchand@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>
>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Subject: [PATCH v4 5/7] vhost: improve NUMA reallocation
>>
>> This patch improves the numa_realloc() function by making use
>> of rte_realloc_socket(), which takes care of the memory copy
>> and freeing of the old data.
>>
>> Suggested-by: David Marchand <david.marchand@redhat.com>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>>  lib/vhost/vhost_user.c | 195 ++++++++++++++++++-----------------------
>>  1 file changed, 86 insertions(+), 109 deletions(-)
>>
>> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
>> index 6e7b327ef8..0590ef6d14 100644
>> --- a/lib/vhost/vhost_user.c
>> +++ b/lib/vhost/vhost_user.c
>> @@ -480,144 +480,121 @@ vhost_user_set_vring_num(struct virtio_net **pdev,
>>  static struct virtio_net*
>>  numa_realloc(struct virtio_net *dev, int index)
>>  {
>> -	int oldnode, newnode;
>> +	int node, dev_node;
>>  	struct virtio_net *old_dev;
>> -	struct vhost_virtqueue *old_vq, *vq;
>> -	struct vring_used_elem *new_shadow_used_split;
>> -	struct vring_used_elem_packed *new_shadow_used_packed;
>> -	struct batch_copy_elem *new_batch_copy_elems;
>> +	struct vhost_virtqueue *vq;
>> +	struct batch_copy_elem *bce;
>> +	struct guest_page *gp;
>> +	struct rte_vhost_memory *mem;
>> +	size_t mem_size;
>>  	int ret;
>>
>>  	old_dev = dev;
>> -	vq = old_vq = dev->virtqueue[index];
>> -
>> -	ret = get_mempolicy(&newnode, NULL, 0, old_vq->desc,
>> -			    MPOL_F_NODE | MPOL_F_ADDR);
>> +	vq = dev->virtqueue[index];
>>
>> -	/* check if we need to reallocate vq */
>> -	ret |= get_mempolicy(&oldnode, NULL, 0, old_vq,
>> -			     MPOL_F_NODE | MPOL_F_ADDR);
>> +	ret = get_mempolicy(&node, NULL, 0, vq->desc, MPOL_F_NODE | MPOL_F_ADDR);
>>  	if (ret) {
>> -		VHOST_LOG_CONFIG(ERR,
>> -			"Unable to get vq numa information.\n");
>> +		VHOST_LOG_CONFIG(ERR, "Unable to get virtqueue %d numa
>> information.\n", index);
>>  		return dev;
>>  	}
>> -	if (oldnode != newnode) {
>> -		if (vq->ready) {
>> -			vq->ready = false;
>> -			vhost_user_notify_queue_state(dev, index, 0);
>> -		}
>>
>> -		VHOST_LOG_CONFIG(INFO,
>> -			"reallocate vq from %d to %d node\n", oldnode, newnode);
>> -		vq = rte_malloc_socket(NULL, sizeof(*vq), 0, newnode);
>> -		if (!vq)
>> -			return dev;
>> +	if (vq->ready) {
>> +		vq->ready = false;
>> +		vhost_user_notify_queue_state(dev, index, 0);
>> +	}
>>
>> -		memcpy(vq, old_vq, sizeof(*vq));
>> +	vq = rte_realloc_socket(vq, sizeof(*vq), 0, node);
>> +	if (!vq) {
>> +		VHOST_LOG_CONFIG(ERR, "Failed to realloc virtqueue %d on
>> node %d\n",
>> +				index, node);
>> +		return dev;
>> +	}
>>
>> -		if (vq_is_packed(dev)) {
>> -			new_shadow_used_packed = rte_malloc_socket(NULL,
>> -					vq->size *
>> -					sizeof(struct vring_used_elem_packed),
>> -					RTE_CACHE_LINE_SIZE,
>> -					newnode);
>> -			if (new_shadow_used_packed) {
>> -				rte_free(vq->shadow_used_packed);
>> -				vq->shadow_used_packed = new_shadow_used_packed;
>> -			}
>> -		} else {
>> -			new_shadow_used_split = rte_malloc_socket(NULL,
>> -					vq->size *
>> -					sizeof(struct vring_used_elem),
>> -					RTE_CACHE_LINE_SIZE,
>> -					newnode);
>> -			if (new_shadow_used_split) {
>> -				rte_free(vq->shadow_used_split);
>> -				vq->shadow_used_split = new_shadow_used_split;
>> -			}
>> -		}
>> +	if (vq != dev->virtqueue[index]) {
>> +		VHOST_LOG_CONFIG(INFO, "reallocated virtqueue on node %d\n", node);
>> +		dev->virtqueue[index] = vq;
>> +		vhost_user_iotlb_init(dev, index);
>> +	}
>> +
>> +	if (vq_is_packed(dev)) {
>> +		struct vring_used_elem_packed *sup;
>>
>> -		new_batch_copy_elems = rte_malloc_socket(NULL,
>> -			vq->size * sizeof(struct batch_copy_elem),
>> -			RTE_CACHE_LINE_SIZE,
>> -			newnode);
>> -		if (new_batch_copy_elems) {
>> -			rte_free(vq->batch_copy_elems);
>> -			vq->batch_copy_elems = new_batch_copy_elems;
>> +		sup = rte_realloc_socket(vq->shadow_used_packed, vq->size *
>> sizeof(*sup),
>> +				RTE_CACHE_LINE_SIZE, node);
>> +		if (!sup) {
>> +			VHOST_LOG_CONFIG(ERR, "Failed to realloc shadow packed on
>> node %d\n", node);
>> +			return dev;
>>  		}
>> +		vq->shadow_used_packed = sup;
>>
> 
> Above blank line could be deleted?

OK, will do.

Thanks,
Maxime

> Thanks,
> Chenbo
>