>> 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
>> ...
>> + /*
>> + * 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.
>
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
I think we can fix it with:
diff --git a/kernel/sched/auto_group.c b/kernel/sched/auto_group.c
index 8a2e230..8c3a169 100644
--- a/kernel/sched/auto_group.c
+++ b/kernel/sched/auto_group.c
@@ -115,9 +115,6 @@ bool task_wants_autogroup(struct task_struct *p, struct
task_group *tg)
if (tg != &root_task_group)
return false;
- if (p->sched_class != &fair_sched_class)
- return false;
-
/*
* We can only assume the task group can't go away on us if
* autogroup_move_group() can see us on ->thread_group list.
> So what I think you want is something like the below; preferably with a
> comment on ;-)
>
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.
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 89e7283..3f6c3ad 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -7460,6 +7460,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;
@@ -7540,6 +7543,9 @@ static int __rt_schedulable(struct task_group *tg, u64
period, u64 runtime)
.rt_runtime = runtime,
};
+ if (tg == &root_task_group && runtime == 0)
+ return -EINVAL;
+
rcu_read_lock();
ret = walk_tg_tree(tg_rt_schedulable, tg_nop, &data);
rcu_read_unlock();
> ---
> 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
