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

[Parav Pandit <pandit.parav@gmail.com>]

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.