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

[Parav Pandit]

Hi Tejun,

On Sat, Mar 5, 2016 at 10:50 PM, Parav Pandit <pandit.parav@xxxxxxxxx> wrote:
> Hi Tejun,
>
> On Sat, Mar 5, 2016 at 6:22 PM, Tejun Heo <tj@xxxxxxxxxx> wrote:
>> Hello, Parav.
>>
>> On Sat, Mar 05, 2016 at 04:45:09PM +0530, Parav Pandit wrote:
>>> Design that remains same from v6 to v10.
>>>   * spin lock is still fine grained at cgroup level instead of one
>>> global shared lock among all cgroups.
>>>      In future it can be optimized further to do per cpu or using
>>> single lock if required.
>>>   * file type enums are still present for max and current, as
>>> read/write call to those files is already taken care by common
>>> functions with required if/else.
>>>   * Resource limit setting is as it is, because number of devices are
>>> in range of 1 to 4 count in most use cases (as explained in
>>> documentation), and its not hot path.
>>
>> 1 and 2 are not okay.
> For (1) shall I have one spin lock that is uses across multiple
> hierarchy and multiple cgroup.
> Essentially one global lock among all cgroup. During hierarchical
> charging, continue to use same lock it at each level.
> Would that work in this first release?
>

I am waiting for your reply.
Shall one lock for all cgroup is ok with you?

> Can you please review the code for (2), I cannot think of any further
> helper functions that I can write.
> For both the file types, all the code is already common.
> file types are used only to find out whether to reference max variable
> or usage variable in structure.
> Which can also be made as array, but I do not want to lose the code
> readability for that little gain.
> What exactly is the issue in current implementation? You just
> mentioned that "its not good sign".
> Its readable, simple and serves the purpose, what am I missing?
>
If this is ok. I will keep the code as it is, because it uses common
helper functions for max and current files.


>> 3 is fine but resource [un]charging is not hot path?
> charge/uncharge is hot path from cgroup perspective.
> Considering 1 to 4 devices in system rpool list would grow upto 4
> entry deep at each cgroup level.
> I believe this is good enough to start with. O complexity wise its
> O(N). where N is number of devices in system.
>
>
>>
>> Thanks.
>>
>> --
>> tejun