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

Groups > linux.kernel > #1301884 > unrolled thread

[PATCHv1 0/6] rdma controller support

Back to article view | Back to linux.kernel

Contents

  [PATCHv1 0/6] rdma controller support Parav Pandit <pandit.parav@gmail.com> - 2016-01-05 20:00 +0100
    [PATCHv1 4/6] IB/core: rdmacg support infrastructure APIs Parav Pandit <pandit.parav@gmail.com> - 2016-01-05 20:10 +0100
    [PATCHv1 5/6] IB/core: use rdma cgroup for resource accounting Parav Pandit <pandit.parav@gmail.com> - 2016-01-05 20:10 +0100
    [PATCHv1 2/6] IB/core: Added members to support rdma cgroup Parav Pandit <pandit.parav@gmail.com> - 2016-01-05 20:10 +0100
      Re: [PATCHv1 2/6] IB/core: Added members to support rdma cgroup Tejun Heo <tj@kernel.org> - 2016-01-05 23:00 +0100
        Re: [PATCHv1 2/6] IB/core: Added members to support rdma cgroup Parav Pandit <pandit.parav@gmail.com> - 2016-01-07 00:20 +0100
          Re: [PATCHv1 2/6] IB/core: Added members to support rdma cgroup Tejun Heo <tj@kernel.org> - 2016-01-07 16:10 +0100
            Re: [PATCHv1 2/6] IB/core: Added members to support rdma cgroup Parav Pandit <pandit.parav@gmail.com> - 2016-01-07 20:50 +0100
    [PATCHv1 6/6] rdmacg: Added documentation for rdma controller. Parav Pandit <pandit.parav@gmail.com> - 2016-01-05 20:10 +0100
      Re: [PATCHv1 6/6] rdmacg: Added documentation for rdma controller. Tejun Heo <tj@kernel.org> - 2016-01-05 23:00 +0100
        Re: [PATCHv1 6/6] rdmacg: Added documentation for rdma controller. Parav Pandit <pandit.parav@gmail.com> - 2016-01-06 23:50 +0100
          Re: [PATCHv1 6/6] rdmacg: Added documentation for rdma controller. Tejun Heo <tj@kernel.org> - 2016-01-07 00:20 +0100
            Re: [PATCHv1 6/6] rdmacg: Added documentation for rdma controller. Parav Pandit <pandit.parav@gmail.com> - 2016-01-07 01:00 +0100
              Re: [PATCHv1 6/6] rdmacg: Added documentation for rdma controller. Tejun Heo <tj@kernel.org> - 2016-01-07 16:50 +0100
                Re: [PATCHv1 6/6] rdmacg: Added documentation for rdma controller. Parav Pandit <pandit.parav@gmail.com> - 2016-01-07 20:50 +0100
    [PATCHv1 1/6] rdmacg: Added rdma cgroup header file Parav Pandit <pandit.parav@gmail.com> - 2016-01-05 20:10 +0100
    [PATCHv1 3/6] rdmacg: implements rdma cgroup Parav Pandit <pandit.parav@gmail.com> - 2016-01-05 20:10 +0100
      Re: [PATCHv1 3/6] rdmacg: implements rdma cgroup Tejun Heo <tj@kernel.org> - 2016-01-05 23:10 +0100
        Re: [PATCHv1 3/6] rdmacg: implements rdma cgroup Parav Pandit <pandit.parav@gmail.com> - 2016-01-07 00:40 +0100
          Re: [PATCHv1 3/6] rdmacg: implements rdma cgroup Tejun Heo <tj@kernel.org> - 2016-01-07 16:30 +0100
            Re: [PATCHv1 3/6] rdmacg: implements rdma cgroup Parav Pandit <pandit.parav@gmail.com> - 2016-01-07 21:30 +0100
              Re: [PATCHv1 3/6] rdmacg: implements rdma cgroup Tejun Heo <tj@kernel.org> - 2016-01-07 21:30 +0100
                Re: [PATCHv1 3/6] rdmacg: implements rdma cgroup Parav Pandit <pandit.parav@gmail.com> - 2016-01-07 21:40 +0100
                  Re: [PATCHv1 3/6] rdmacg: implements rdma cgroup Tejun Heo <tj@kernel.org> - 2016-01-07 21:50 +0100
    Re: [PATCHv1 0/6] rdma controller support Tejun Heo <tj@kernel.org> - 2016-01-05 23:00 +0100
      Re: [PATCHv1 0/6] rdma controller support Parav Pandit <pandit.parav@gmail.com> - 2016-01-07 00:20 +0100
        Re: [PATCHv1 0/6] rdma controller support Tejun Heo <tj@kernel.org> - 2016-01-07 16:10 +0100
          Re: [PATCHv1 0/6] rdma controller support Tejun Heo <tj@kernel.org> - 2016-01-07 21:10 +0100
            Re: [PATCHv1 0/6] rdma controller support Parav Pandit <pandit.parav@gmail.com> - 2016-01-07 21:40 +0100
              Re: [PATCHv1 0/6] rdma controller support Tejun Heo <tj@kernel.org> - 2016-01-07 21:40 +0100
                Re: [PATCHv1 0/6] rdma controller support Tejun Heo <tj@kernel.org> - 2016-01-07 21:50 +0100
                  Re: [PATCHv1 0/6] rdma controller support Tejun Heo <tj@kernel.org> - 2016-01-07 22:00 +0100
                    Re: [PATCHv1 0/6] rdma controller support Parav Pandit <pandit.parav@gmail.com> - 2016-01-07 22:10 +0100
                      Re: [PATCHv1 0/6] rdma controller support Tejun Heo <tj@kernel.org> - 2016-01-07 22:10 +0100
                        Re: [PATCHv1 0/6] rdma controller support Parav Pandit <pandit.parav@gmail.com> - 2016-01-07 22:20 +0100
                  Re: [PATCHv1 0/6] rdma controller support Tejun Heo <tj@kernel.org> - 2016-01-07 22:10 +0100
                  Re: [PATCHv1 0/6] rdma controller support Parav Pandit <pandit.parav@gmail.com> - 2016-01-07 22:10 +0100
                Re: [PATCHv1 0/6] rdma controller support Parav Pandit <pandit.parav@gmail.com> - 2016-01-07 21:50 +0100
          Re: [PATCHv1 0/6] rdma controller support Parav Pandit <pandit.parav@gmail.com> - 2016-01-07 21:10 +0100

Page 2 of 2 — ← Prev page 1 [2]

#1303873 — Re: [PATCHv1 3/6] rdmacg: implements rdma cgroup

On Thu, Jan 7, 2016 at 8:59 PM, Tejun Heo <tj@kernel.org> wrote:
> Hello,
>
> On Thu, Jan 07, 2016 at 05:03:07AM +0530, Parav Pandit wrote:
>> Rdma resource can be allocated by parent process, used and freed by
>> child process.
>> Child process could belong to different rdma cgroup.
>> Parent process might have been terminated after creation of rdma
>> cgroup. (Followed by cgroup might have been deleted too).
>> Its discussed in https://lkml.org/lkml/2015/11/2/307
>>
>> In nutshell, there is process that clearly owns the rdma resource.
>> So to keep the design simple, rdma resource is owned by the creator
>> process and cgroup without modifying the task_struct.
>
> So, a resource created by a task in a cgroup staying in the cgroup
> when the task gets migrated is fine; however, a resource being
> allocated in a previous cgroup of the task isn't fine.  Once
> allocated, the resource themselves should be associated with the
> cgroup so that they can be freed from the ones they're allocated from.
>
I probably didn't explain it well in previous email. Let me try again
and see if it make sense.
Whenever resource is allocated, it belongs to a given rdma cgroup.
Whenever its freed, its freed from same rdma cgroup.
(even if the original allocating process is terminated or the original
cgroup is offline now).

Every time a new resource is allocated, its current rdma cgroup is
being considered for allocation.

IB stack keeps the resource ownership with the pid (tgid) structure
and holds reference count to it, so that even if original process is
terminated, its pid structure keeps hovering around until last
resource is freed.

Above functionality is achieved, by maintaining the map this tgid and
associated original cgroup at try_charge(), uncharge() time.

In alternate method,
Its simple to store the pointer of rdma_cgroup structure in the IB
resource structure and hold on reference count to rdma_cgroup.
so that when its freed, uncharge_resource can accept rdma_cgroup
structure pointer.
This method will eliminate current pid map infrastructure all together
and achieve same functionality as described above.

try_charge() would return pointer to rdma_cg or NULL based on
successful charge, or fail respectively.
uncharge() will have rdma_cg as input argument.
Are you ok with that approach?

> If I'm understanding it correctly, the code is bending basic rules
> around how resource and task cgroup membership is tracked, you really
> can't do that.
>
>> > I'm pretty sure you can get away with an fixed length array of
>> > counters.  Please keep it simple.  It's a simple hard limit enforcer.
>> > There's no need to create a massive dynamic infrastrucure.
>>
>> Every resource pool for verbs resource is fixed length array. Length
>> of the array is defined by the IB stack modules.
>> This array is per cgroup, per device.
>> Its per device, because we agreed that we want to address requirement
>> of controlling/configuring them on per device basis.
>> Devices appear and disappear. Therefore they are allocated dynamically.
>> Otherwise this array could be static in cgroup structure.
>
> Please see the previous response.
>
> Thanks.
>
> --
> tejun

[toc] | [prev] | [next] | [standalone]

#1303874 — Re: [PATCHv1 3/6] rdmacg: implements rdma cgroup

Hello, Parav.

On Fri, Jan 08, 2016 at 01:55:09AM +0530, Parav Pandit wrote:
...
> Above functionality is achieved, by maintaining the map this tgid and
> associated original cgroup at try_charge(), uncharge() time.

Hmmm, what happens after the following?

1. A process allocates some rdma resources and get registered on the
   hash table.

2. The process gets migrated to a different cgroup.

3. The process allocates more rdma resources.

Which cgroup would the resources from #3 be attributed to?

> In alternate method,
> Its simple to store the pointer of rdma_cgroup structure in the IB
> resource structure and hold on reference count to rdma_cgroup.
> so that when its freed, uncharge_resource can accept rdma_cgroup
> structure pointer.

That'd be a lot more in line with how other controllers behave.

Thanks.

-- 
tejun

[toc] | [prev] | [next] | [standalone]

#1303877 — Re: [PATCHv1 3/6] rdmacg: implements rdma cgroup

On Fri, Jan 8, 2016 at 1:58 AM, Tejun Heo <tj@kernel.org> wrote:
> Hello, Parav.
>
> On Fri, Jan 08, 2016 at 01:55:09AM +0530, Parav Pandit wrote:
> ...
>> Above functionality is achieved, by maintaining the map this tgid and
>> associated original cgroup at try_charge(), uncharge() time.
>
> Hmmm, what happens after the following?
>
> 1. A process allocates some rdma resources and get registered on the
>    hash table.
>
> 2. The process gets migrated to a different cgroup.
>
> 3. The process allocates more rdma resources.
>
> Which cgroup would the resources from #3 be attributed to?

Since the pid/tgid of the process doesn't change in step_3, it
allocates from original cgroup of step_1.

However in next patch V2, as described below, since IB resource will
store rdma_cg pointer,
in step_3, new resource will be allocated from new cgroup.
old resource will be freed from older cgroup.
This is what you are expecting, right?


>
>> In alternate method,
>> Its simple to store the pointer of rdma_cgroup structure in the IB
>> resource structure and hold on reference count to rdma_cgroup.
>> so that when its freed, uncharge_resource can accept rdma_cgroup
>> structure pointer.
>
> That'd be a lot more in line with how other controllers behave.
>

> Thanks.
>
> --
> tejun

[toc] | [prev] | [next] | [standalone]

#1303882 — Re: [PATCHv1 3/6] rdmacg: implements rdma cgroup

Hello, Parav.

On Fri, Jan 08, 2016 at 02:09:23AM +0530, Parav Pandit wrote:
> > Hmmm, what happens after the following?
> >
> > 1. A process allocates some rdma resources and get registered on the
> >    hash table.
> >
> > 2. The process gets migrated to a different cgroup.
> >
> > 3. The process allocates more rdma resources.
> >
> > Which cgroup would the resources from #3 be attributed to?
> 
> Since the pid/tgid of the process doesn't change in step_3, it
> allocates from original cgroup of step_1.

It shouldn't work that way.

> However in next patch V2, as described below, since IB resource will
> store rdma_cg pointer,
> in step_3, new resource will be allocated from new cgroup.
> old resource will be freed from older cgroup.
> This is what you are expecting, right?

Yeap.

Thanks.

-- 
tejun

[toc] | [prev] | [next] | [standalone]

#1302237

Hello,

On Wed, Jan 06, 2016 at 12:28:00AM +0530, Parav Pandit wrote:
> Resources are not defined by the RDMA cgroup. Resources are defined
> by RDMA/IB stack & optionally by HCA vendor device drivers.

As I wrote before, I don't think this is a good idea.  Drivers will
inevitably add non-sensical "resources" which don't make any sense
without much scrutiny.  If different controllers can't agree upon the
same set of resources, which probably is a pretty good sign that this
isn't too well thought out to begin with, at least make all resource
types defined by the controller itself and let the controllers enable
them selectively.

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1303134

Hi Tejun,

On Wed, Jan 6, 2016 at 3:26 AM, Tejun Heo <tj@kernel.org> wrote:
> Hello,
>
> On Wed, Jan 06, 2016 at 12:28:00AM +0530, Parav Pandit wrote:
>> Resources are not defined by the RDMA cgroup. Resources are defined
>> by RDMA/IB stack & optionally by HCA vendor device drivers.
>
> As I wrote before, I don't think this is a good idea.  Drivers will
> inevitably add non-sensical "resources" which don't make any sense
> without much scrutiny.

In our last discussion on v0 patch,
http://lkml.iu.edu/hypermail/linux/kernel/1509.1/04331.html

The direction was, that vendor should be able to define their own resources.
> If different controllers can't agree upon the
> same set of resources, which probably is a pretty good sign that this
> isn't too well thought out to begin with,

When you said "different controller" you meant "different hw vendors", right?
Or you meant, rdma, mem, cpu as controller here?

> at least make all resource
> types defined by the controller itself and let the controllers enable
> them selectively.
>
In this V1 patch, resource is defined by the IB stack and rdma cgroup
is facilitator for same.
By doing so, IB stack modules can define new resource without really
making changes to cgroup.
This design also allows hw vendors to define their own resources which
will be reviewed in rdma mailing list anway.
The idea is different hw versions can have different resource support,
so the whole intention is not about defining different resource but
rather enabling it.
But yes, I equally agree that by doing so, different hw controller
vendors can define different hw resources.


> Thanks.
>
> --
> tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1303644

Hello, Parav.

On Thu, Jan 07, 2016 at 04:43:20AM +0530, Parav Pandit wrote:
> > If different controllers can't agree upon the
> > same set of resources, which probably is a pretty good sign that this
> > isn't too well thought out to begin with,
> 
> When you said "different controller" you meant "different hw vendors", right?
> Or you meant, rdma, mem, cpu as controller here?

Different hw vendors.

> > at least make all resource
> > types defined by the controller itself and let the controllers enable
> > them selectively.
> >
> In this V1 patch, resource is defined by the IB stack and rdma cgroup
> is facilitator for same.
> By doing so, IB stack modules can define new resource without really
> making changes to cgroup.
> This design also allows hw vendors to define their own resources which
> will be reviewed in rdma mailing list anway.
> The idea is different hw versions can have different resource support,
> so the whole intention is not about defining different resource but
> rather enabling it.
> But yes, I equally agree that by doing so, different hw controller
> vendors can define different hw resources.

How many vendors and resources are we talking about?  What I was
trying to say was that unless the number is extremely high, it'd be
far simpler to hard code them in the rdma controller and let drivers
enable the ones which apply to them.  It would require updating the
rdma cgroup controller to add new resource types but I think that'd
actually be an upside, not down.  There needs to be some checks and
balances against adding new resource types; otherwise, it'll soon
become a mess.

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1303861

Hello,

On Fri, Jan 08, 2016 at 01:31:06AM +0530, Parav Pandit wrote:
> >  What I was
> > trying to say was that unless the number is extremely high, it'd be
> > far simpler to hard code them in the rdma controller and let drivers
> > enable the ones which apply to them.
> 
> Instead of in rdma controller, its hard coded in IB stack.
> I see this as an advantage where resource definition ownership remains
> with IB stack maintainers, rather than rdma cgroup maintainer.
> rdma cgroup maintainer doesn't have to understand what SRQ vs QP or
> ODP type MR or multicast group is.
> IB stack maintainer is better placed to judge and define it.
> 
> I would like to hear from Jason, Doug, Liran and other RDMA experts
> about their thoughts.

That's fine.  Make it a header file in IB stack which is included from
the rdma cgroup controller.  The only things are not building a huge
dynamic framework for something which can easily be a simple static
thing and having some oversight in adding resource types.

Thanks.

-- 
tejun

[toc] | [prev] | [next] | [standalone]

#1303878

On Fri, Jan 8, 2016 at 1:36 AM, Tejun Heo <tj@kernel.org> wrote:
> Hello,
>
> On Fri, Jan 08, 2016 at 01:31:06AM +0530, Parav Pandit wrote:
>> >  What I was
>> > trying to say was that unless the number is extremely high, it'd be
>> > far simpler to hard code them in the rdma controller and let drivers
>> > enable the ones which apply to them.
>>
>> Instead of in rdma controller, its hard coded in IB stack.
>> I see this as an advantage where resource definition ownership remains
>> with IB stack maintainers, rather than rdma cgroup maintainer.
>> rdma cgroup maintainer doesn't have to understand what SRQ vs QP or
>> ODP type MR or multicast group is.
>> IB stack maintainer is better placed to judge and define it.
>>
>> I would like to hear from Jason, Doug, Liran and other RDMA experts
>> about their thoughts.
>
> That's fine.  Make it a header file in IB stack which is included from
> the rdma cgroup controller.  The only things are not building a huge
> dynamic framework for something which can easily be a simple static
> thing and having some oversight in adding resource types.
>
o.k. That doable. I want to make sure that we are on same page on below design.
rpool (which will contain static array based on header file ) would be
still there, because resource limits are on per device basis. Number
of devices are variable and dynamically appear. Therefore rdma_cg will
have the list of rpool attached to it. Do you agree?

> Thanks.
>
> --
> tejun

[toc] | [prev] | [next] | [standalone]

#1303880

On Fri, Jan 08, 2016 at 02:02:20AM +0530, Parav Pandit wrote:
> o.k. That doable. I want to make sure that we are on same page on below design.
> rpool (which will contain static array based on header file ) would be
> still there, because resource limits are on per device basis. Number
> of devices are variable and dynamically appear. Therefore rdma_cg will
> have the list of rpool attached to it. Do you agree?

Yeap.  Would it make more sense to hang them off of whatever struct
which presents a rdma device tho?  And then just walk them from cgroup
controller?

Thanks.

-- 
tejun

[toc] | [prev] | [next] | [standalone]

#1303884

Hello, Parav.

On Fri, Jan 08, 2016 at 02:16:59AM +0530, Parav Pandit wrote:
> Let me think through it. Its been late night for me currently. So dont
> want to conclude in hurry.

Sure thing.

> At high level it looks doable by maintaining hash table head on per
> device basis, that further reduces hash contention by one level.
> I will get back on this tomorrow.

Hmmm... why would it need a hash table?  Let's say there's a struct
rdma_device for each rdma_device and then that stuct can simply have
rdma_device->res_table[] or whatever to track limits and consumptions
and rdma_device->res_enabled mask to tell which resources are enabled
on the device.

Thanks.

-- 
tejun

[toc] | [prev] | [next] | [standalone]

#1303893

Ooh, btw, please don't bother to create separate interfaces for v1 and
v2 hierarchies.  Just creating one following v2 conventions and using
the same thing for v1 should do and is a lot easier for everybody.

Thanks.

-- 
tejun

[toc] | [prev] | [next] | [standalone]

#1303907

On Fri, Jan 8, 2016 at 2:20 AM, Tejun Heo <tj@kernel.org> wrote:
> Ooh, btw, please don't bother to create separate interfaces for v1 and
> v2 hierarchies.  Just creating one following v2 conventions and using
> the same thing for v1 should do and is a lot easier for everybody.
>
Sure. I have already made that change to remove list.
and changed rdma.resource.verb.limit to rdma.verb.max etc.

I will keep rdma.hw.max around until we get some consensus.

Alternatively I was thinking to merge that as another attribute in
rdma.max file, like

mlx4_0 verb ah=max pd=10 qp=10
mlx4_0 hw ah=10 pd=100
ocrdma hw ah=10 pd=100

This remove hw specific extra file for rare feature.

Parav



> Thanks.
>
> --
> tejun

[toc] | [prev] | [next] | [standalone]

#1303911

On Fri, Jan 08, 2016 at 02:31:26AM +0530, Parav Pandit wrote:
> On Fri, Jan 8, 2016 at 2:20 AM, Tejun Heo <tj@kernel.org> wrote:
> > Ooh, btw, please don't bother to create separate interfaces for v1 and
> > v2 hierarchies.  Just creating one following v2 conventions and using
> > the same thing for v1 should do and is a lot easier for everybody.
> >
> Sure. I have already made that change to remove list.
> and changed rdma.resource.verb.limit to rdma.verb.max etc.
> 
> I will keep rdma.hw.max around until we get some consensus.
> 
> Alternatively I was thinking to merge that as another attribute in
> rdma.max file, like
> 
> mlx4_0 verb ah=max pd=10 qp=10
> mlx4_0 hw ah=10 pd=100
> ocrdma hw ah=10 pd=100
> 
> This remove hw specific extra file for rare feature.

Hmm... if there are duplicate keys, I think rdma.verb.max and
rdma.hw.max would be cleaner.

Thanks.

-- 
tejun

[toc] | [prev] | [next] | [standalone]

#1303915

On Fri, Jan 8, 2016 at 2:37 AM, Tejun Heo <tj@kernel.org> wrote:
> On Fri, Jan 08, 2016 at 02:31:26AM +0530, Parav Pandit wrote:
>> On Fri, Jan 8, 2016 at 2:20 AM, Tejun Heo <tj@kernel.org> wrote:
>> > Ooh, btw, please don't bother to create separate interfaces for v1 and
>> > v2 hierarchies.  Just creating one following v2 conventions and using
>> > the same thing for v1 should do and is a lot easier for everybody.
>> >
>> Sure. I have already made that change to remove list.
>> and changed rdma.resource.verb.limit to rdma.verb.max etc.
>>
>> I will keep rdma.hw.max around until we get some consensus.
>>
>> Alternatively I was thinking to merge that as another attribute in
>> rdma.max file, like
>>
>> mlx4_0 verb ah=max pd=10 qp=10
>> mlx4_0 hw ah=10 pd=100
>> ocrdma hw ah=10 pd=100
>>
>> This remove hw specific extra file for rare feature.
>
> Hmm... if there are duplicate keys, I think rdma.verb.max and
> rdma.hw.max would be cleaner.
>
ok. I agree. I will keep as you described.

> Thanks.
>
> --
> tejun

[toc] | [prev] | [next] | [standalone]

#1303906

Hello,

On Fri, Jan 08, 2016 at 02:34:49AM +0530, Parav Pandit wrote:
> That table won't be sufficient, because rdma_device is shared among
> multiple rdma_cgroups each such cgroup has different individual
> resource limit and usage count. This is currently rpool structure.
> For res_table[] needs to be per cgroup basis.

Ah, you're right.  Please do whatever seems better to you.

Thanks.

-- 
tejun

[toc] | [prev] | [next] | [standalone]

#1303912

On Fri, Jan 8, 2016 at 2:19 AM, Tejun Heo <tj@kernel.org> wrote:
> Hello, Parav.
>
> On Fri, Jan 08, 2016 at 02:16:59AM +0530, Parav Pandit wrote:
>> Let me think through it. Its been late night for me currently. So dont
>> want to conclude in hurry.
>
> Sure thing.
>
>> At high level it looks doable by maintaining hash table head on per
>> device basis, that further reduces hash contention by one level.
>> I will get back on this tomorrow.
>
> Hmmm... why would it need a hash table?  Let's say there's a struct
> rdma_device for each rdma_device and then that stuct can simply have
> rdma_device->res_table[] or whatever to track limits and consumptions
> and rdma_device->res_enabled mask to tell which resources are enabled
> on the device.
>

That table won't be sufficient, because rdma_device is shared among
multiple rdma_cgroups each such cgroup has different individual
resource limit and usage count. This is currently rpool structure.
For res_table[] needs to be per cgroup basis.


> Thanks.
>
> --
> tejun

[toc] | [prev] | [next] | [standalone]

#1303890

On Fri, Jan 8, 2016 at 2:04 AM, Tejun Heo <tj@kernel.org> wrote:
> On Fri, Jan 08, 2016 at 02:02:20AM +0530, Parav Pandit wrote:
>> o.k. That doable. I want to make sure that we are on same page on below design.
>> rpool (which will contain static array based on header file ) would be
>> still there, because resource limits are on per device basis. Number
>> of devices are variable and dynamically appear. Therefore rdma_cg will
>> have the list of rpool attached to it. Do you agree?
>
> Yeap.  Would it make more sense to hang them off of whatever struct
> which presents a rdma device tho?  And then just walk them from cgroup
> controller?
>

Let me think through it. Its been late night for me currently. So dont
want to conclude in hurry.
At high level it looks doable by maintaining hash table head on per
device basis, that further reduces hash contention by one level.
I will get back on this tomorrow.

> Thanks.
>
> --
> tejun

[toc] | [prev] | [next] | [standalone]

#1303864

On Thu, Jan 7, 2016 at 8:37 PM, Tejun Heo <tj@kernel.org> wrote:
> Hello, Parav.
>
> On Thu, Jan 07, 2016 at 04:43:20AM +0530, Parav Pandit wrote:
>> > If different controllers can't agree upon the
>> > same set of resources, which probably is a pretty good sign that this
>> > isn't too well thought out to begin with,
>>
>> When you said "different controller" you meant "different hw vendors", right?
>> Or you meant, rdma, mem, cpu as controller here?
>
> Different hw vendors.
>
>> > at least make all resource
>> > types defined by the controller itself and let the controllers enable
>> > them selectively.
>> >
>> In this V1 patch, resource is defined by the IB stack and rdma cgroup
>> is facilitator for same.
>> By doing so, IB stack modules can define new resource without really
>> making changes to cgroup.
>> This design also allows hw vendors to define their own resources which
>> will be reviewed in rdma mailing list anway.
>> The idea is different hw versions can have different resource support,
>> so the whole intention is not about defining different resource but
>> rather enabling it.
>> But yes, I equally agree that by doing so, different hw controller
>> vendors can define different hw resources.
>
> How many vendors and resources are we talking about?
To my knowledge Intel HFI driver is the only one.

>  What I was
> trying to say was that unless the number is extremely high, it'd be
> far simpler to hard code them in the rdma controller and let drivers
> enable the ones which apply to them.

Instead of in rdma controller, its hard coded in IB stack.
I see this as an advantage where resource definition ownership remains
with IB stack maintainers, rather than rdma cgroup maintainer.
rdma cgroup maintainer doesn't have to understand what SRQ vs QP or
ODP type MR or multicast group is.
IB stack maintainer is better placed to judge and define it.

I would like to hear from Jason, Doug, Liran and other RDMA experts
about their thoughts.

> It would require updating the
> rdma cgroup controller to add new resource types but I think that'd
> actually be an upside, not down.  There needs to be some checks and
> balances against adding new resource types; otherwise, it'll soon
> become a mess.

I think this checks for new resource type can be ensured by IB stack
maintainer to avoid the mess.

My preference is IB stack to define resource, but I am ok to let go
this feature if consensus is RDMA cgroup.

>
> Thanks.
>
> --
> tejun

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web