Messages in this thread | | | Date | Tue, 24 Nov 2015 11:27:28 -0500 | From | Tejun Heo <> | Subject | Re: [PATCH 3/8] cgroup: add function to get task's cgroup |
| |
Hello,
On Mon, Nov 16, 2015 at 01:51:40PM -0600, serge@hallyn.com wrote: > diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h > index 22e3754..29f0b02 100644 > --- a/include/linux/cgroup.h > +++ b/include/linux/cgroup.h > @@ -326,6 +326,7 @@ static inline bool css_tryget_online(struct cgroup_subsys_state *css) > return percpu_ref_tryget_live(&css->refcnt); > return true; > } > +struct cgroup *get_task_cgroup(struct task_struct *task);
Please move this where other prototypes are.
> +/* > + * get_task_cgroup - returns the cgroup of the task in the default cgroup > + * hierarchy. > + * > + * @task: target task > + * This function returns the @task's cgroup on the default cgroup hierarchy. The > + * returned cgroup has its reference incremented (by calling cgroup_get()). So > + * the caller must cgroup_put() the obtained reference once it is done with it. > + */ > +struct cgroup *get_task_cgroup(struct task_struct *task) > +{ > + struct cgroup *cgrp; > + > + mutex_lock(&cgroup_mutex); > + spin_lock_bh(&css_set_lock); > + > + cgrp = task_cgroup_from_root(task, &cgrp_dfl_root); > + cgroup_get(cgrp); > + > + spin_unlock_bh(&css_set_lock); > + mutex_unlock(&cgroup_mutex); > + return cgrp; > +} > +EXPORT_SYMBOL_GPL(get_task_cgroup);
So, exposing cgroup_mutex this way can lead to ugly lock dependency issues as cgroup_mutex is expected to be outside of pretty much everything. task_cgroup_path() does it but it has no users (should prolly removed) and cgroup_attach_task_all() is pretty specific.
Hmmm... cc'ing Li (btw, please cc him and Johannes from the next posting). Li, I don't think cset_cgroup_from_root() really needs cgroup_mutex. css_set_lock seems to be enough. What do you think?
Thanks.
-- tejun
| |