[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAG53R5UQzgWX327MbNV0HVC3XusTQEBN+a-CSUGMyztdE7Tr4w@mail.gmail.com>
Date: Sun, 21 Feb 2016 20:45:05 +0530
From: Parav Pandit <pandit.parav@...il.com>
To: leon@...n.nu, Parav Pandit <pandit.parav@...il.com>,
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>,
Haggai Eran <haggaie@...lanox.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 1/3] rdmacg: Added rdma cgroup controller
On Sun, Feb 21, 2016 at 8:39 PM, Leon Romanovsky <leon@...n.nu> wrote:
> On Sun, Feb 21, 2016 at 07:41:08PM +0530, Parav Pandit wrote:
>> CONFIG_CGROUP_RDMA
>>
>> On Sun, Feb 21, 2016 at 7:15 PM, Leon Romanovsky <leon@...n.nu> wrote:
>> > On Sun, Feb 21, 2016 at 05:03:05PM +0530, Parav Pandit wrote:
>> >> On Sun, Feb 21, 2016 at 1:13 PM, Leon Romanovsky <leon@...n.nu> wrote:
>> >> > On Sat, Feb 20, 2016 at 04:30:04PM +0530, Parav Pandit wrote:
>> >> > Can you place this ifdef before declaring struct rdma_cgroup?
>> >> Yes. I missed out this cleanup. Done locally now.
>> >
>> > Great, additional thing which spotted my attention was related to
>> > declaring and using the new cgroups functions. There are number of
>> > places where you protected the calls by specific ifdefs in the
>> > IB/core c-files and not in h-files as it is usually done.
>> >
>> ib_device_register_rdmacg, ib_device_unregister_rdmacg are the only
>> two functions called from IB/core as its tied to functionality.
>> They can also be implemented as NULL call when CONFIG_CGROUP_RDMA is undefined.
>> (Similar to ib_rdmacg_try_charge and others).
>> I didn't do because occurrence of call of register and unregister is
>> limited to single file and only twice compare to charge/uncharge
>> functions.
>> Either way is fine with me, I can make the changes which you
>> described. Let me know.
>
> Please do,
> IMHO, it is better to have one place which handles all relevant ifdefs
> and functions. IB/core doesn't need to know about cgroups implementation.
>
ok. Done. Thanks for the review. I will accumulate more comments from
Tejun and others before spinning v7.
Powered by blists - more mailing lists