Re: [PATCHv6 2/3] IB/core: added support to use rdma cgroup controller

Parav Pandit
2016-02-24T16:05:00Z

On Wed, Feb 24, 2016 at 7:13 PM, Haggai Eran <haggaie@mellanox.com> wrote:

>> + * all the resources are deallocated, and after a stage when any
>> + * other resource allocation of user application cannot be done
>> + * for this device to avoid any leak in accounting.
>> + * HCA drivers should clear resource pool ops after ib stack
>> + * unregisters with rdma cgroup.
> HCA drivers don't supply their own ops in this version, right?
> If so, you can update the comment.
Yes. I will update.

>> +
> I'm not sure I understand why you need to keep the PD here. Why
> don't you use the same ib_device that is used to create the QP?
> The same applies comment also applies to other uverbs commands.

My bad. Left out of previous implementation. I have changed it now.

>
>> if (cmd->qp_type == IB_QPT_XRC_TGT) {
>> xrcd = idr_read_xrcd(cmd->pd_handle, file->ucontext,
>> &xrcd_uobj);
>> @@ -1811,8 +1891,7 @@ static int create_qp(struct ib_uverbs_file *file,
>>
>> scq = idr_read_cq(cmd->send_cq_handle, file->ucontext, !!rcq);
>> rcq = rcq ?: scq;
>> - pd = idr_read_pd(cmd->pd_handle, file->ucontext);
>> - if (!pd || !scq) {
>> + if (!scq) {
>> ret = -EINVAL;
>> goto err_put;
>> }
>> @@ -1858,6 +1937,11 @@ static int create_qp(struct ib_uverbs_file *file,
>> goto err_put;
>> }
>>
>> + ret = ib_rdmacg_try_charge(&obj->uevent.uobject.cg_obj, pd->device,
>> + RDMA_VERB_RESOURCE_QP, 1);
>> + if (ret)
>> + goto err_put;
>> +
>> if (cmd->qp_type == IB_QPT_XRC_TGT)
>> qp = ib_create_qp(pd, &attr);
>> else
>> @@ -1865,7 +1949,7 @@ static int create_qp(struct ib_uverbs_file *file,
>>
>> if (IS_ERR(qp)) {
>> ret = PTR_ERR(qp);
>> - goto err_put;
>> + goto err_create;
>> }
>>
>> if (cmd->qp_type != IB_QPT_XRC_TGT) {
>> @@ -1940,6 +2024,10 @@ err_cb:
>> err_destroy:
>> ib_destroy_qp(qp);
>>
>> +err_create:
>> + ib_rdmacg_uncharge(&obj->uevent.uobject.cg_obj, device,
>> + RDMA_VERB_RESOURCE_QP, 1);
>> +
>> err_put:
>> if (xrcd)
>> put_xrcd_read(xrcd_uobj);
>
>> @@ -3323,6 +3441,11 @@ static int __uverbs_create_xsrq(struct ib_uverbs_file *file,
>> obj->uevent.events_reported = 0;
>> INIT_LIST_HEAD(&obj->uevent.event_list);
>>
>> + ret = ib_rdmacg_try_charge(&obj->uevent.uobject.cg_obj, pd->device,
>> + RDMA_VERB_RESOURCE_SRQ, 1);
>> + if (ret)
>> + goto err_put_cq;
>> +
> I think you need a new error label to release the PD IDR but not
> uncharge.

Yes. Changed.