Re: [dpdk-dev] [PATCH v7 7/7] vhost: convert inflight data to DPDK allocation API

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

On 6/30/21 9:55 AM, David Marchand wrote:
> On Tue, Jun 29, 2021 at 6:11 PM Maxime Coquelin
> <maxime.coquelin@redhat.com> wrote:
>>
>> Inflight metadata are allocated using glibc's calloc.
>> This patch converts them to rte_zmalloc_socket to take
>> care of the NUMA affinity.
> 
> About the title, maybe:
> vhost: use DPDK allocations for inflight data

Agree with your proposal.

>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> 
> [snip]
> 
>> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
>> index d8ec087dfc..67935c4ccc 100644
>> --- a/lib/vhost/vhost_user.c
>> +++ b/lib/vhost/vhost_user.c
> 
> [snip]
> 
>> @@ -1779,19 +1820,21 @@ vhost_check_queue_inflights_split(struct virtio_net *dev,
>>         vq->last_avail_idx += resubmit_num;
>>
>>         if (resubmit_num) {
>> -               resubmit  = calloc(1, sizeof(struct rte_vhost_resubmit_info));
>> +               resubmit  = rte_zmalloc_socket("resubmit", sizeof(struct rte_vhost_resubmit_info),
> 
> Nit: double space.
> 

Can be fixed while applying.

>> +                               0, vq->numa_node);
>>                 if (!resubmit) {
>>                         VHOST_LOG_CONFIG(ERR,
>>                                 "failed to allocate memory for resubmit info.\n");
>>                         return RTE_VHOST_MSG_RESULT_ERR;
>>                 }
>>
>> -               resubmit->resubmit_list = calloc(resubmit_num,
>> -                       sizeof(struct rte_vhost_resubmit_desc));
>> +               resubmit->resubmit_list = rte_zmalloc_socket("resubmit_list",
>> +                               resubmit_num * sizeof(struct rte_vhost_resubmit_desc),
>> +                               0, vq->numa_node);
>>                 if (!resubmit->resubmit_list) {
>>                         VHOST_LOG_CONFIG(ERR,
>>                                 "failed to allocate memory for inflight desc.\n");
>> -                       free(resubmit);
>> +                       rte_free(resubmit);
>>                         return RTE_VHOST_MSG_RESULT_ERR;
>>                 }
>>
>> @@ -1873,19 +1916,21 @@ vhost_check_queue_inflights_packed(struct virtio_net *dev,
>>         }
>>
>>         if (resubmit_num) {
>> -               resubmit = calloc(1, sizeof(struct rte_vhost_resubmit_info));
>> +               resubmit  = rte_zmalloc_socket("resubmit", sizeof(struct rte_vhost_resubmit_info),
> 
> Copy/paste detected :-)
> Double space.

Indeed!

> Having a single allocator between split and packed implems would avoid
> this, but it might not be that easy and this is out of the scope for
> this patch.
> 

Agree, I think the inflight code may be simpler.
We can think of a refactoring for v21.11.

Thanks,
Maxime

> 
>> +                               0, vq->numa_node);
>>                 if (resubmit == NULL) {
>>                         VHOST_LOG_CONFIG(ERR,
>>                                 "failed to allocate memory for resubmit info.\n");
>>                         return RTE_VHOST_MSG_RESULT_ERR;
>>                 }
>>
>> -               resubmit->resubmit_list = calloc(resubmit_num,
>> -                       sizeof(struct rte_vhost_resubmit_desc));
>> +               resubmit->resubmit_list = rte_zmalloc_socket("resubmit_list",
>> +                               resubmit_num * sizeof(struct rte_vhost_resubmit_desc),
>> +                               0, vq->numa_node);
>>                 if (resubmit->resubmit_list == NULL) {
>>                         VHOST_LOG_CONFIG(ERR,
>>                                 "failed to allocate memory for resubmit desc.\n");
>> -                       free(resubmit);
>> +                       rte_free(resubmit);
>>                         return RTE_VHOST_MSG_RESULT_ERR;
>>                 }
>>
>> --
>> 2.31.1
>>
> 
>