Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1338899

Re: [PATCHv6 1/3] rdmacg: Added rdma cgroup controller

From Parav Pandit <pandit.parav@gmail.com>
Newsgroups linux.kernel
Subject Re: [PATCHv6 1/3] rdmacg: Added rdma cgroup controller
Date 2016-02-21 19:00 +0100
Message-ID <r4GsA-16v-37@gated-at.bofh.it> (permalink)
References (2 earlier) <r4GiT-12G-21@gated-at.bofh.it> <r4GiT-12G-23@gated-at.bofh.it> <r4GiT-12G-25@gated-at.bofh.it> <r4GiT-12G-27@gated-at.bofh.it> <r4GiT-12G-19@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Sun, Feb 21, 2016 at 8:39 PM, Leon Romanovsky <leon@leon.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@leon.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@leon.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.

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCHv6 0/3] rdmacg: IB/core: rdma controller support Parav Pandit <pandit.parav@gmail.com> - 2016-02-20 12:10 +0100
  [PATCHv6 2/3] IB/core: added support to use rdma cgroup controller Parav Pandit <pandit.parav@gmail.com> - 2016-02-20 12:10 +0100
  [PATCHv6 3/3] rdmacg: Added documentation for rdmacg Parav Pandit <pandit.parav@gmail.com> - 2016-02-20 12:10 +0100
  [PATCHv6 1/3] rdmacg: Added rdma cgroup controller Parav Pandit <pandit.parav@gmail.com> - 2016-02-20 12:10 +0100
    Re: [PATCHv6 1/3] rdmacg: Added rdma cgroup controller Leon Romanovsky <leon@leon.nu> - 2016-02-21 18:50 +0100
      Re: [PATCHv6 1/3] rdmacg: Added rdma cgroup controller Parav Pandit <pandit.parav@gmail.com> - 2016-02-21 19:00 +0100
    Re: [PATCHv6 1/3] rdmacg: Added rdma cgroup controller Leon Romanovsky <leon@leon.nu> - 2016-02-21 19:00 +0100
      Re: [PATCHv6 1/3] rdmacg: Added rdma cgroup controller Parav Pandit <pandit.parav@gmail.com> - 2016-02-21 19:30 +0100
    Re: [PATCHv6 1/3] rdmacg: Added rdma cgroup controller Parav Pandit <pandit.parav@gmail.com> - 2016-02-21 19:40 +0100
    Re: [PATCHv6 1/3] rdmacg: Added rdma cgroup controller Leon Romanovsky <leon@leon.nu> - 2016-02-21 20:20 +0100
  Re: [PATCHv6 0/3] rdmacg: IB/core: rdma controller support Parav Pandit <pandit.parav@gmail.com> - 2016-02-22 06:00 +0100

csiph-web