RE: [PATCH V2] cgroup: Introduce cgroup_detach_task().
Maybe your point can be added to the README or FAQ for cgroup. Try to shoot a patch for that, it will be more effective... Thanks, Xun -Original Message- From: cgroups-ow...@vger.kernel.org [mailto:cgroups-ow...@vger.kernel.org] On Behalf Of Dongsheng Yang Sent: Tuesday, August 26, 2014 10:16 AM To: Li Zefan Cc: Tejun Heo; Dongsheng Yang; cgro...@vger.kernel.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH V2] cgroup: Introduce cgroup_detach_task(). On Tue, Aug 26, 2014 at 9:35 AM, Li Zefan wrote: > On 2014/8/25 23:00, Dongsheng Yang wrote: >> On Mon, Aug 25, 2014 at 10:47 PM, Tejun Heo wrote: >>> On Mon, Aug 25, 2014 at 10:46:03PM +0800, Dongsheng Yang wrote: >>>> My point here is that attaching and detaching are a pair of operations. >>> >>> There is no detaching from a cgroup. A task is always attached to a >>> cgroup whether that's a root or non-root cgroup. >> >> Okey, I should not think it as attaching and detaching. Just treat >> them as a move between root and non-root cgroup. >> >> It sounds reasonable to me now. >> > > I from time to time have to explain this to other people. Ha, we usually want to find a detach function when we saw a function named as cgroup_attach_task(). > -- To unsubscribe from this list: send the line "unsubscribe cgroups" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] cgroup: Introduce cgroup_detach_task().
On Tue, Aug 26, 2014 at 9:35 AM, Li Zefan wrote: > On 2014/8/25 23:00, Dongsheng Yang wrote: >> On Mon, Aug 25, 2014 at 10:47 PM, Tejun Heo wrote: >>> On Mon, Aug 25, 2014 at 10:46:03PM +0800, Dongsheng Yang wrote: My point here is that attaching and detaching are a pair of operations. >>> >>> There is no detaching from a cgroup. A task is always attached to a >>> cgroup whether that's a root or non-root cgroup. >> >> Okey, I should not think it as attaching and detaching. Just treat them as >> a move between root and non-root cgroup. >> >> It sounds reasonable to me now. >> > > I from time to time have to explain this to other people. Ha, we usually want to find a detach function when we saw a function named as cgroup_attach_task(). > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] cgroup: Introduce cgroup_detach_task().
On 2014/8/25 23:00, Dongsheng Yang wrote: > On Mon, Aug 25, 2014 at 10:47 PM, Tejun Heo wrote: >> On Mon, Aug 25, 2014 at 10:46:03PM +0800, Dongsheng Yang wrote: >>> My point here is that attaching and detaching are a pair of operations. >> >> There is no detaching from a cgroup. A task is always attached to a >> cgroup whether that's a root or non-root cgroup. > > Okey, I should not think it as attaching and detaching. Just treat them as > a move between root and non-root cgroup. > > It sounds reasonable to me now. > I from time to time have to explain this to other people. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] cgroup: Introduce cgroup_detach_task().
On Mon, Aug 25, 2014 at 10:47 PM, Tejun Heo wrote: > On Mon, Aug 25, 2014 at 10:46:03PM +0800, Dongsheng Yang wrote: >> My point here is that attaching and detaching are a pair of operations. > > There is no detaching from a cgroup. A task is always attached to a > cgroup whether that's a root or non-root cgroup. Okey, I should not think it as attaching and detaching. Just treat them as a move between root and non-root cgroup. It sounds reasonable to me now. Thanx. > > Thanks. > > -- > tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] cgroup: Introduce cgroup_detach_task().
On Mon, Aug 25, 2014 at 10:46:03PM +0800, Dongsheng Yang wrote: > My point here is that attaching and detaching are a pair of operations. There is no detaching from a cgroup. A task is always attached to a cgroup whether that's a root or non-root cgroup. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] cgroup: Introduce cgroup_detach_task().
Hi tj. On Mon, Aug 25, 2014 at 10:12 PM, Tejun Heo wrote: > On Mon, Aug 25, 2014 at 07:32:10PM +0800, Dongsheng Yang wrote: >> Currently, the only method to detach a task from a cgroup is moving >> it to others. It looks not natrual to me. > > Ummm... how is this different from moving it to the root cgroup? > "just because" usually isn't a good enough rationale. > Thanx for your reply :). My point here is that attaching and detaching are a pair of operations. And the things involved in these operations should be a process and a cgroup. But currently, when we want to detach a process from a cgroup (A), we have to attach it to another cgroup (B). I think it is not easy to understand. People would say: "Why I want to detach a process from A, I have to care about another cgroup." So I created this patch here wanting to make these operations more easy to understand. And make attaching and detaching to be antonymous in action. I think maybe we can use it in the Cgroup V2 which is in developing. :) That's all my reason for this patch. But you could think it makes no sense. It's okey to me. Thanx > Thanks. > > -- > tejun > -- > To unsubscribe from this list: send the line "unsubscribe cgroups" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] cgroup: Introduce cgroup_detach_task().
On Mon, Aug 25, 2014 at 07:32:10PM +0800, Dongsheng Yang wrote: > Currently, the only method to detach a task from a cgroup is moving > it to others. It looks not natrual to me. Ummm... how is this different from moving it to the root cgroup? "just because" usually isn't a good enough rationale. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH V2] cgroup: Introduce cgroup_detach_task().
Currently, the only method to detach a task from a cgroup is moving it to others. It looks not natrual to me. Inspired by cgroup_subtree_control_write(), this patch introduce allow user to at-detach a process to/from a cgroup by echo "+/-pid" to cgroup.procs. In addition, we keep the old method to allow user echo "pid" without "+/-" to cgroup.procs as a attaching behavior. Signed-off-by: Dongsheng Yang --- >From v1: *Sorry for a incorrect comment in v1. kernel/cgroup.c | 61 +++-- 1 file changed, 59 insertions(+), 2 deletions(-) diff --git a/kernel/cgroup.c b/kernel/cgroup.c index 7dc8788..a4bb604 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c @@ -2348,6 +2348,43 @@ static int cgroup_attach_task(struct cgroup *dst_cgrp, return ret; } +/** + * cgroup_detach_task - detach a task or a whole threadgroup to a cgroup + * @src_cgrp: the cgroup to detach from + * @leader: the task or the leader of the threadgroup to be attached + * @threadgroup: attach the whole threadgroup? + * + * Call holding cgroup_mutex and threadgroup_lock of @leader. + */ +static int cgroup_detach_task(struct cgroup *src_cgrp __maybe_unused, + struct task_struct *leader, bool threadgroup) +{ + LIST_HEAD(preloaded_csets); + struct task_struct *task; + int ret; + + /* look up all src csets */ + down_read(&css_set_rwsem); + rcu_read_lock(); + task = leader; + do { + cgroup_migrate_add_src(task_css_set(task), &cgrp_dfl_root.cgrp, + &preloaded_csets); + if (!threadgroup) + break; + } while_each_thread(leader, task); + rcu_read_unlock(); + up_read(&css_set_rwsem); + + /* prepare dst csets and commit */ + ret = cgroup_migrate_prepare_dst(&cgrp_dfl_root.cgrp, &preloaded_csets); + if (!ret) + ret = cgroup_migrate(&cgrp_dfl_root.cgrp, leader, threadgroup); + + cgroup_migrate_finish(&preloaded_csets); + return ret; +} + /* * Find the task_struct of the task to attach by vpid and pass it along to the * function to attach either it or all tasks in its threadgroup. Will lock @@ -2361,8 +2398,25 @@ static ssize_t __cgroup_procs_write(struct kernfs_open_file *of, char *buf, struct cgroup *cgrp; pid_t pid; int ret; + bool attach; - if (kstrtoint(strstrip(buf), 0, &pid) || pid < 0) + /* +* Parse input - pid prefixed with either + or -. +*/ + buf = strstrip(buf); + if (*buf == '+') { + attach = true; + buf++; + } else if (*buf == '-') { + attach = false; + buf++; + } else { + if (!isdigit(*buf)) + return -EINVAL; + attach = true; + } + + if (kstrtoint(buf, 0, &pid) || pid < 0) return -EINVAL; cgrp = cgroup_kn_lock_live(of->kn); @@ -2426,7 +2480,10 @@ retry_find_task: } } - ret = cgroup_attach_task(cgrp, tsk, threadgroup); + if (attach) + ret = cgroup_attach_task(cgrp, tsk, threadgroup); + else + ret = cgroup_detach_task(cgrp, tsk, threadgroup); threadgroup_unlock(tsk); -- 1.8.4.2 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/