From mboxrd@z Thu Jan 1 00:00:00 1970 From: Parav Pandit Subject: Re: [PATCHv6 1/3] rdmacg: Added rdma cgroup controller Date: Sun, 21 Feb 2016 20:45:05 +0530 Message-ID: References: <1455966006-13774-1-git-send-email-pandit.parav@gmail.com> <1455966006-13774-2-git-send-email-pandit.parav@gmail.com> <20160221074301.GK30450@leon.nu> <20160221134518.GM30450@leon.nu> <20160221150932.GN30450@leon.nu> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: In-Reply-To: <20160221150932.GN30450@leon.nu> Sender: linux-kernel-owner@vger.kernel.org To: leon@leon.nu, Parav Pandit , cgroups@vger.kernel.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-rdma@vger.kernel.org, Tejun Heo , lizefan@huawei.com, Johannes Weiner , Doug Ledford , Liran Liss , "Hefty, Sean" , Jason Gunthorpe , Haggai Eran , Jonathan Corbet , james.l.morris@oracle.com, serge@hallyn.com, Or Gerlitz , Matan Barak , raindel@mellanox.com, akpm@linux-foundation.org, linux-security-module@vger.kernel.org List-Id: linux-rdma@vger.kernel.org On Sun, Feb 21, 2016 at 8:39 PM, Leon Romanovsky 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 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 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.