Re: [PATCH] sched, autogroup: Fix failure when writing to cpu.rt_runtime_us

[Peter Zijlstra]

On Fri, Feb 06, 2015 at 09:30:51AM +0800, Zefan Li wrote:
> After running the test program, we have:
> 
>   root
>    |
>   autogroup   <-- the RT test program is now here
> 
> root.rt_runtime = 950000
> autogroup.rt_runtime = 0
> 
> Now if you try to change the rt_runtime of the root cgroup, it returns
> -EBUSY.
> 
> That comes from:
> 
> static int tg_rt_schedulable(struct task_group *tg, void *data)
> (
> 	...
>         if (rt_bandwidth_enabled() && !runtime && tg_has_rt_tasks(tg))
>                 return -EBUSY;
> 	...
> }
> 
> static inline int tg_has_rt_tasks(struct task_group *tg)
> {
>         struct task_struct *g, *p;
> 
>         for_each_process_thread(g, p) {
>                 if (rt_task(p) && task_group(p) == tg)
>                         return 1;
>         }
> 
>         return 0;
> }
> 
> here tg == autogroup, p == the test program, and task_group(p)
> returns autogroup, which is wrong.

No its not.

> See this commit:
> 
> commit f44937718ce3b8360f72f6c68c9481712517a867
> Author: Mike Galbraith <efault@xxxxxx>
> Date:   Thu Jan 13 04:54:50 2011 +0100
> 
>     sched, autogroup: Fix CONFIG_RT_GROUP_SCHED sched_setscheduler() failure
> ...
> +       /*
> +        * Autogroup RT tasks are redirected to the root task group
> 	...
> +        * the policy change to proceed.  Thereafter, task_group()
> +        * returns &root_task_group, ...
> +        */

That comment is misleading; if you look at the actual code what we do is
redirect RT programs to _run_ in the root_task_group, but their
task_group() should still be autogroup.

Otherwise people could escape their cgroup by switching to and from a RT
class.

So what I think you want is something like the below; preferably with a
comment on ;-)

---
 kernel/sched/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 1f37fe7f77a4..f4fd048ce7cf 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -7644,6 +7644,9 @@ static inline int tg_has_rt_tasks(struct task_group *tg)
 {
 	struct task_struct *g, *p;
 
+	if (task_group_is_autogroup(tg))
+		return 0;
+
 	for_each_process_thread(g, p) {
 		if (rt_task(p) && task_group(p) == tg)
 			return 1;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/