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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 24 Feb 2016 21:35:00 +0530
From:	Parav Pandit <pandit.parav@...il.com>
To:	Haggai Eran <haggaie@...lanox.com>
Cc:	cgroups@...r.kernel.org, linux-doc@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-rdma@...r.kernel.org,
	Tejun Heo <tj@...nel.org>, lizefan@...wei.com,
	Johannes Weiner <hannes@...xchg.org>,
	Doug Ledford <dledford@...hat.com>,
	Liran Liss <liranl@...lanox.com>,
	"Hefty, Sean" <sean.hefty@...el.com>,
	Jason Gunthorpe <jgunthorpe@...idianresearch.com>,
	Jonathan Corbet <corbet@....net>, james.l.morris@...cle.com,
	serge@...lyn.com, Or Gerlitz <ogerlitz@...lanox.com>,
	Matan Barak <matanb@...lanox.com>, raindel@...lanox.com,
	akpm@...ux-foundation.org, linux-security-module@...r.kernel.org
Subject: Re: [PATCHv6 2/3] IB/core: added support to use rdma cgroup controller

On Wed, Feb 24, 2016 at 7:13 PM, Haggai Eran <haggaie@...lanox.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.