On 28/10/2021 23:17, Dr. David Alan Gilbert wrote: > * Li Zhijian (lizhij...@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 > >