On Wed 24-07-13 17:59:12, Li Zefan wrote: > This enables us to lookup a cgroup by its id. > > Signed-off-by: Li Zefan <[email protected]>
Reviewed-by: Michal Hocko <[email protected]> One nit/question bellow [...] > @@ -4268,15 +4271,19 @@ static long cgroup_create(struct cgroup *parent, > struct dentry *dentry, > if (!cgrp) > return -ENOMEM; > > + /* > + * Temporarily set the pointer to NULL, so idr_find() won't return > + * a half-baked cgroup. > + */ > + cgrp->id = idr_alloc(&root->cgroup_idr, NULL, 1, 0, GFP_KERNEL); > + if (cgrp->id < 0) > + goto err_free_cgrp; > + > name = cgroup_alloc_name(dentry); > if (!name) > - goto err_free_cgrp; > + goto err_free_id; > rcu_assign_pointer(cgrp->name, name); > > - cgrp->id = ida_simple_get(&root->cgroup_ida, 1, 0, GFP_KERNEL); > - if (cgrp->id < 0) > - goto err_free_name; > - Is the move necessary? You would safe few lines in the patch if you kept the ordering. > /* > * Only live parents can have children. Note that the liveliness > * check isn't strictly necessary because cgroup_mkdir() and > @@ -4286,7 +4293,7 @@ static long cgroup_create(struct cgroup *parent, struct > dentry *dentry, > */ > if (!cgroup_lock_live_group(parent)) { > err = -ENODEV; > - goto err_free_id; > + goto err_free_name; > } > > /* Grab a reference on the superblock so the hierarchy doesn't > @@ -4371,6 +4378,8 @@ static long cgroup_create(struct cgroup *parent, struct > dentry *dentry, > } > } > > + idr_replace(&root->cgroup_idr, cgrp, cgrp->id); > + > err = cgroup_addrm_files(cgrp, NULL, cgroup_base_files, true); > if (err) > goto err_destroy; > @@ -4396,10 +4405,10 @@ err_free_all: > mutex_unlock(&cgroup_mutex); > /* Release the reference count that we took on the superblock */ > deactivate_super(sb); > -err_free_id: > - ida_simple_remove(&root->cgroup_ida, cgrp->id); > err_free_name: > kfree(rcu_dereference_raw(cgrp->name)); > +err_free_id: > + idr_remove(&root->cgroup_idr, cgrp->id); > err_free_cgrp: > kfree(cgrp); > return err; [...] -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
