sched, autogroup: Fix failure when writing to cpu.rt_runtime_us

This is how to reproduce the bug:

int main() {
    struct sched_param param = {.sched_priority=1};

    if (fork() > 0)
        exit(0);

    setsid();

    if (sched_setscheduler(0, SCHED_RR, &param) < 0){
        perror("failed to sched_setscheduler()");
        return -1;
    }

    while(1)
        ;
}

  # ./test
  # mount -t cgroup -o cpu xxx /cgroup
  # cat /cgroup/cpu.rt_runtime_us
  950000
  # echo 940000 > /cgroup/cpu.rt_runtime_us
  Device or Resource busy

Quoted 8 lines · Zefan Li

This is how to reproduce the bug:

int main() {
    struct sched_param param = {.sched_priority=1};

    if (fork() > 0)
        exit(0);

Quoted 7 lines

    setsid();

    if (sched_setscheduler(0, SCHED_RR, &param) < 0){
        perror("failed to sched_setscheduler()");
        return -1;
    }

Quoted 10 lines

    while(1)
        ;
}

  # ./test
  # mount -t cgroup -o cpu xxx /cgroup
  # cat /cgroup/cpu.rt_runtime_us
  950000
  # echo 940000 > /cgroup/cpu.rt_runtime_us
  Device or Resource busy

That's -EBUSY, but you're changing an -EPERM condition. Neither your patch nor explanation of the matter make sense.

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.

See this commit:

commit f44937718ce3b8360f72f6c68c9481712517a867

Author: Mike Galbraith <[email protected]>
Date:   Thu Jan 13 04:54:50 2011 +0100

sched, autogroup: Fix CONFIG_RT_GROUP_SCHED sched_setscheduler() failure
...

Quoted 8 lines

See this commit:

commit f44937718ce3b8360f72f6c68c9481712517a867

Author: Mike Galbraith <[email protected]>
Date:   Thu Jan 13 04:54:50 2011 +0100

sched, autogroup: Fix CONFIG_RT_GROUP_SCHED sched_setscheduler() failure
...

Review

Quoted 6 lines

(quoted diff) +5 -0

+       /*
+        * 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(+)

kernel/sched/core.c +3 -0

index 1f37fe7f77a4..f4fd048ce7cf 100644
@@ -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;

.

Before 8323f26ce342 "sched: Fix race in task_group()", task_group() of those RT tasks always return root_task_group, but the escape can't happen.

After commit 8323f26ce342, task_group always return root_task_group except for the case I showed:

1. Change scheduling policy before setsid():

 # cat /proc/sched_debug | grep test
 R           test  4194     24851.893077       945   120     24851.893077     11196.482331         0.000000 /

2. Change policy after setsid():

R test 4142 4962.517723 420 120 4962.517723 4974.126149 0.000000 /autogroup-44

kernel/sched/auto_group.c +0 -0

 This is exactly what I did at first, but besides the issue described above,

seems it might lead to starving RT tasks.

If there's some rt task in autogroups but none in root cgroup, it's allowed to set rt_runtime to 0, so I think we have to disallow this setting, like what we already do with global rt_runtime.

Indeed, setting runtime=0 for the root group is a very bad thing regardless of this patch. It would disallow the kernel from creating RT threads, which it needs for 'correct' operation in a number of cases.

But lets make that a separate patch.

Subject: sched, autogroup: Fix failure to set cpu.rt_runtime_us
From: Peter Zijlstra <[email protected]>
Date: Mon Feb 9 11:53:18 CET 2015

Because task_group() uses a cache of autogroup_task_group(), whoes output depends on sched_class, switching classes can generate problems.

In particular, when started as fair, the cache points to the autogroup, so when switching to RT the tg_rt_schedulable() test fails for every cpu.rt_{runtime,period}_us change because now the autogroup has tasks and no runtime.

Furthermore, going back to the previous semantics of varying task_group() with sched_class has the down-side that the sched_debug output varies as well, even though the task really is in the autogroup.

Therefore add an autogroup exception to tg_has_rt_tasks() -- such that both (all) task_group() usages in sched/core now have one. And remove all the remnants of the variable task_group() output.

Cc Mike Galbraith <[email protected]>

Cc Stefan Bader <[email protected]>

Reported-by Zefan Li <[email protected]>

Fixes 8323f26ce342 ("sched: Fix race in task_group()")

Signed-off-by Peter Zijlstra (Intel) <[email protected]>