From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752225Ab0H3Fra (ORCPT ); Mon, 30 Aug 2010 01:47:30 -0400 Received: from TYO201.gate.nec.co.jp ([202.32.8.193]:41729 "EHLO tyo201.gate.nec.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751551Ab0H3Fr2 (ORCPT ); Mon, 30 Aug 2010 01:47:28 -0400 Date: Mon, 30 Aug 2010 14:44:23 +0900 From: Daisuke Nishimura To: KAMEZAWA Hiroyuki Cc: linux-mm@kvack.org, "balbir@linux.vnet.ibm.com" , gthelen@google.com, m-ikeda@ds.jp.nec.com, "akpm@linux-foundation.org" , "linux-kernel@vger.kernel.org" , "menage@google.com" , "lizf@cn.fujitsu.com" , Daisuke Nishimura Subject: Re: [PATCH 1/5] cgroup: do ID allocation under css allocator. Message-Id: <20100830144423.e9516b7e.nishimura@mxp.nes.nec.co.jp> In-Reply-To: <20100825170640.5f365629.kamezawa.hiroyu@jp.fujitsu.com> References: <20100825170435.15f8eb73.kamezawa.hiroyu@jp.fujitsu.com> <20100825170640.5f365629.kamezawa.hiroyu@jp.fujitsu.com> X-Mailer: Sylpheed 3.0.3 (GTK+ 2.10.14; i686-pc-mingw32) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 25 Aug 2010 17:06:40 +0900 KAMEZAWA Hiroyuki wrote: > From: KAMEZAWA Hiroyuki > > Now, css'id is allocated after ->create() is called. But to make use of ID > in ->create(), it should be available before ->create(). > > In another thinking, considering the ID is tightly coupled with "css", > it should be allocated when "css" is allocated. > This patch moves alloc_css_id() to css allocation routine. Now, only 2 subsys, > memory and blkio are useing ID. (To support complicated hierarchy walk.) > > ID will be used in mem cgroup's ->create(), later. > > Note: > If someone changes rules of css allocation, ID allocation should be moved too. > > Signed-off-by: KAMEZAWA Hiroyuki I think we need some signs from Paul and Li, but anyway Reviewed-by: Daisuke Nishimura Thanks, Daisuke Nishimura. > --- > block/blk-cgroup.c | 9 ++++++++ > include/linux/cgroup.h | 16 ++++++++------- > kernel/cgroup.c | 50 ++++++++++++++----------------------------------- > mm/memcontrol.c | 5 ++++ > 4 files changed, 38 insertions(+), 42 deletions(-) > > Index: mmotm-0811/kernel/cgroup.c > =================================================================== > --- mmotm-0811.orig/kernel/cgroup.c > +++ mmotm-0811/kernel/cgroup.c > @@ -289,9 +289,6 @@ struct cg_cgroup_link { > static struct css_set init_css_set; > static struct cg_cgroup_link init_css_set_link; > > -static int cgroup_init_idr(struct cgroup_subsys *ss, > - struct cgroup_subsys_state *css); > - > /* css_set_lock protects the list of css_set objects, and the > * chain of tasks off each css_set. Nests outside task->alloc_lock > * due to cgroup_iter_start() */ > @@ -770,9 +767,6 @@ static struct backing_dev_info cgroup_ba > .capabilities = BDI_CAP_NO_ACCT_AND_WRITEBACK, > }; > > -static int alloc_css_id(struct cgroup_subsys *ss, > - struct cgroup *parent, struct cgroup *child); > - > static struct inode *cgroup_new_inode(mode_t mode, struct super_block *sb) > { > struct inode *inode = new_inode(sb); > @@ -3257,7 +3251,8 @@ static void init_cgroup_css(struct cgrou > css->cgroup = cgrp; > atomic_set(&css->refcnt, 1); > css->flags = 0; > - css->id = NULL; > + if (!ss->use_id) > + css->id = NULL; > if (cgrp == dummytop) > set_bit(CSS_ROOT, &css->flags); > BUG_ON(cgrp->subsys[ss->subsys_id]); > @@ -3342,12 +3337,6 @@ static long cgroup_create(struct cgroup > goto err_destroy; > } > init_cgroup_css(css, ss, cgrp); > - if (ss->use_id) { > - err = alloc_css_id(ss, parent, cgrp); > - if (err) > - goto err_destroy; > - } > - /* At error, ->destroy() callback has to free assigned ID. */ > } > > cgroup_lock_hierarchy(root); > @@ -3709,17 +3698,6 @@ int __init_or_module cgroup_load_subsys( > > /* our new subsystem will be attached to the dummy hierarchy. */ > init_cgroup_css(css, ss, dummytop); > - /* init_idr must be after init_cgroup_css because it sets css->id. */ > - if (ss->use_id) { > - int ret = cgroup_init_idr(ss, css); > - if (ret) { > - dummytop->subsys[ss->subsys_id] = NULL; > - ss->destroy(ss, dummytop); > - subsys[i] = NULL; > - mutex_unlock(&cgroup_mutex); > - return ret; > - } > - } > > /* > * Now we need to entangle the css into the existing css_sets. unlike > @@ -3888,8 +3866,6 @@ int __init cgroup_init(void) > struct cgroup_subsys *ss = subsys[i]; > if (!ss->early_init) > cgroup_init_subsys(ss); > - if (ss->use_id) > - cgroup_init_idr(ss, init_css_set.subsys[ss->subsys_id]); > } > > /* Add init_css_set to the hash table */ > @@ -4603,8 +4579,8 @@ err_out: > > } > > -static int __init_or_module cgroup_init_idr(struct cgroup_subsys *ss, > - struct cgroup_subsys_state *rootcss) > +static int cgroup_init_idr(struct cgroup_subsys *ss, > + struct cgroup_subsys_state *rootcss) > { > struct css_id *newid; > > @@ -4616,21 +4592,25 @@ static int __init_or_module cgroup_init_ > return PTR_ERR(newid); > > newid->stack[0] = newid->id; > - newid->css = rootcss; > - rootcss->id = newid; > + rcu_assign_pointer(newid->css, rootcss); > + rcu_assign_pointer(rootcss->id, newid); > return 0; > } > > -static int alloc_css_id(struct cgroup_subsys *ss, struct cgroup *parent, > - struct cgroup *child) > +int alloc_css_id(struct cgroup_subsys *ss, > + struct cgroup *cgrp, struct cgroup_subsys_state *css) > { > int subsys_id, i, depth = 0; > - struct cgroup_subsys_state *parent_css, *child_css; > + struct cgroup_subsys_state *parent_css; > + struct cgroup *parent; > struct css_id *child_id, *parent_id; > > + if (cgrp == dummytop) > + return cgroup_init_idr(ss, css); > + > + parent = cgrp->parent; > subsys_id = ss->subsys_id; > parent_css = parent->subsys[subsys_id]; > - child_css = child->subsys[subsys_id]; > parent_id = parent_css->id; > depth = parent_id->depth + 1; > > @@ -4645,7 +4625,7 @@ static int alloc_css_id(struct cgroup_su > * child_id->css pointer will be set after this cgroup is available > * see cgroup_populate_dir() > */ > - rcu_assign_pointer(child_css->id, child_id); > + rcu_assign_pointer(css->id, child_id); > > return 0; > } > Index: mmotm-0811/include/linux/cgroup.h > =================================================================== > --- mmotm-0811.orig/include/linux/cgroup.h > +++ mmotm-0811/include/linux/cgroup.h > @@ -583,9 +583,11 @@ int cgroup_attach_task_current_cg(struct > /* > * CSS ID is ID for cgroup_subsys_state structs under subsys. This only works > * if cgroup_subsys.use_id == true. It can be used for looking up and scanning. > - * CSS ID is assigned at cgroup allocation (create) automatically > - * and removed when subsys calls free_css_id() function. This is because > - * the lifetime of cgroup_subsys_state is subsys's matter. > + * CSS ID must be assigned by subsys itself at cgroup creation and deleted > + * when subsys calls free_css_id() function. This is because the life time of > + * of cgroup_subsys_state is subsys's matter. > + * > + * ID->css look up is available after cgroup's directory is populated. > * > * Looking up and scanning function should be called under rcu_read_lock(). > * Taking cgroup_mutex()/hierarchy_mutex() is not necessary for following calls. > @@ -593,10 +595,10 @@ int cgroup_attach_task_current_cg(struct > * destroyed". The caller should check css and cgroup's status. > */ > > -/* > - * Typically Called at ->destroy(), or somewhere the subsys frees > - * cgroup_subsys_state. > - */ > +/* Should be called in ->create() by subsys itself */ > +int alloc_css_id(struct cgroup_subsys *ss, struct cgroup *newgr, > + struct cgroup_subsys_state *css); > +/* Typically Called at ->destroy(), or somewhere the subsys frees css */ > void free_css_id(struct cgroup_subsys *ss, struct cgroup_subsys_state *css); > > /* Find a cgroup_subsys_state which has given ID */ > Index: mmotm-0811/mm/memcontrol.c > =================================================================== > --- mmotm-0811.orig/mm/memcontrol.c > +++ mmotm-0811/mm/memcontrol.c > @@ -4141,6 +4141,11 @@ mem_cgroup_create(struct cgroup_subsys * > if (alloc_mem_cgroup_per_zone_info(mem, node)) > goto free_out; > > + error = alloc_css_id(ss, cont, &mem->css); > + if (error) > + goto free_out; > + /* Here, css_id(&mem->css) works. but css_lookup(id)->mem doesn't */ > + > /* root ? */ > if (cont->parent == NULL) { > int cpu; > Index: mmotm-0811/block/blk-cgroup.c > =================================================================== > --- mmotm-0811.orig/block/blk-cgroup.c > +++ mmotm-0811/block/blk-cgroup.c > @@ -958,9 +958,13 @@ blkiocg_create(struct cgroup_subsys *sub > { > struct blkio_cgroup *blkcg; > struct cgroup *parent = cgroup->parent; > + int ret; > > if (!parent) { > blkcg = &blkio_root_cgroup; > + ret = alloc_css_id(subsys, cgroup, &blkcg->css); > + if (ret) > + return ERR_PTR(ret); > goto done; > } > > @@ -971,6 +975,11 @@ blkiocg_create(struct cgroup_subsys *sub > blkcg = kzalloc(sizeof(*blkcg), GFP_KERNEL); > if (!blkcg) > return ERR_PTR(-ENOMEM); > + ret = alloc_css_id(subsys, cgroup, &blkcg->css); > + if (ret) { > + kfree(blkcg); > + return ERR_PTR(ret); > + } > > blkcg->weight = BLKIO_WEIGHT_DEFAULT; > done: > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail203.messagelabs.com (mail203.messagelabs.com [216.82.254.243]) by kanga.kvack.org (Postfix) with ESMTP id 8A4B56B01F2 for ; Mon, 30 Aug 2010 01:47:21 -0400 (EDT) Date: Mon, 30 Aug 2010 14:44:23 +0900 From: Daisuke Nishimura Subject: Re: [PATCH 1/5] cgroup: do ID allocation under css allocator. Message-Id: <20100830144423.e9516b7e.nishimura@mxp.nes.nec.co.jp> In-Reply-To: <20100825170640.5f365629.kamezawa.hiroyu@jp.fujitsu.com> References: <20100825170435.15f8eb73.kamezawa.hiroyu@jp.fujitsu.com> <20100825170640.5f365629.kamezawa.hiroyu@jp.fujitsu.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org To: KAMEZAWA Hiroyuki Cc: linux-mm@kvack.org, "balbir@linux.vnet.ibm.com" , gthelen@google.com, m-ikeda@ds.jp.nec.com, "akpm@linux-foundation.org" , "linux-kernel@vger.kernel.org" , "menage@google.com" , "lizf@cn.fujitsu.com" , Daisuke Nishimura List-ID: On Wed, 25 Aug 2010 17:06:40 +0900 KAMEZAWA Hiroyuki wrote: > From: KAMEZAWA Hiroyuki > > Now, css'id is allocated after ->create() is called. But to make use of ID > in ->create(), it should be available before ->create(). > > In another thinking, considering the ID is tightly coupled with "css", > it should be allocated when "css" is allocated. > This patch moves alloc_css_id() to css allocation routine. Now, only 2 subsys, > memory and blkio are useing ID. (To support complicated hierarchy walk.) > > ID will be used in mem cgroup's ->create(), later. > > Note: > If someone changes rules of css allocation, ID allocation should be moved too. > > Signed-off-by: KAMEZAWA Hiroyuki I think we need some signs from Paul and Li, but anyway Reviewed-by: Daisuke Nishimura Thanks, Daisuke Nishimura. > --- > block/blk-cgroup.c | 9 ++++++++ > include/linux/cgroup.h | 16 ++++++++------- > kernel/cgroup.c | 50 ++++++++++++++----------------------------------- > mm/memcontrol.c | 5 ++++ > 4 files changed, 38 insertions(+), 42 deletions(-) > > Index: mmotm-0811/kernel/cgroup.c > =================================================================== > --- mmotm-0811.orig/kernel/cgroup.c > +++ mmotm-0811/kernel/cgroup.c > @@ -289,9 +289,6 @@ struct cg_cgroup_link { > static struct css_set init_css_set; > static struct cg_cgroup_link init_css_set_link; > > -static int cgroup_init_idr(struct cgroup_subsys *ss, > - struct cgroup_subsys_state *css); > - > /* css_set_lock protects the list of css_set objects, and the > * chain of tasks off each css_set. Nests outside task->alloc_lock > * due to cgroup_iter_start() */ > @@ -770,9 +767,6 @@ static struct backing_dev_info cgroup_ba > .capabilities = BDI_CAP_NO_ACCT_AND_WRITEBACK, > }; > > -static int alloc_css_id(struct cgroup_subsys *ss, > - struct cgroup *parent, struct cgroup *child); > - > static struct inode *cgroup_new_inode(mode_t mode, struct super_block *sb) > { > struct inode *inode = new_inode(sb); > @@ -3257,7 +3251,8 @@ static void init_cgroup_css(struct cgrou > css->cgroup = cgrp; > atomic_set(&css->refcnt, 1); > css->flags = 0; > - css->id = NULL; > + if (!ss->use_id) > + css->id = NULL; > if (cgrp == dummytop) > set_bit(CSS_ROOT, &css->flags); > BUG_ON(cgrp->subsys[ss->subsys_id]); > @@ -3342,12 +3337,6 @@ static long cgroup_create(struct cgroup > goto err_destroy; > } > init_cgroup_css(css, ss, cgrp); > - if (ss->use_id) { > - err = alloc_css_id(ss, parent, cgrp); > - if (err) > - goto err_destroy; > - } > - /* At error, ->destroy() callback has to free assigned ID. */ > } > > cgroup_lock_hierarchy(root); > @@ -3709,17 +3698,6 @@ int __init_or_module cgroup_load_subsys( > > /* our new subsystem will be attached to the dummy hierarchy. */ > init_cgroup_css(css, ss, dummytop); > - /* init_idr must be after init_cgroup_css because it sets css->id. */ > - if (ss->use_id) { > - int ret = cgroup_init_idr(ss, css); > - if (ret) { > - dummytop->subsys[ss->subsys_id] = NULL; > - ss->destroy(ss, dummytop); > - subsys[i] = NULL; > - mutex_unlock(&cgroup_mutex); > - return ret; > - } > - } > > /* > * Now we need to entangle the css into the existing css_sets. unlike > @@ -3888,8 +3866,6 @@ int __init cgroup_init(void) > struct cgroup_subsys *ss = subsys[i]; > if (!ss->early_init) > cgroup_init_subsys(ss); > - if (ss->use_id) > - cgroup_init_idr(ss, init_css_set.subsys[ss->subsys_id]); > } > > /* Add init_css_set to the hash table */ > @@ -4603,8 +4579,8 @@ err_out: > > } > > -static int __init_or_module cgroup_init_idr(struct cgroup_subsys *ss, > - struct cgroup_subsys_state *rootcss) > +static int cgroup_init_idr(struct cgroup_subsys *ss, > + struct cgroup_subsys_state *rootcss) > { > struct css_id *newid; > > @@ -4616,21 +4592,25 @@ static int __init_or_module cgroup_init_ > return PTR_ERR(newid); > > newid->stack[0] = newid->id; > - newid->css = rootcss; > - rootcss->id = newid; > + rcu_assign_pointer(newid->css, rootcss); > + rcu_assign_pointer(rootcss->id, newid); > return 0; > } > > -static int alloc_css_id(struct cgroup_subsys *ss, struct cgroup *parent, > - struct cgroup *child) > +int alloc_css_id(struct cgroup_subsys *ss, > + struct cgroup *cgrp, struct cgroup_subsys_state *css) > { > int subsys_id, i, depth = 0; > - struct cgroup_subsys_state *parent_css, *child_css; > + struct cgroup_subsys_state *parent_css; > + struct cgroup *parent; > struct css_id *child_id, *parent_id; > > + if (cgrp == dummytop) > + return cgroup_init_idr(ss, css); > + > + parent = cgrp->parent; > subsys_id = ss->subsys_id; > parent_css = parent->subsys[subsys_id]; > - child_css = child->subsys[subsys_id]; > parent_id = parent_css->id; > depth = parent_id->depth + 1; > > @@ -4645,7 +4625,7 @@ static int alloc_css_id(struct cgroup_su > * child_id->css pointer will be set after this cgroup is available > * see cgroup_populate_dir() > */ > - rcu_assign_pointer(child_css->id, child_id); > + rcu_assign_pointer(css->id, child_id); > > return 0; > } > Index: mmotm-0811/include/linux/cgroup.h > =================================================================== > --- mmotm-0811.orig/include/linux/cgroup.h > +++ mmotm-0811/include/linux/cgroup.h > @@ -583,9 +583,11 @@ int cgroup_attach_task_current_cg(struct > /* > * CSS ID is ID for cgroup_subsys_state structs under subsys. This only works > * if cgroup_subsys.use_id == true. It can be used for looking up and scanning. > - * CSS ID is assigned at cgroup allocation (create) automatically > - * and removed when subsys calls free_css_id() function. This is because > - * the lifetime of cgroup_subsys_state is subsys's matter. > + * CSS ID must be assigned by subsys itself at cgroup creation and deleted > + * when subsys calls free_css_id() function. This is because the life time of > + * of cgroup_subsys_state is subsys's matter. > + * > + * ID->css look up is available after cgroup's directory is populated. > * > * Looking up and scanning function should be called under rcu_read_lock(). > * Taking cgroup_mutex()/hierarchy_mutex() is not necessary for following calls. > @@ -593,10 +595,10 @@ int cgroup_attach_task_current_cg(struct > * destroyed". The caller should check css and cgroup's status. > */ > > -/* > - * Typically Called at ->destroy(), or somewhere the subsys frees > - * cgroup_subsys_state. > - */ > +/* Should be called in ->create() by subsys itself */ > +int alloc_css_id(struct cgroup_subsys *ss, struct cgroup *newgr, > + struct cgroup_subsys_state *css); > +/* Typically Called at ->destroy(), or somewhere the subsys frees css */ > void free_css_id(struct cgroup_subsys *ss, struct cgroup_subsys_state *css); > > /* Find a cgroup_subsys_state which has given ID */ > Index: mmotm-0811/mm/memcontrol.c > =================================================================== > --- mmotm-0811.orig/mm/memcontrol.c > +++ mmotm-0811/mm/memcontrol.c > @@ -4141,6 +4141,11 @@ mem_cgroup_create(struct cgroup_subsys * > if (alloc_mem_cgroup_per_zone_info(mem, node)) > goto free_out; > > + error = alloc_css_id(ss, cont, &mem->css); > + if (error) > + goto free_out; > + /* Here, css_id(&mem->css) works. but css_lookup(id)->mem doesn't */ > + > /* root ? */ > if (cont->parent == NULL) { > int cpu; > Index: mmotm-0811/block/blk-cgroup.c > =================================================================== > --- mmotm-0811.orig/block/blk-cgroup.c > +++ mmotm-0811/block/blk-cgroup.c > @@ -958,9 +958,13 @@ blkiocg_create(struct cgroup_subsys *sub > { > struct blkio_cgroup *blkcg; > struct cgroup *parent = cgroup->parent; > + int ret; > > if (!parent) { > blkcg = &blkio_root_cgroup; > + ret = alloc_css_id(subsys, cgroup, &blkcg->css); > + if (ret) > + return ERR_PTR(ret); > goto done; > } > > @@ -971,6 +975,11 @@ blkiocg_create(struct cgroup_subsys *sub > blkcg = kzalloc(sizeof(*blkcg), GFP_KERNEL); > if (!blkcg) > return ERR_PTR(-ENOMEM); > + ret = alloc_css_id(subsys, cgroup, &blkcg->css); > + if (ret) { > + kfree(blkcg); > + return ERR_PTR(ret); > + } > > blkcg->weight = BLKIO_WEIGHT_DEFAULT; > done: > -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org