Re: [PATCH v3] migration/rdma: Fix out of order wrid

[address@hidden]

On 28/10/2021 23:17, Dr. David Alan Gilbert wrote:
> * Li Zhijian (lizhijian@cn.fujitsu.com) wrote:
>
> Apologies for taking so long.
It's okay :), thanks for your review.

>
>>       /*
>> -     * Completion queue can be filled by both read and write work requests,
>> -     * so must reflect the sum of both possible queue sizes.
>> +     * Completion queue can be filled by read work requests.
>>        */
>> -    rdma->cq = ibv_create_cq(rdma->verbs, (RDMA_SIGNALED_SEND_MAX * 3),
>> -            NULL, rdma->comp_channel, 0);
>> -    if (!rdma->cq) {
>> +    rdma->recv_cq = ibv_create_cq(rdma->verbs, (RDMA_SIGNALED_SEND_MAX * 3),
>> +                                  NULL, rdma->recv_comp_channel, 0);
>> +    if (!rdma->recv_cq) {
>> +        error_report("failed to allocate completion queue");
> Minor: It would be good to make this different from the error below;
> e.g. 'failed to allocate receive completion queue'

Good catch, i will amend them soon.


>
>> +        goto err_alloc_pd_cq;
>> +    }
>> +
>> +    /* create send completion channel */
>> +    rdma->send_comp_channel = ibv_create_comp_channel(rdma->verbs);
>> +    if (!rdma->send_comp_channel) {
>> +        error_report("failed to allocate completion channel");
>> +        goto err_alloc_pd_cq;
>> +    }
>> +
>> +    rdma->send_cq = ibv_create_cq(rdma->verbs, (RDMA_SIGNALED_SEND_MAX * 3),
>> +                                  NULL, rdma->send_comp_channel, 0);
>> +    if (!rdma->send_cq) {
>>           error_report("failed to allocate completion queue");
>>           goto err_alloc_pd_cq;
>>       }
>> @@ -1083,11 +1098,19 @@ err_alloc_pd_cq:
>>       if (rdma->pd) {
>>           ibv_dealloc_pd(rdma->pd);
>>       }
>> -    if (rdma->comp_channel) {
>> -        ibv_destroy_comp_channel(rdma->comp_channel);
>> +    if (rdma->recv_comp_channel) {
>> +        ibv_destroy_comp_channel(rdma->recv_comp_channel);
>> +    }
>> +    if (rdma->send_comp_channel) {
>> +        ibv_destroy_comp_channel(rdma->send_comp_channel);
>> +    }
>> +    if (rdma->recv_cq) {
>> +        ibv_destroy_cq(rdma->recv_cq);
>> +        rdma->recv_cq = NULL;
>>       }
> Don't you need to destroy the send_cq as well?

we don't need to do that since send_cq is that last element we allot, that means
send_cq will always be NULL once the code reaches here.

Thanks
Zhijian

>
> (Other than that I think it's fine)
>
> Dave
>
>