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

[Parav Pandit]

On Tue, Apr 5, 2016 at 7:01 AM, Tejun Heo <tj@xxxxxxxxxx> wrote:
> Hello, Parav.
>
> On Mon, Apr 04, 2016 at 07:22:38PM -0700, Parav Pandit wrote:
>> > Is it actually customary to have rdma core module updated more
>> > frequently separate from the kernel?  Out-of-tree modules being
>> > updated separately happens quite a bit but this is an in-kernel
>> > module, which usually is tightly coupled with the rest of the kernel.
>> >
>> Yes.
>> rdma core module is compiled as kernel module.
>> Its often updated for new features, fixes.
>> So kernel version can be one but RDMA core module(s) get updated more
>> frequently than kernel.
>
> As it is a fairly isolated domain, to certain extent, it could be okay
> to let it go.  At the same time, I know that these enterprise things
> tend to go completely wayward and am worried about individual drivers
> going crazy with custom attributes in a non-sensical way.

If its crazy at driver level, I am sure it will be equally crazy for
any end-user too. Therefore no user would use those crazy resources
either.
Intent is certainly not for the individual drivers as we agreed in
past. IB stack maintainers would be reviewing new enum addition
anyway, whether its verb or hw resource (unlikely).

> The
> interface this patch is proposing easily allows that and that at the
> cost of internal engineering flexibility.  I don't really want to be
> caught up in a situation where we're forced to include broken usages
> because that's what's already out in the wild.  I personally would
> much prefer the resources to be defined rigidly.  Let's see how the
> discussion with Christoph evolves.
>
Sure. I will wait for his comments.

>> > I don't remember the details well but the code was vastly more complex
>> > back then and the object lifetime management was muddy at best.  If I
>> > reviewed in a contradicting way, my apologies, but given the current
>> > code, it'd be better to have objects creation upfront.
>>
>> Do you mean,
>> try_charge() should
>> lock()
>> run loop to allocate in hierarchy, if not allocated.
>> run loop again to charge.
>> unlock()
>>
>> If so, I prefer to run the loop once.
>
> In the original review message, I mentioned creating an interface
> which creates the hierarchy of objects as necessary and returns the
> target pool with lock held, can you please give it a shot?  Something
> like the following.

o.k. I will attempt. Looks doable.
I am on travel so it will take few more days for me to turn around
with below approach with tested code.