On 03/09/2019 12:31, Michal Kalderon wrote:
>> From: linux-rdma-owner@xxxxxxxxxxxxxxx <linux-rdma-
>> owner@xxxxxxxxxxxxxxx> On Behalf Of Gal Pressman
>>
>> On 02/09/2019 19:23, Michal Kalderon wrote:
>>> int efa_destroy_qp(struct ib_qp *ibqp, struct ib_udata *udata) {
>>> + struct efa_ucontext *ucontext =
>> rdma_udata_to_drv_context(udata,
>>> + struct efa_ucontext, ibucontext);
>>> struct efa_dev *dev = to_edev(ibqp->pd->device);
>>> struct efa_qp *qp = to_eqp(ibqp);
>>> int err;
>>>
>>> ibdev_dbg(&dev->ibdev, "Destroy qp[%u]\n", ibqp->qp_num);
>>> +
>>> + efa_qp_user_mmap_entries_remove(ucontext, qp);
>>> +
>>> err = efa_destroy_qp_handle(dev, qp->qp_handle);
>>> if (err)
>>> return err;
>>> @@ -509,57 +412,114 @@ int efa_destroy_qp(struct ib_qp *ibqp, struct
>> ib_udata *udata)
>>> return 0;
>>> }
>>>
>>> void efa_destroy_cq(struct ib_cq *ibcq, struct ib_udata *udata) {
>>> + struct efa_ucontext *ucontext =
>> rdma_udata_to_drv_context(udata,
>>> + struct efa_ucontext, ibucontext);
>>> +
>>> struct efa_dev *dev = to_edev(ibcq->device);
>>> struct efa_cq *cq = to_ecq(ibcq);
>>>
>>> @@ -897,17 +862,28 @@ void efa_destroy_cq(struct ib_cq *ibcq, struct
>> ib_udata *udata)
>>> efa_destroy_cq_idx(dev, cq->cq_idx);
>>> dma_unmap_single(&dev->pdev->dev, cq->dma_addr, cq->size,
>>> DMA_FROM_DEVICE);
>>> + rdma_user_mmap_entry_remove(&ucontext->ibucontext,
>>> + cq->mmap_key);
>>
>> How come in destroy_qp we do entry removal first, but in destroy_cq it's
>> last?
>> Shouldn't it be the same?
> Yes, you're right, it should be done after memory is unmapped, I'll move it down
> In the destroy qp flow. Is this the only comment on this series ?
Other than that the patch looks good to me,
Acked-by: Gal Pressman <galpress@xxxxxxxxxx>
A few nits (feel free to ignore):
* The rdma_user_mmap_entry is always referred to as rdma_entry except in
efa_mmap_free declaration and to_emmap.
* efa_qp_user_mmap_entries_remove isn't really in reverse insertion order but OK :).
* In case of length mismatch in __efa_mmap two error messages are printed,
consider keeping the "Couldn't mmap address ..." print in the if (not part of
the goto label).
Thanks for doing this!
