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

On 25/02/2016 16:26, Parav Pandit wrote: >> Can we call kfree() with spin_lock held? All these years I tend to >> avoid doing so. > Also it doesn't look correct to hold the lock while freeing the memory > which is totally unrelated to the lock. > With that I think current code appears ok with

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

On 25/02/2016 16:26, Parav Pandit wrote: >> Can we call kfree() with spin_lock held? All these years I tend to >> avoid doing so. > Also it doesn't look correct to hold the lock while freeing the memory > which is totally unrelated to the lock. > With that I think current code appears ok with

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

On 25/02/2016 15:34, Parav Pandit wrote: > On Thu, Feb 25, 2016 at 5:33 PM, Haggai Eran wrote: > +retry: > + spin_lock(>rpool_list_lock); > + rpool = find_cg_rpool_locked(cg, device); > + if (!rpool) { > +

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

On 25/02/2016 15:34, Parav Pandit wrote: > On Thu, Feb 25, 2016 at 5:33 PM, Haggai Eran wrote: > +retry: > + spin_lock(>rpool_list_lock); > + rpool = find_cg_rpool_locked(cg, device); > + if (!rpool) { > + spin_unlock(>rpool_list_lock); > +

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

> Can we call kfree() with spin_lock held? All these years I tend to > avoid doing so. Also it doesn't look correct to hold the lock while freeing the memory which is totally unrelated to the lock. With that I think current code appears ok with exception that its duplicated at two place for code

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

> Can we call kfree() with spin_lock held? All these years I tend to > avoid doing so. Also it doesn't look correct to hold the lock while freeing the memory which is totally unrelated to the lock. With that I think current code appears ok with exception that its duplicated at two place for code

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

On Thu, Feb 25, 2016 at 5:33 PM, Haggai Eran wrote: +retry: + spin_lock(>rpool_list_lock); + rpool = find_cg_rpool_locked(cg, device); + if (!rpool) { + spin_unlock(>rpool_list_lock); + ret =

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

On Thu, Feb 25, 2016 at 5:33 PM, Haggai Eran wrote: +retry: + spin_lock(>rpool_list_lock); + rpool = find_cg_rpool_locked(cg, device); + if (!rpool) { + spin_unlock(>rpool_list_lock); + ret = alloc_cg_rpool(cg, device); +

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

On 24/02/2016 18:16, Parav Pandit wrote: >>> + struct rdmacg_resource_pool *rpool; >>> + struct rdmacg_pool_info *pool_info = >pool_info; >>> + >>> + spin_lock(>rpool_list_lock); >>> + rpool = find_cg_rpool_locked(cg, device); >> Is it possible for rpool to be NULL? >> > Unlikely,

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

On 24/02/2016 18:16, Parav Pandit wrote: >>> + struct rdmacg_resource_pool *rpool; >>> + struct rdmacg_pool_info *pool_info = >pool_info; >>> + >>> + spin_lock(>rpool_list_lock); >>> + rpool = find_cg_rpool_locked(cg, device); >> Is it possible for rpool to be NULL? >> > Unlikely,

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

Hi, Overall I the patch looks good to me. I have a few comments below. On 20/02/2016 13:00, Parav Pandit wrote: > Resource pool is created/destroyed dynamically whenever > charging/uncharging occurs respectively and whenever user > configuration is done. Its a tradeoff of memory vs little more

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

Hi, Overall I the patch looks good to me. I have a few comments below. On 20/02/2016 13:00, Parav Pandit wrote: > Resource pool is created/destroyed dynamically whenever > charging/uncharging occurs respectively and whenever user > configuration is done. Its a tradeoff of memory vs little more

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

On Wed, Feb 24, 2016 at 6:43 PM, Haggai Eran wrote: > Hi, > > Overall I the patch looks good to me. I have a few comments below. > Thanks for the review. Addressing most comments one. Some comments inline. > Its -> It's Ok. >> +void rdmacg_query_limit(struct rdmacg_device

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

On Wed, Feb 24, 2016 at 6:43 PM, Haggai Eran wrote: > Hi, > > Overall I the patch looks good to me. I have a few comments below. > Thanks for the review. Addressing most comments one. Some comments inline. > Its -> It's Ok. >> +void rdmacg_query_limit(struct rdmacg_device *device, >> +

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

On Sat, Feb 20, 2016 at 04:30:04PM +0530, Parav Pandit wrote: > Added rdma cgroup controller that does accounting, limit enforcement > on rdma/IB verbs and hw resources. > > Added rdma cgroup header file which defines its APIs to perform > charing/uncharing functionality and device registration

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

On Sat, Feb 20, 2016 at 04:30:04PM +0530, Parav Pandit wrote: > Added rdma cgroup controller that does accounting, limit enforcement > on rdma/IB verbs and hw resources. > > Added rdma cgroup header file which defines its APIs to perform > charing/uncharing functionality and device registration

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

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. > Thanks

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

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. > Thanks

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

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: >>

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

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

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

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

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

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

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

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

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

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,

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

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

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

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,

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

Added rdma cgroup controller that does accounting, limit enforcement on rdma/IB verbs and hw resources. Added rdma cgroup header file which defines its APIs to perform charing/uncharing functionality and device registration which will participate in controller functions of accounting and limit

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

Added rdma cgroup controller that does accounting, limit enforcement on rdma/IB verbs and hw resources. Added rdma cgroup header file which defines its APIs to perform charing/uncharing functionality and device registration which will participate in controller functions of accounting and limit