autogroup: sched_setscheduler() fails

autogroup: sched_setscheduler() fails

[Bharata B Rao]

Hi,

With autogroup ON, sched_setscheduler() fails when I try to change the
scheduling policy of a normal task to either RR or FIFO.

sched_setscheduler() returns -EPERM when it finds that the group doesn't
have any rt bandwidth. This is expected because with autogroup, the task
is in an autogroup for which task_group(p)->rt_bandwidth.rt_runtime is 0.

I guess the task needs to be moved to appropriate cgroup (from its current
autogroup) when such policy change is requested, but I wasn't sure if
group change from sched_setscheduler() can be achieved easily.

Regards,
Bharata.

[patch] Re: autogroup: sched_setscheduler() fails

[Mike Galbraith]

On Mon, 2011-01-10 at 14:46 +0530, Bharata B Rao wrote:
> Hi,
> 
> With autogroup ON, sched_setscheduler() fails when I try to change the
> scheduling policy of a normal task to either RR or FIFO.
> 
> sched_setscheduler() returns -EPERM when it finds that the group doesn't
> have any rt bandwidth. This is expected because with autogroup, the task
> is in an autogroup for which task_group(p)->rt_bandwidth.rt_runtime is 0.
> 
> I guess the task needs to be moved to appropriate cgroup (from its current
> autogroup) when such policy change is requested, but I wasn't sure if
> group change from sched_setscheduler() can be achieved easily.

Right, autogroup should be excluding CONFIG_RT_GROUP_SCHED, since it
doesn't currently support it.

sched: SCHED_AUTOGROUP selection excludes RT_GROUP_SCHED

Autogroup does not support realtime task groups, so make selection of
SCHED_AUTOGROUP exclude RT_GROUP_SCHED.

Signed-off-by: Mike Galbraith <efault@gmx.de>
Reported-by:Bharata B Rao <bharata@linux.vnet.ibm.com>

---
 init/Kconfig |    5 +++++
 1 file changed, 5 insertions(+)

Index: linux-2.6/init/Kconfig
===================================================================
--- linux-2.6.orig/init/Kconfig
+++ linux-2.6/init/Kconfig
@@ -699,6 +699,7 @@ config RT_GROUP_SCHED
 	bool "Group scheduling for SCHED_RR/FIFO"
 	depends on EXPERIMENTAL
 	depends on CGROUP_SCHED
+	depends on !SCHED_AUTOGROUP
 	default n
 	help
 	  This feature lets you explicitly allocate real CPU bandwidth
@@ -807,6 +808,10 @@ config SCHED_AUTOGROUP
 	  desktop applications.  Task group autogeneration is currently based
 	  upon task session.
 
+	  Note: SCHED_AUTOGROUP does not support realtime tasks, and prevents
+	  enabling RT_GROUP_SCHED.  If you intend to use group scheduling for
+	  realtime tasks, do not enable this option.
+
 config MM_OWNER
 	bool
 

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Peter Zijlstra]

On Mon, 2011-01-10 at 11:29 +0100, Mike Galbraith wrote:
> 
> Right, autogroup should be excluding CONFIG_RT_GROUP_SCHED, since it
> doesn't currently support it.
> 
> sched: SCHED_AUTOGROUP selection excludes RT_GROUP_SCHED
> 
> Autogroup does not support realtime task groups, so make selection of
> SCHED_AUTOGROUP exclude RT_GROUP_SCHED.
> 
> Signed-off-by: Mike Galbraith <efault@gmx.de>
> Reported-by:Bharata B Rao <bharata@linux.vnet.ibm.com>
> 
> ---
>  init/Kconfig |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> Index: linux-2.6/init/Kconfig
> ===================================================================
> --- linux-2.6.orig/init/Kconfig
> +++ linux-2.6/init/Kconfig
> @@ -699,6 +699,7 @@ config RT_GROUP_SCHED
>         bool "Group scheduling for SCHED_RR/FIFO"
>         depends on EXPERIMENTAL
>         depends on CGROUP_SCHED
> +       depends on !SCHED_AUTOGROUP
>         default n
>         help
>           This feature lets you explicitly allocate real CPU bandwidth
> @@ -807,6 +808,10 @@ config SCHED_AUTOGROUP
>           desktop applications.  Task group autogeneration is currently based
>           upon task session.
>  
> +         Note: SCHED_AUTOGROUP does not support realtime tasks, and prevents
> +         enabling RT_GROUP_SCHED.  If you intend to use group scheduling for
> +         realtime tasks, do not enable this option.
> +
>  config MM_OWNER
>         bool
>   

Uhm, no. The right way is to put tasks back into the root group and then
perform sched_setscheduler().

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Mike Galbraith]

On Mon, 2011-01-10 at 11:59 +0100, Peter Zijlstra wrote:
> On Mon, 2011-01-10 at 11:29 +0100, Mike Galbraith wrote:
> >
> > Autogroup does not support realtime task groups, so make selection of
> > SCHED_AUTOGROUP exclude RT_GROUP_SCHED.
> > 

> Uhm, no. The right way is to put tasks back into the root group and then
> perform sched_setscheduler().

I think that would take more lines, this look like a right way too?

sched, autogroup: move tasks to the appropriate runqueue on policy change.

If CONFIG_RT_GROUP_SCHED is set, sched_setscheduler() fails due to autogroup
not allocating rt_runtime.  Fool __sched_setscheduler() into proceeding on, and
move tasks to the appropriate runqueue upon policy change.

Signed-off-by: Mike Galbraith <efault@gmx.de>
Reported-by: Bharata B Rao <bharata@linux.vnet.ibm.com>

---
 kernel/sched.c           |    9 ++++++---
 kernel/sched_autogroup.c |   18 ++++++++++++++++++
 kernel/sched_autogroup.h |    1 +
 3 files changed, 25 insertions(+), 3 deletions(-)

Index: linux-2.6/kernel/sched.c
===================================================================
--- linux-2.6.orig/kernel/sched.c
+++ linux-2.6/kernel/sched.c
@@ -4571,6 +4571,8 @@ void rt_mutex_setprio(struct task_struct
 
 	p->prio = prio;
 
+	autogroup_setscheduler(p, on_rq);
+
 	if (running)
 		p->sched_class->set_curr_task(rq);
 	if (on_rq) {
@@ -4738,7 +4740,7 @@ static struct task_struct *find_process_
 
 /* Actually do priority change: must hold rq lock. */
 static void
-__setscheduler(struct rq *rq, struct task_struct *p, int policy, int prio)
+__setscheduler(struct rq *rq, struct task_struct *p, int policy, int prio, int on_rq)
 {
 	BUG_ON(p->se.on_rq);
 
@@ -4752,6 +4754,7 @@ __setscheduler(struct rq *rq, struct tas
 	else
 		p->sched_class = &fair_sched_class;
 	set_load_weight(p);
+	autogroup_setscheduler(p, on_rq);
 }
 
 /*
@@ -4900,7 +4903,7 @@ recheck:
 
 	oldprio = p->prio;
 	prev_class = p->sched_class;
-	__setscheduler(rq, p, policy, param->sched_priority);
+	__setscheduler(rq, p, policy, param->sched_priority, on_rq);
 
 	if (running)
 		p->sched_class->set_curr_task(rq);
@@ -8116,7 +8119,7 @@ static void normalize_task(struct rq *rq
 	on_rq = p->se.on_rq;
 	if (on_rq)
 		deactivate_task(rq, p, 0);
-	__setscheduler(rq, p, SCHED_NORMAL, 0);
+	__setscheduler(rq, p, SCHED_NORMAL, 0, on_rq);
 	if (on_rq) {
 		activate_task(rq, p, 0);
 		resched_task(rq->curr);
Index: linux-2.6/kernel/sched_autogroup.c
===================================================================
--- linux-2.6.orig/kernel/sched_autogroup.c
+++ linux-2.6/kernel/sched_autogroup.c
@@ -73,6 +73,15 @@ static inline struct autogroup *autogrou
 	ag->id = atomic_inc_return(&autogroup_seq_nr);
 	ag->tg = tg;
 	tg->autogroup = ag;
+#ifdef CONFIG_RT_GROUP_SCHED
+	/*
+	 * HACK: autogroup RT tasks run in the root task group.
+	 * This fools __sched_setscheduler() into proceeding on
+	 * so we can move the task to the appropriate runqueue
+	 * upon scheduling policy change.
+	 */
+	tg->rt_bandwidth.rt_runtime = RUNTIME_INF;
+#endif
 
 	return ag;
 
@@ -143,6 +152,15 @@ autogroup_move_group(struct task_struct
 	autogroup_kref_put(prev);
 }
 
+static inline void
+autogroup_setscheduler(struct task_struct *p, int on_rq)
+{
+	if (p->sched_class->task_move_group)
+		p->sched_class->task_move_group(p, on_rq);
+	else
+		set_task_rq(p, task_cpu(p));
+}
+
 /* Allocates GFP_KERNEL, cannot be called under any spinlock */
 void sched_autogroup_create_attach(struct task_struct *p)
 {
Index: linux-2.6/kernel/sched_autogroup.h
===================================================================
--- linux-2.6.orig/kernel/sched_autogroup.h
+++ linux-2.6/kernel/sched_autogroup.h
@@ -15,6 +15,7 @@ autogroup_task_group(struct task_struct
 
 static inline void autogroup_init(struct task_struct *init_task) {  }
 static inline void autogroup_free(struct task_group *tg) { }
+static inline void autogroup_setscheduler(struct task_struct *p, int on_rq) { }
 
 static inline struct task_group *
 autogroup_task_group(struct task_struct *p, struct task_group *tg)

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Bharata B Rao]

On Mon, Jan 10, 2011 at 05:42:26PM +0100, Mike Galbraith wrote:
> Index: linux-2.6/kernel/sched_autogroup.c
> ===================================================================
> --- linux-2.6.orig/kernel/sched_autogroup.c
> +++ linux-2.6/kernel/sched_autogroup.c
> @@ -73,6 +73,15 @@ static inline struct autogroup *autogrou
>  	ag->id = atomic_inc_return(&autogroup_seq_nr);
>  	ag->tg = tg;
>  	tg->autogroup = ag;
> +#ifdef CONFIG_RT_GROUP_SCHED
> +	/*
> +	 * HACK: autogroup RT tasks run in the root task group.
> +	 * This fools __sched_setscheduler() into proceeding on
> +	 * so we can move the task to the appropriate runqueue
> +	 * upon scheduling policy change.
> +	 */
> +	tg->rt_bandwidth.rt_runtime = RUNTIME_INF;
> +#endif
> 
>  	return ag;
> 
> @@ -143,6 +152,15 @@ autogroup_move_group(struct task_struct
>  	autogroup_kref_put(prev);
>  }
> 
> +static inline void
> +autogroup_setscheduler(struct task_struct *p, int on_rq)
> +{
> +	if (p->sched_class->task_move_group)
> +		p->sched_class->task_move_group(p, on_rq);
> +	else
> +		set_task_rq(p, task_cpu(p));
> +}
> +

IIUC, with the above changes you are actually queing the task into
rt_rq of an autogroup. But the task's autogroup interface
(/proc/<pid>/autogroup) allows you to control the bandwidth of only
cfs_rq tasks, while the rt tasks in the group get RUNTIME_INF bandwidth.

I think what we need is a real group change here (which is difficult I
think) and not just sched_class change.

Regards,
Bharata.

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Mike Galbraith]

On Tue, 2011-01-11 at 22:40 +0530, Bharata B Rao wrote:
> On Mon, Jan 10, 2011 at 05:42:26PM +0100, Mike Galbraith wrote:
> > Index: linux-2.6/kernel/sched_autogroup.c
> > ===================================================================
> > --- linux-2.6.orig/kernel/sched_autogroup.c
> > +++ linux-2.6/kernel/sched_autogroup.c
> > @@ -73,6 +73,15 @@ static inline struct autogroup *autogrou
> >  	ag->id = atomic_inc_return(&autogroup_seq_nr);
> >  	ag->tg = tg;
> >  	tg->autogroup = ag;
> > +#ifdef CONFIG_RT_GROUP_SCHED
> > +	/*
> > +	 * HACK: autogroup RT tasks run in the root task group.
> > +	 * This fools __sched_setscheduler() into proceeding on
> > +	 * so we can move the task to the appropriate runqueue
> > +	 * upon scheduling policy change.
> > +	 */
> > +	tg->rt_bandwidth.rt_runtime = RUNTIME_INF;
> > +#endif
> > 
> >  	return ag;
> > 
> > @@ -143,6 +152,15 @@ autogroup_move_group(struct task_struct
> >  	autogroup_kref_put(prev);
> >  }
> > 
> > +static inline void
> > +autogroup_setscheduler(struct task_struct *p, int on_rq)
> > +{
> > +	if (p->sched_class->task_move_group)
> > +		p->sched_class->task_move_group(p, on_rq);
> > +	else
> > +		set_task_rq(p, task_cpu(p));
> > +}
> > +
> 
> IIUC, with the above changes you are actually queing the task into
> rt_rq of an autogroup. But the task's autogroup interface
> (/proc/<pid>/autogroup) allows you to control the bandwidth of only
> cfs_rq tasks, while the rt tasks in the group get RUNTIME_INF bandwidth.
> 
> I think what we need is a real group change here (which is difficult I
> think) and not just sched_class change.

Unless I fscked up, set_task_rq() is the group change.  As soon as the
task's class changes, it'll be moved to the root_task_group.

	-Mike

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Bharata B Rao]

On Tue, Jan 11, 2011 at 07:48:06PM +0100, Mike Galbraith wrote:
> On Tue, 2011-01-11 at 22:40 +0530, Bharata B Rao wrote:
> > On Mon, Jan 10, 2011 at 05:42:26PM +0100, Mike Galbraith wrote:
> > > Index: linux-2.6/kernel/sched_autogroup.c
> > > ===================================================================
> > > --- linux-2.6.orig/kernel/sched_autogroup.c
> > > +++ linux-2.6/kernel/sched_autogroup.c
> > > @@ -73,6 +73,15 @@ static inline struct autogroup *autogrou
> > >  	ag->id = atomic_inc_return(&autogroup_seq_nr);
> > >  	ag->tg = tg;
> > >  	tg->autogroup = ag;
> > > +#ifdef CONFIG_RT_GROUP_SCHED
> > > +	/*
> > > +	 * HACK: autogroup RT tasks run in the root task group.
> > > +	 * This fools __sched_setscheduler() into proceeding on
> > > +	 * so we can move the task to the appropriate runqueue
> > > +	 * upon scheduling policy change.
> > > +	 */
> > > +	tg->rt_bandwidth.rt_runtime = RUNTIME_INF;
> > > +#endif
> > > 
> > >  	return ag;
> > > 
> > > @@ -143,6 +152,15 @@ autogroup_move_group(struct task_struct
> > >  	autogroup_kref_put(prev);
> > >  }
> > > 
> > > +static inline void
> > > +autogroup_setscheduler(struct task_struct *p, int on_rq)
> > > +{
> > > +	if (p->sched_class->task_move_group)
> > > +		p->sched_class->task_move_group(p, on_rq);
> > > +	else
> > > +		set_task_rq(p, task_cpu(p));
> > > +}
> > > +
> > 
> > IIUC, with the above changes you are actually queing the task into
> > rt_rq of an autogroup. But the task's autogroup interface
> > (/proc/<pid>/autogroup) allows you to control the bandwidth of only
> > cfs_rq tasks, while the rt tasks in the group get RUNTIME_INF bandwidth.
> > 
> > I think what we need is a real group change here (which is difficult I
> > think) and not just sched_class change.
> 
> Unless I fscked up, set_task_rq() is the group change.  As soon as the
> task's class changes, it'll be moved to the root_task_group.

Ok, this is what I understand, may be Peter Z can confirm...

set_task_rq() just changes the task's cfs_rq and rt_rq as per its
task_group(). The normal way to change a task's group is to first
change its cgroup pointer (task->cgroups, see cgroup_attach_task())
After this you change the runqueues by calling set_task_rq() which
now refers to new group's runqueues.

In your code, I don't see where you really change task's group. AFAICS,
it continues to remain in the same autogroup.

Regards,
Bharata.

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Yong Zhang]

On Wed, Jan 12, 2011 at 11:37 AM, Bharata B Rao
<bharata@linux.vnet.ibm.com> wrote:
>> Unless I fscked up, set_task_rq() is the group change.  As soon as the
>> task's class changes, it'll be moved to the root_task_group.
>
> Ok, this is what I understand, may be Peter Z can confirm...
>
> set_task_rq() just changes the task's cfs_rq and rt_rq as per its
> task_group(). The normal way to change a task's group is to first
> change its cgroup pointer (task->cgroups, see cgroup_attach_task())
> After this you change the runqueues by calling set_task_rq() which
> now refers to new group's runqueues.
>
> In your code, I don't see where you really change task's group. AFAICS,
> it continues to remain in the same autogroup.

IMHO, task->cgroups will not change when autogroup take effect.
IOW, autogroup will only take effect when task belongs to root_task_group.

Thanks,
Yong

>
> Regards,
> Bharata.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>



-- 
Only stand for myself

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Mike Galbraith]

On Tue, 2011-01-11 at 19:48 +0100, Mike Galbraith wrote:
> On Tue, 2011-01-11 at 22:40 +0530, Bharata B Rao wrote:
> > On Mon, Jan 10, 2011 at 05:42:26PM +0100, Mike Galbraith wrote:
> > > Index: linux-2.6/kernel/sched_autogroup.c
> > > ===================================================================
> > > --- linux-2.6.orig/kernel/sched_autogroup.c
> > > +++ linux-2.6/kernel/sched_autogroup.c
> > > @@ -73,6 +73,15 @@ static inline struct autogroup *autogrou
> > >  	ag->id = atomic_inc_return(&autogroup_seq_nr);
> > >  	ag->tg = tg;
> > >  	tg->autogroup = ag;
> > > +#ifdef CONFIG_RT_GROUP_SCHED
> > > +	/*
> > > +	 * HACK: autogroup RT tasks run in the root task group.
> > > +	 * This fools __sched_setscheduler() into proceeding on
> > > +	 * so we can move the task to the appropriate runqueue
> > > +	 * upon scheduling policy change.
> > > +	 */
> > > +	tg->rt_bandwidth.rt_runtime = RUNTIME_INF;
> > > +#endif
> > > 
> > >  	return ag;
> > > 
> > > @@ -143,6 +152,15 @@ autogroup_move_group(struct task_struct
> > >  	autogroup_kref_put(prev);
> > >  }
> > > 
> > > +static inline void
> > > +autogroup_setscheduler(struct task_struct *p, int on_rq)
> > > +{
> > > +	if (p->sched_class->task_move_group)
> > > +		p->sched_class->task_move_group(p, on_rq);
> > > +	else
> > > +		set_task_rq(p, task_cpu(p));
> > > +}
> > > +
> > 
> > IIUC, with the above changes you are actually queing the task into
> > rt_rq of an autogroup. But the task's autogroup interface
> > (/proc/<pid>/autogroup) allows you to control the bandwidth of only
> > cfs_rq tasks, while the rt tasks in the group get RUNTIME_INF bandwidth.
> > 
> > I think what we need is a real group change here (which is difficult I
> > think) and not just sched_class change.
> 
> Unless I fscked up, set_task_rq() is the group change.  As soon as the
> task's class changes, it'll be moved to the root_task_group.

But it'd be better to not touch anything, just deflect RT tasks away
from useless queues from the start.

sched, autogroup: fix CONFIG_RT_GROUP_SCHED sched_setscheduler() failure.

If CONFIG_RT_GROUP_SCHED is set, sched_setscheduler() fails due to autogroup
not allocating rt_runtime.  Squirrel allocated but useless rt_se and rt_rq
away for group destruction time, and deflect RT tasks to the root task group.

Signed-off-by: Mike Galbraith <efault@gmx.de>
Reported-by: Bharata B Rao <bharata@linux.vnet.ibm.com>

---
 kernel/sched_autogroup.c |   13 +++++++++++++
 kernel/sched_autogroup.h |    4 ++++
 2 files changed, 17 insertions(+)

Index: linux-2.6/kernel/sched_autogroup.c
===================================================================
--- linux-2.6.orig/kernel/sched_autogroup.c
+++ linux-2.6/kernel/sched_autogroup.c
@@ -27,6 +27,11 @@ static inline void autogroup_destroy(str
 {
 	struct autogroup *ag = container_of(kref, struct autogroup, kref);
 
+#ifdef CONFIG_RT_GROUP_SCHED
+	ag->tg->rt_se = ag->rt_se;
+	ag->tg->rt_rq = ag->rt_rq;
+	ag->tg->rt_bandwidth.rt_runtime = 0;
+#endif
 	sched_destroy_group(ag->tg);
 }
 
@@ -72,6 +77,14 @@ static inline struct autogroup *autogrou
 	init_rwsem(&ag->lock);
 	ag->id = atomic_inc_return(&autogroup_seq_nr);
 	ag->tg = tg;
+#ifdef CONFIG_RT_GROUP_SCHED
+	/* Sorry, we don't do RT, go away. */
+	ag->rt_se = tg->rt_se;
+	ag->rt_rq = tg->rt_rq;
+	tg->rt_se = root_task_group.rt_se;
+	tg->rt_rq = root_task_group.rt_rq;
+	tg->rt_bandwidth.rt_runtime = root_task_group.rt_bandwidth.rt_runtime;
+#endif
 	tg->autogroup = ag;
 
 	return ag;
Index: linux-2.6/kernel/sched_autogroup.h
===================================================================
--- linux-2.6.orig/kernel/sched_autogroup.h
+++ linux-2.6/kernel/sched_autogroup.h
@@ -6,6 +6,10 @@ struct autogroup {
 	struct rw_semaphore	lock;
 	unsigned long		id;
 	int			nice;
+#ifdef CONFIG_RT_GROUP_SCHED
+	struct sched_rt_entity **rt_se;
+	struct rt_rq **rt_rq;
+#endif
 };
 
 static inline struct task_group *
 

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Yong Zhang]

On Tue, Jan 11, 2011 at 12:42 AM, Mike Galbraith <efault@gmx.de> wrote:
> Index: linux-2.6/kernel/sched_autogroup.c
> ===================================================================
> --- linux-2.6.orig/kernel/sched_autogroup.c
> +++ linux-2.6/kernel/sched_autogroup.c
> @@ -73,6 +73,15 @@ static inline struct autogroup *autogrou
>        ag->id = atomic_inc_return(&autogroup_seq_nr);
>        ag->tg = tg;
>        tg->autogroup = ag;
> +#ifdef CONFIG_RT_GROUP_SCHED
> +       /*
> +        * HACK: autogroup RT tasks run in the root task group.
> +        * This fools __sched_setscheduler() into proceeding on
> +        * so we can move the task to the appropriate runqueue
> +        * upon scheduling policy change.
> +        */
> +       tg->rt_bandwidth.rt_runtime = RUNTIME_INF;
> +#endif
>
>        return ag;
>
> @@ -143,6 +152,15 @@ autogroup_move_group(struct task_struct
>        autogroup_kref_put(prev);
>  }
>
> +static inline void
> +autogroup_setscheduler(struct task_struct *p, int on_rq)
> +{
> +       if (p->sched_class->task_move_group)
> +               p->sched_class->task_move_group(p, on_rq);

This will be called even if we don't change the sched_class in
which case it is not necessary.

Thanks,
Yong

-- 
Only stand for myself

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Mike Galbraith]

On Wed, 2011-01-12 at 09:07 +0530, Bharata B Rao wrote:
> On Tue, Jan 11, 2011 at 07:48:06PM +0100, Mike Galbraith wrote:
> > On Tue, 2011-01-11 at 22:40 +0530, Bharata B Rao wrote:
> > > On Mon, Jan 10, 2011 at 05:42:26PM +0100, Mike Galbraith wrote:
> > > > Index: linux-2.6/kernel/sched_autogroup.c
> > > > ===================================================================
> > > > --- linux-2.6.orig/kernel/sched_autogroup.c
> > > > +++ linux-2.6/kernel/sched_autogroup.c
> > > > @@ -73,6 +73,15 @@ static inline struct autogroup *autogrou
> > > >  	ag->id = atomic_inc_return(&autogroup_seq_nr);
> > > >  	ag->tg = tg;
> > > >  	tg->autogroup = ag;
> > > > +#ifdef CONFIG_RT_GROUP_SCHED
> > > > +	/*
> > > > +	 * HACK: autogroup RT tasks run in the root task group.
> > > > +	 * This fools __sched_setscheduler() into proceeding on
> > > > +	 * so we can move the task to the appropriate runqueue
> > > > +	 * upon scheduling policy change.
> > > > +	 */
> > > > +	tg->rt_bandwidth.rt_runtime = RUNTIME_INF;
> > > > +#endif
> > > > 
> > > >  	return ag;
> > > > 
> > > > @@ -143,6 +152,15 @@ autogroup_move_group(struct task_struct
> > > >  	autogroup_kref_put(prev);
> > > >  }
> > > > 
> > > > +static inline void
> > > > +autogroup_setscheduler(struct task_struct *p, int on_rq)
> > > > +{
> > > > +	if (p->sched_class->task_move_group)
> > > > +		p->sched_class->task_move_group(p, on_rq);
> > > > +	else
> > > > +		set_task_rq(p, task_cpu(p));
> > > > +}
> > > > +
> > > 
> > > IIUC, with the above changes you are actually queing the task into
> > > rt_rq of an autogroup. But the task's autogroup interface
> > > (/proc/<pid>/autogroup) allows you to control the bandwidth of only
> > > cfs_rq tasks, while the rt tasks in the group get RUNTIME_INF bandwidth.
> > > 
> > > I think what we need is a real group change here (which is difficult I
> > > think) and not just sched_class change.
> > 
> > Unless I fscked up, set_task_rq() is the group change.  As soon as the
> > task's class changes, it'll be moved to the root_task_group.
> 
> Ok, this is what I understand, may be Peter Z can confirm...
> 
> set_task_rq() just changes the task's cfs_rq and rt_rq as per its
> task_group().

Exactly.  When you call task_group() for an RT task, it returns
&root_task_group, so a task making the transition to RT will be dequeued
from it's autogroup cfs_rq, and enqueued in root_task_group's rt_rq.

>  The normal way to change a task's group is to first
> change its cgroup pointer (task->cgroups, see cgroup_attach_task())
> After this you change the runqueues by calling set_task_rq() which
> now refers to new group's runqueues.

Autogroup doesn't need to do cgroup stuff.  If a task is in a cgroup,
it's autogroup remains intact, but is ignored.  Move it back to root,
and it's autogroup will be used again.

	-Mike

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Mike Galbraith]

On Wed, 2011-01-12 at 13:43 +0800, Yong Zhang wrote:
> On Tue, Jan 11, 2011 at 12:42 AM, Mike Galbraith <efault@gmx.de> wrote:
> > Index: linux-2.6/kernel/sched_autogroup.c
> > ===================================================================
> > --- linux-2.6.orig/kernel/sched_autogroup.c
> > +++ linux-2.6/kernel/sched_autogroup.c
> > @@ -73,6 +73,15 @@ static inline struct autogroup *autogrou
> >        ag->id = atomic_inc_return(&autogroup_seq_nr);
> >        ag->tg = tg;
> >        tg->autogroup = ag;
> > +#ifdef CONFIG_RT_GROUP_SCHED
> > +       /*
> > +        * HACK: autogroup RT tasks run in the root task group.
> > +        * This fools __sched_setscheduler() into proceeding on
> > +        * so we can move the task to the appropriate runqueue
> > +        * upon scheduling policy change.
> > +        */
> > +       tg->rt_bandwidth.rt_runtime = RUNTIME_INF;
> > +#endif
> >
> >        return ag;
> >
> > @@ -143,6 +152,15 @@ autogroup_move_group(struct task_struct
> >        autogroup_kref_put(prev);
> >  }
> >
> > +static inline void
> > +autogroup_setscheduler(struct task_struct *p, int on_rq)
> > +{
> > +       if (p->sched_class->task_move_group)
> > +               p->sched_class->task_move_group(p, on_rq);
> 
> This will be called even if we don't change the sched_class in
> which case it is not necessary.

Yes.  It'll also piddle around with tasks it shouldn't.  I sent what I
hope is a better solution a few minutes ago.  This one works, but
probably upset Peter's tummy mightily :)

	-Mike

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Yong Zhang]

On Wed, Jan 12, 2011 at 1:40 PM, Mike Galbraith <mgalbraith@suse.de> wrote:
> But it'd be better to not touch anything, just deflect RT tasks away
> from useless queues from the start.
>
> sched, autogroup: fix CONFIG_RT_GROUP_SCHED sched_setscheduler() failure.
>
> If CONFIG_RT_GROUP_SCHED is set, sched_setscheduler() fails due to autogroup
> not allocating rt_runtime.  Squirrel allocated but useless rt_se and rt_rq
> away for group destruction time, and deflect RT tasks to the root task group.
>
> Signed-off-by: Mike Galbraith <efault@gmx.de>
> Reported-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
>
> ---
>  kernel/sched_autogroup.c |   13 +++++++++++++
>  kernel/sched_autogroup.h |    4 ++++
>  2 files changed, 17 insertions(+)
>
> Index: linux-2.6/kernel/sched_autogroup.c
> ===================================================================
> --- linux-2.6.orig/kernel/sched_autogroup.c
> +++ linux-2.6/kernel/sched_autogroup.c
> @@ -27,6 +27,11 @@ static inline void autogroup_destroy(str
>  {
>        struct autogroup *ag = container_of(kref, struct autogroup, kref);
>
> +#ifdef CONFIG_RT_GROUP_SCHED
> +       ag->tg->rt_se = ag->rt_se;
> +       ag->tg->rt_rq = ag->rt_rq;
> +       ag->tg->rt_bandwidth.rt_runtime = 0;
> +#endif
>        sched_destroy_group(ag->tg);
>  }
>
> @@ -72,6 +77,14 @@ static inline struct autogroup *autogrou
>        init_rwsem(&ag->lock);
>        ag->id = atomic_inc_return(&autogroup_seq_nr);
>        ag->tg = tg;
> +#ifdef CONFIG_RT_GROUP_SCHED
> +       /* Sorry, we don't do RT, go away. */
> +       ag->rt_se = tg->rt_se;
> +       ag->rt_rq = tg->rt_rq;
> +       tg->rt_se = root_task_group.rt_se;
> +       tg->rt_rq = root_task_group.rt_rq;
> +       tg->rt_bandwidth.rt_runtime = root_task_group.rt_bandwidth.rt_runtime;

Setting rt_runtime both in this patch and the previous one will make tune
other's rt_runtime/rt_period trouble, I guess.

Thanks,
Yong

-- 
Only stand for myself

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Bharata B Rao]

On Wed, Jan 12, 2011 at 01:40:32PM +0800, Yong Zhang wrote:
> On Wed, Jan 12, 2011 at 11:37 AM, Bharata B Rao
> <bharata@linux.vnet.ibm.com> wrote:
> >> Unless I fscked up, set_task_rq() is the group change.  As soon as the
> >> task's class changes, it'll be moved to the root_task_group.
> >
> > Ok, this is what I understand, may be Peter Z can confirm...
> >
> > set_task_rq() just changes the task's cfs_rq and rt_rq as per its
> > task_group(). The normal way to change a task's group is to first
> > change its cgroup pointer (task->cgroups, see cgroup_attach_task())
> > After this you change the runqueues by calling set_task_rq() which
> > now refers to new group's runqueues.
> >
> > In your code, I don't see where you really change task's group. AFAICS,
> > it continues to remain in the same autogroup.
> 
> IMHO, task->cgroups will not change when autogroup take effect.

Yes and I believe this is cause for some of the weird semantics I see
with autogroup and cgroups. I am not sure if this has already been
discussed ealier, may be I need to go back and check the archives, but
consider this:

I have cpu cgroup mounted at /cgroup and see that all the tasks in the
system are listed in /cgroup/tasks file.

Now I start a task like this:

# ./while1 &
[1] 2761

and see that this task belongs to root cgroup.

# grep 2761 /cgroup/tasks
2671

But we know that this task really belongs to an autogroup and is not
sitting directly on root_task_group.

# cat /proc/2761/autogroup 
/autogroup-49 nice 0

So we have a task in an autogroup (which is a sub group of root_task_group)
but is being shown as part of root_task_group. Is this by design ?

Now say I create a sub cgroup and move this task to it.

# mkdir /cgroup/1
# echo 2671 > /cgroup/1/tasks

So now the task moved to a real cgroup.

# cat /cgroup/1/tasks 
2761

But the /proc/2761/autogroup isn't updated. It still shows
/autogroup-49 nice 0

May be this was all discussed earlier as I noted in the beginning, but I find
this semantics a bit unusual.

Regards,
Bharata.

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Bharata B Rao]

On Wed, Jan 12, 2011 at 07:17:52AM +0100, Mike Galbraith wrote:
> On Wed, 2011-01-12 at 09:07 +0530, Bharata B Rao wrote:
> > On Tue, Jan 11, 2011 at 07:48:06PM +0100, Mike Galbraith wrote:
> > > On Tue, 2011-01-11 at 22:40 +0530, Bharata B Rao wrote:
> > > > On Mon, Jan 10, 2011 at 05:42:26PM +0100, Mike Galbraith wrote:
> > > > > Index: linux-2.6/kernel/sched_autogroup.c
> > > > > ===================================================================
> > > > > --- linux-2.6.orig/kernel/sched_autogroup.c
> > > > > +++ linux-2.6/kernel/sched_autogroup.c
> > > > > @@ -73,6 +73,15 @@ static inline struct autogroup *autogrou
> > > > >  	ag->id = atomic_inc_return(&autogroup_seq_nr);
> > > > >  	ag->tg = tg;
> > > > >  	tg->autogroup = ag;
> > > > > +#ifdef CONFIG_RT_GROUP_SCHED
> > > > > +	/*
> > > > > +	 * HACK: autogroup RT tasks run in the root task group.
> > > > > +	 * This fools __sched_setscheduler() into proceeding on
> > > > > +	 * so we can move the task to the appropriate runqueue
> > > > > +	 * upon scheduling policy change.
> > > > > +	 */
> > > > > +	tg->rt_bandwidth.rt_runtime = RUNTIME_INF;
> > > > > +#endif
> > > > > 
> > > > >  	return ag;
> > > > > 
> > > > > @@ -143,6 +152,15 @@ autogroup_move_group(struct task_struct
> > > > >  	autogroup_kref_put(prev);
> > > > >  }
> > > > > 
> > > > > +static inline void
> > > > > +autogroup_setscheduler(struct task_struct *p, int on_rq)
> > > > > +{
> > > > > +	if (p->sched_class->task_move_group)
> > > > > +		p->sched_class->task_move_group(p, on_rq);
> > > > > +	else
> > > > > +		set_task_rq(p, task_cpu(p));
> > > > > +}
> > > > > +
> > > > 
> > > > IIUC, with the above changes you are actually queing the task into
> > > > rt_rq of an autogroup. But the task's autogroup interface
> > > > (/proc/<pid>/autogroup) allows you to control the bandwidth of only
> > > > cfs_rq tasks, while the rt tasks in the group get RUNTIME_INF bandwidth.
> > > > 
> > > > I think what we need is a real group change here (which is difficult I
> > > > think) and not just sched_class change.
> > > 
> > > Unless I fscked up, set_task_rq() is the group change.  As soon as the
> > > task's class changes, it'll be moved to the root_task_group.
> > 
> > Ok, this is what I understand, may be Peter Z can confirm...
> > 
> > set_task_rq() just changes the task's cfs_rq and rt_rq as per its
> > task_group().
> 
> Exactly.  When you call task_group() for an RT task, it returns
> &root_task_group, so a task making the transition to RT will be dequeued
> from it's autogroup cfs_rq, and enqueued in root_task_group's rt_rq.

Oh yes, forgot that addition to task_group() for a moment!

Regards,
Bharata.

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Mike Galbraith]

On Wed, 2011-01-12 at 12:05 +0530, Bharata B Rao wrote:
> On Wed, Jan 12, 2011 at 01:40:32PM +0800, Yong Zhang wrote:

> > IMHO, task->cgroups will not change when autogroup take effect.
> 
> Yes and I believe this is cause for some of the weird semantics I see
> with autogroup and cgroups. I am not sure if this has already been
> discussed ealier, may be I need to go back and check the archives, but
> consider this:
> 
> I have cpu cgroup mounted at /cgroup and see that all the tasks in the
> system are listed in /cgroup/tasks file.
> 
> Now I start a task like this:
> 
> # ./while1 &
> [1] 2761
> 
> and see that this task belongs to root cgroup.

Yes, cgroups sees cgroup associations, not autogroup associations.

> # grep 2761 /cgroup/tasks
> 2671
> 
> But we know that this task really belongs to an autogroup and is not
> sitting directly on root_task_group.

Yup, cgroups doesn't want it, so autogroup (if enabled) claims it.

> # cat /proc/2761/autogroup 
> /autogroup-49 nice 0
> 
> So we have a task in an autogroup (which is a sub group of root_task_group)
> but is being shown as part of root_task_group. Is this by design ?

Yes, it's supposed to be transparent to cgroups.

> Now say I create a sub cgroup and move this task to it.
> 
> # mkdir /cgroup/1
> # echo 2671 > /cgroup/1/tasks
> 
> So now the task moved to a real cgroup.
> 
> # cat /cgroup/1/tasks 
> 2761
> 
> But the /proc/2761/autogroup isn't updated. It still shows
> /autogroup-49 nice 0

The proc interface shows task->autogroup associations, which always
exist.  This is autogroups' view of the world, just as cgroup/tasks is
cgroups' view of it's world.

> May be this was all discussed earlier as I noted in the beginning, but I find
> this semantics a bit unusual.

Makes perfect sense to me.

	-Mike

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Bharata B Rao]

On Wed, Jan 12, 2011 at 08:24:41AM +0100, Mike Galbraith wrote:
> On Wed, 2011-01-12 at 12:05 +0530, Bharata B Rao wrote:
> > On Wed, Jan 12, 2011 at 01:40:32PM +0800, Yong Zhang wrote:
> 
> > > IMHO, task->cgroups will not change when autogroup take effect.
> > 
> > Yes and I believe this is cause for some of the weird semantics I see
> > with autogroup and cgroups. I am not sure if this has already been
> > discussed ealier, may be I need to go back and check the archives, but
> > consider this:
> > 
> > I have cpu cgroup mounted at /cgroup and see that all the tasks in the
> > system are listed in /cgroup/tasks file.
> > 
> > Now I start a task like this:
> > 
> > # ./while1 &
> > [1] 2761
> > 
> > and see that this task belongs to root cgroup.
> 
> Yes, cgroups sees cgroup associations, not autogroup associations.
> 
> > # grep 2761 /cgroup/tasks
> > 2671
> > 
> > But we know that this task really belongs to an autogroup and is not
> > sitting directly on root_task_group.
> 
> Yup, cgroups doesn't want it, so autogroup (if enabled) claims it.
> 
> > # cat /proc/2761/autogroup 
> > /autogroup-49 nice 0
> > 
> > So we have a task in an autogroup (which is a sub group of root_task_group)
> > but is being shown as part of root_task_group. Is this by design ?
> 
> Yes, it's supposed to be transparent to cgroups.

If cgroups doesn't want that task and if its transparent to cgroups,
why is it shown as part of /cgroups/tasks ? The task really doesn't
belong in there. Its not on the runqueue of root_task_group.

So basically I feel that a task consumed by autogroup shouldn't ideally be
shown in root cgroup's tasks file.

Regards,
Bharata.

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Mike Galbraith]

On Wed, 2011-01-12 at 13:36 +0530, Bharata B Rao wrote:
> On Wed, Jan 12, 2011 at 08:24:41AM +0100, Mike Galbraith wrote:

> > Yes, it's supposed to be transparent to cgroups.
> 
> If cgroups doesn't want that task and if its transparent to cgroups,
> why is it shown as part of /cgroups/tasks ? The task really doesn't
> belong in there. Its not on the runqueue of root_task_group.
> 
> So basically I feel that a task consumed by autogroup shouldn't ideally be
> shown in root cgroup's tasks file.

>From cgroups' viewpoint, root is where it's at.  The task isn't consumed
by autogroup either, it's only borrowed until cgroups claims it.

Would you prefer that when you mount /cgroups, you see a nearly empty
root task list because box was booted with autogroup enabled?  Should
tasks appear and disappear on the fly while you're watching?

The goal is peaceful coexistence.  The user can turn either or both both
on/off at his/her whim.

	-Mike

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Mike Galbraith]

On Wed, 2011-01-12 at 14:32 +0800, Yong Zhang wrote:

> Setting rt_runtime both in this patch and the previous one will make tune
> other's rt_runtime/rt_period trouble, I guess.

Hrmph, how annoying.  I'll take another look at rt_runtime.

	-Mike

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Bharata B Rao]

On Wed, Jan 12, 2011 at 09:47:19AM +0100, Mike Galbraith wrote:
> On Wed, 2011-01-12 at 13:36 +0530, Bharata B Rao wrote:
> > On Wed, Jan 12, 2011 at 08:24:41AM +0100, Mike Galbraith wrote:
> 
> > > Yes, it's supposed to be transparent to cgroups.
> > 
> > If cgroups doesn't want that task and if its transparent to cgroups,
> > why is it shown as part of /cgroups/tasks ? The task really doesn't
> > belong in there. Its not on the runqueue of root_task_group.
> > 
> > So basically I feel that a task consumed by autogroup shouldn't ideally be
> > shown in root cgroup's tasks file.
> 
> >From cgroups' viewpoint, root is where it's at.  The task isn't consumed
> by autogroup either, it's only borrowed until cgroups claims it.
> 
> Would you prefer that when you mount /cgroups, you see a nearly empty
> root task list because box was booted with autogroup enabled?

If I had my way, I would say yes :) I would even say make autogroups visible
so that we know exactly what it is. Let 'auto' in autogroups mean only
'automatic' and not necessarily 'hidden'.

> Should
> tasks appear and disappear on the fly while you're watching?

Why would they unless we explicitly move them ?

Anyway I guess you would have gone through these questions already.
Thanks for patiently answering :)

Regards,
Bharata.

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Mike Galbraith]

(mailer switched me to work account again.  grrrr, switch yet again:)

On Wed, 2011-01-12 at 14:32 +0800, Yong Zhang wrote:

> Setting rt_runtime both in this patch and the previous one will make tune
> other's rt_runtime/rt_period trouble, I guess.

Yeah, as in making it impossible to allocate even 1 usec :)  We need
zero bandwidth though, so tell  __sched_setscheduler() that autogroups
having zero allocated is perfectly fine.

sched, autogroup: fix CONFIG_RT_GROUP_SCHED sched_setscheduler() failure.

If CONFIG_RT_GROUP_SCHED is set, __sched_setscheduler() fails due to autogroup
not allocating rt_runtime.  Free unused/unusable rt_se and rt_rq, redirect RT
tasks to the root task group, and tell __sched_setscheduler() that it's ok.

Signed-off-by: Mike Galbraith <efault@gmx.de>
Reported-by: Bharata B Rao <bharata@linux.vnet.ibm.com>

---
 kernel/sched.c           |    3 ++-
 kernel/sched_autogroup.c |   27 +++++++++++++++++++++++++++
 kernel/sched_autogroup.h |    4 ++++
 3 files changed, 33 insertions(+), 1 deletion(-)

Index: linux-2.6/kernel/sched_autogroup.c
===================================================================
--- linux-2.6.orig/kernel/sched_autogroup.c
+++ linux-2.6/kernel/sched_autogroup.c
@@ -27,6 +27,11 @@ static inline void autogroup_destroy(str
 {
 	struct autogroup *ag = container_of(kref, struct autogroup, kref);
 
+#ifdef CONFIG_RT_GROUP_SCHED
+	/* We've redirected RT tasks to the root task group... */
+	ag->tg->rt_se = NULL;
+	ag->tg->rt_rq = NULL;
+#endif
 	sched_destroy_group(ag->tg);
 }
 
@@ -55,6 +60,10 @@ static inline struct autogroup *autogrou
 	return ag;
 }
 
+#ifdef CONFIG_RT_GROUP_SCHED
+static void free_rt_sched_group(struct task_group *tg);
+#endif
+
 static inline struct autogroup *autogroup_create(void)
 {
 	struct autogroup *ag = kzalloc(sizeof(*ag), GFP_KERNEL);
@@ -72,6 +81,19 @@ static inline struct autogroup *autogrou
 	init_rwsem(&ag->lock);
 	ag->id = atomic_inc_return(&autogroup_seq_nr);
 	ag->tg = tg;
+#ifdef CONFIG_RT_GROUP_SCHED
+	/*
+	 * Autogroup RT tasks are redirected to the root task group
+	 * so we don't have to move tasks around upon policy change,
+	 * or flail around trying to allocate bandwidth on the fly.
+	 * A bandwidth exception in __sched_setscheduler() allows
+	 * the policy change to proceed.  Thereafter, task_group()
+	 * returns &root_task_group, so zero bandwidth is required.
+	 */
+	free_rt_sched_group(tg);
+	tg->rt_se = root_task_group.rt_se;
+	tg->rt_rq = root_task_group.rt_rq;
+#endif
 	tg->autogroup = ag;
 
 	return ag;
@@ -106,6 +128,11 @@ task_wants_autogroup(struct task_struct
 	return true;
 }
 
+static inline bool task_group_is_autogroup(struct task_group *tg)
+{
+	return tg != &root_task_group && tg->autogroup;
+}
+
 static inline struct task_group *
 autogroup_task_group(struct task_struct *p, struct task_group *tg)
 {
Index: linux-2.6/kernel/sched_autogroup.h
===================================================================
--- linux-2.6.orig/kernel/sched_autogroup.h
+++ linux-2.6/kernel/sched_autogroup.h
@@ -15,6 +15,10 @@ autogroup_task_group(struct task_struct
 
 static inline void autogroup_init(struct task_struct *init_task) {  }
 static inline void autogroup_free(struct task_group *tg) { }
+static inline bool task_group_is_autogroup(struct task_group *tg)
+{
+	return 0;
+}
 
 static inline struct task_group *
 autogroup_task_group(struct task_struct *p, struct task_group *tg)
Index: linux-2.6/kernel/sched.c
===================================================================
--- linux-2.6.orig/kernel/sched.c
+++ linux-2.6/kernel/sched.c
@@ -4874,7 +4874,8 @@ recheck:
 		 * assigned.
 		 */
 		if (rt_bandwidth_enabled() && rt_policy(policy) &&
-				task_group(p)->rt_bandwidth.rt_runtime == 0) {
+				task_group(p)->rt_bandwidth.rt_runtime == 0 &&
+				!task_group_is_autogroup(task_group(p))) {
 			__task_rq_unlock(rq);
 			raw_spin_unlock_irqrestore(&p->pi_lock, flags);
 			return -EPERM;

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Yong Zhang]

On Thu, Jan 13, 2011 at 11:54 AM, Mike Galbraith <efault@gmx.de> wrote:
> sched, autogroup: fix CONFIG_RT_GROUP_SCHED sched_setscheduler() failure.
>
> If CONFIG_RT_GROUP_SCHED is set, __sched_setscheduler() fails due to autogroup
> not allocating rt_runtime.  Free unused/unusable rt_se and rt_rq, redirect RT
> tasks to the root task group, and tell __sched_setscheduler() that it's ok.
>
> Signed-off-by: Mike Galbraith <efault@gmx.de>
> Reported-by: Bharata B Rao <bharata@linux.vnet.ibm.com>

This looks more clear ;)

And a little comment below

>
> ---
>  kernel/sched.c           |    3 ++-
>  kernel/sched_autogroup.c |   27 +++++++++++++++++++++++++++
>  kernel/sched_autogroup.h |    4 ++++
>  3 files changed, 33 insertions(+), 1 deletion(-)
> @@ -106,6 +128,11 @@ task_wants_autogroup(struct task_struct
>        return true;
>  }
>
> +static inline bool task_group_is_autogroup(struct task_group *tg)
> +{
> +       return tg != &root_task_group && tg->autogroup;

Isn't just checking tg->autogroup sufficient?

if tg == &root_task_group

> --- linux-2.6.orig/kernel/sched.c
> +++ linux-2.6/kernel/sched.c
> @@ -4874,7 +4874,8 @@ recheck:
>                 * assigned.
>                 */
>                if (rt_bandwidth_enabled() && rt_policy(policy) &&
> -                               task_group(p)->rt_bandwidth.rt_runtime == 0) {
> +                               task_group(p)->rt_bandwidth.rt_runtime == 0 &&

this check will fail.

Thanks,
Yong

-- 
Only stand for myself

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Yong Zhang]

On Thu, Jan 13, 2011 at 1:59 PM, Yong Zhang <yong.zhang0@gmail.com> wrote:
> On Thu, Jan 13, 2011 at 11:54 AM, Mike Galbraith <efault@gmx.de> wrote:
>> sched, autogroup: fix CONFIG_RT_GROUP_SCHED sched_setscheduler() failure.
>>
>> If CONFIG_RT_GROUP_SCHED is set, __sched_setscheduler() fails due to autogroup
>> not allocating rt_runtime.  Free unused/unusable rt_se and rt_rq, redirect RT
>> tasks to the root task group, and tell __sched_setscheduler() that it's ok.
>>
>> Signed-off-by: Mike Galbraith <efault@gmx.de>
>> Reported-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
>
> This looks more clear ;)
>
> And a little comment below
>
>>
>> ---
>>  kernel/sched.c           |    3 ++-
>>  kernel/sched_autogroup.c |   27 +++++++++++++++++++++++++++
>>  kernel/sched_autogroup.h |    4 ++++
>>  3 files changed, 33 insertions(+), 1 deletion(-)
>> @@ -106,6 +128,11 @@ task_wants_autogroup(struct task_struct
>>        return true;
>>  }
>>
>> +static inline bool task_group_is_autogroup(struct task_group *tg)
>> +{
>> +       return tg != &root_task_group && tg->autogroup;
>
> Isn't just checking tg->autogroup sufficient?

But if task_group_is_autogroup() will be called in other place in the future,
checking (tg != &root_task_group) is needed.

>
> if tg == &root_task_group
>
>> --- linux-2.6.orig/kernel/sched.c
>> +++ linux-2.6/kernel/sched.c
>> @@ -4874,7 +4874,8 @@ recheck:
>>                 * assigned.
>>                 */
>>                if (rt_bandwidth_enabled() && rt_policy(policy) &&
>> -                               task_group(p)->rt_bandwidth.rt_runtime == 0) {
>> +                               task_group(p)->rt_bandwidth.rt_runtime == 0 &&
>
> this check will fail.
>
> Thanks,
> Yong
>
> --
> Only stand for myself
>



-- 
Only stand for myself

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Mike Galbraith]

On Thu, 2011-01-13 at 13:59 +0800, Yong Zhang wrote:
> On Thu, Jan 13, 2011 at 11:54 AM, Mike Galbraith <efault@gmx.de> wrote:
> > sched, autogroup: fix CONFIG_RT_GROUP_SCHED sched_setscheduler() failure.
> >
> > If CONFIG_RT_GROUP_SCHED is set, __sched_setscheduler() fails due to autogroup
> > not allocating rt_runtime.  Free unused/unusable rt_se and rt_rq, redirect RT
> > tasks to the root task group, and tell __sched_setscheduler() that it's ok.
> >
> > Signed-off-by: Mike Galbraith <efault@gmx.de>
> > Reported-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> 
> This looks more clear ;)
> 
> And a little comment below
> 
> >
> > ---
> >  kernel/sched.c           |    3 ++-
> >  kernel/sched_autogroup.c |   27 +++++++++++++++++++++++++++
> >  kernel/sched_autogroup.h |    4 ++++
> >  3 files changed, 33 insertions(+), 1 deletion(-)
> > @@ -106,6 +128,11 @@ task_wants_autogroup(struct task_struct
> >        return true;
> >  }
> >
> > +static inline bool task_group_is_autogroup(struct task_group *tg)
> > +{
> > +       return tg != &root_task_group && tg->autogroup;
> 
> Isn't just checking tg->autogroup sufficient?
> 
> if tg == &root_task_group
> 
> > --- linux-2.6.orig/kernel/sched.c
> > +++ linux-2.6/kernel/sched.c
> > @@ -4874,7 +4874,8 @@ recheck:
> >                 * assigned.
> >                 */
> >                if (rt_bandwidth_enabled() && rt_policy(policy) &&
> > -                               task_group(p)->rt_bandwidth.rt_runtime == 0) {
> > +                               task_group(p)->rt_bandwidth.rt_runtime == 0 &&
> 
> this check will fail.

Yes, it's intended to fail.  I intend to make it explicit throughout
autogroup that root_task_group is not an autogroup, and will drop the
test for root_task_group then.

	-Mike

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Bharata B Rao]

On Thu, Jan 13, 2011 at 04:54:50AM +0100, Mike Galbraith wrote:
> (mailer switched me to work account again.  grrrr, switch yet again:)
> 
> On Wed, 2011-01-12 at 14:32 +0800, Yong Zhang wrote:
> 
> > Setting rt_runtime both in this patch and the previous one will make tune
> > other's rt_runtime/rt_period trouble, I guess.
> 
> Yeah, as in making it impossible to allocate even 1 usec :)  We need
> zero bandwidth though, so tell  __sched_setscheduler() that autogroups
> having zero allocated is perfectly fine.
> 
> sched, autogroup: fix CONFIG_RT_GROUP_SCHED sched_setscheduler() failure.
> 
> If CONFIG_RT_GROUP_SCHED is set, __sched_setscheduler() fails due to autogroup
> not allocating rt_runtime.  Free unused/unusable rt_se and rt_rq, redirect RT
> tasks to the root task group, and tell __sched_setscheduler() that it's ok.
> 
> Signed-off-by: Mike Galbraith <efault@gmx.de>
> Reported-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> 
> ---
>  kernel/sched.c           |    3 ++-
>  kernel/sched_autogroup.c |   27 +++++++++++++++++++++++++++
>  kernel/sched_autogroup.h |    4 ++++
>  3 files changed, 33 insertions(+), 1 deletion(-)
> 

Works fine here.

Thanks,
Bharata.

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Peter Zijlstra]

On Thu, 2011-01-13 at 04:54 +0100, Mike Galbraith wrote:
> sched, autogroup: fix CONFIG_RT_GROUP_SCHED sched_setscheduler() failure.
> 
> If CONFIG_RT_GROUP_SCHED is set, __sched_setscheduler() fails due to autogroup
> not allocating rt_runtime.  Free unused/unusable rt_se and rt_rq, redirect RT
> tasks to the root task group, and tell __sched_setscheduler() that it's ok.
> 
> Signed-off-by: Mike Galbraith <efault@gmx.de>
> Reported-by: Bharata B Rao <bharata@linux.vnet.ibm.com>

Thanks, applied!

[tip:sched/urgent] sched, autogroup: Fix CONFIG_RT_GROUP_SCHED sched_setscheduler() failure

[tip-bot for Mike Galbraith]

Commit-ID:  f44937718ce3b8360f72f6c68c9481712517a867
Gitweb:     http://git.kernel.org/tip/f44937718ce3b8360f72f6c68c9481712517a867
Author:     Mike Galbraith <efault@gmx.de>
AuthorDate: Thu, 13 Jan 2011 04:54:50 +0100
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Tue, 18 Jan 2011 15:09:42 +0100

sched, autogroup: Fix CONFIG_RT_GROUP_SCHED sched_setscheduler() failure

If CONFIG_RT_GROUP_SCHED is set, __sched_setscheduler() fails due to autogroup
not allocating rt_runtime.  Free unused/unusable rt_se and rt_rq, redirect RT
tasks to the root task group, and tell __sched_setscheduler() that it's ok.

Reported-and-tested-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
Signed-off-by: Mike Galbraith <efault@gmx.de>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <1294890890.8089.39.camel@marge.simson.net>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/sched.c           |    3 ++-
 kernel/sched_autogroup.c |   27 +++++++++++++++++++++++++++
 kernel/sched_autogroup.h |    4 ++++
 3 files changed, 33 insertions(+), 1 deletions(-)

diff --git a/kernel/sched.c b/kernel/sched.c
index a0eb094..6cbff6b 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -4871,7 +4871,8 @@ recheck:
 		 * assigned.
 		 */
 		if (rt_bandwidth_enabled() && rt_policy(policy) &&
-				task_group(p)->rt_bandwidth.rt_runtime == 0) {
+				task_group(p)->rt_bandwidth.rt_runtime == 0 &&
+				!task_group_is_autogroup(task_group(p))) {
 			__task_rq_unlock(rq);
 			raw_spin_unlock_irqrestore(&p->pi_lock, flags);
 			return -EPERM;
diff --git a/kernel/sched_autogroup.c b/kernel/sched_autogroup.c
index 938d52f..9fb6562 100644
--- a/kernel/sched_autogroup.c
+++ b/kernel/sched_autogroup.c
@@ -27,6 +27,11 @@ static inline void autogroup_destroy(struct kref *kref)
 {
 	struct autogroup *ag = container_of(kref, struct autogroup, kref);
 
+#ifdef CONFIG_RT_GROUP_SCHED
+	/* We've redirected RT tasks to the root task group... */
+	ag->tg->rt_se = NULL;
+	ag->tg->rt_rq = NULL;
+#endif
 	sched_destroy_group(ag->tg);
 }
 
@@ -55,6 +60,10 @@ static inline struct autogroup *autogroup_task_get(struct task_struct *p)
 	return ag;
 }
 
+#ifdef CONFIG_RT_GROUP_SCHED
+static void free_rt_sched_group(struct task_group *tg);
+#endif
+
 static inline struct autogroup *autogroup_create(void)
 {
 	struct autogroup *ag = kzalloc(sizeof(*ag), GFP_KERNEL);
@@ -72,6 +81,19 @@ static inline struct autogroup *autogroup_create(void)
 	init_rwsem(&ag->lock);
 	ag->id = atomic_inc_return(&autogroup_seq_nr);
 	ag->tg = tg;
+#ifdef CONFIG_RT_GROUP_SCHED
+	/*
+	 * Autogroup RT tasks are redirected to the root task group
+	 * so we don't have to move tasks around upon policy change,
+	 * or flail around trying to allocate bandwidth on the fly.
+	 * A bandwidth exception in __sched_setscheduler() allows
+	 * the policy change to proceed.  Thereafter, task_group()
+	 * returns &root_task_group, so zero bandwidth is required.
+	 */
+	free_rt_sched_group(tg);
+	tg->rt_se = root_task_group.rt_se;
+	tg->rt_rq = root_task_group.rt_rq;
+#endif
 	tg->autogroup = ag;
 
 	return ag;
@@ -106,6 +128,11 @@ task_wants_autogroup(struct task_struct *p, struct task_group *tg)
 	return true;
 }
 
+static inline bool task_group_is_autogroup(struct task_group *tg)
+{
+	return tg != &root_task_group && tg->autogroup;
+}
+
 static inline struct task_group *
 autogroup_task_group(struct task_struct *p, struct task_group *tg)
 {
diff --git a/kernel/sched_autogroup.h b/kernel/sched_autogroup.h
index 5358e24..7b859ff 100644
--- a/kernel/sched_autogroup.h
+++ b/kernel/sched_autogroup.h
@@ -15,6 +15,10 @@ autogroup_task_group(struct task_struct *p, struct task_group *tg);
 
 static inline void autogroup_init(struct task_struct *init_task) {  }
 static inline void autogroup_free(struct task_group *tg) { }
+static inline bool task_group_is_autogroup(struct task_group *tg)
+{
+	return 0;
+}
 
 static inline struct task_group *
 autogroup_task_group(struct task_struct *p, struct task_group *tg)

Re: [patch] Re: autogroup: sched_setscheduler() fails

[torbenh]

On Mon, Jan 17, 2011 at 02:16:00PM +0100, Peter Zijlstra wrote:
> On Thu, 2011-01-13 at 04:54 +0100, Mike Galbraith wrote:
> > sched, autogroup: fix CONFIG_RT_GROUP_SCHED sched_setscheduler() failure.
> > 
> > If CONFIG_RT_GROUP_SCHED is set, __sched_setscheduler() fails due to autogroup
> > not allocating rt_runtime.  Free unused/unusable rt_se and rt_rq, redirect RT
> > tasks to the root task group, and tell __sched_setscheduler() that it's ok.
> > 
> > Signed-off-by: Mike Galbraith <efault@gmx.de>
> > Reported-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> 
> Thanks, applied!

while this behaviour is certeinly necessary, i think this is a hack.
it fixes the problem for autogroups.
But its not fixed for things which want to control the cfs shares via
normal cgroups. 

why isnt rt_runtime_us residing in a separate (new) subsystem ?


-- 
torben Hohn

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Mike Galbraith]

On Tue, 2011-02-15 at 16:46 +0100, torbenh wrote:
> On Mon, Jan 17, 2011 at 02:16:00PM +0100, Peter Zijlstra wrote:
> > On Thu, 2011-01-13 at 04:54 +0100, Mike Galbraith wrote:
> > > sched, autogroup: fix CONFIG_RT_GROUP_SCHED sched_setscheduler() failure.
> > > 
> > > If CONFIG_RT_GROUP_SCHED is set, __sched_setscheduler() fails due to autogroup
> > > not allocating rt_runtime.  Free unused/unusable rt_se and rt_rq, redirect RT
> > > tasks to the root task group, and tell __sched_setscheduler() that it's ok.
> > > 
> > > Signed-off-by: Mike Galbraith <efault@gmx.de>
> > > Reported-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > 
> > Thanks, applied!
> 
> while this behaviour is certeinly necessary, i think this is a hack.
> it fixes the problem for autogroups.
> But its not fixed for things which want to control the cfs shares via
> normal cgroups.

You mean automated control ala systemd?  For a static set of groups, it
works fine.  I was wondering how systemd would deal with it.

> why isnt rt_runtime_us residing in a separate (new) subsystem ?

The allocation problem was shamelessly punted back to the user, where I
think it truly belongs.

	-Mike

Re: [patch] Re: autogroup: sched_setscheduler() fails

[torbenh]

On Tue, Feb 15, 2011 at 05:43:30PM +0100, Mike Galbraith wrote:
> On Tue, 2011-02-15 at 16:46 +0100, torbenh wrote:
> > On Mon, Jan 17, 2011 at 02:16:00PM +0100, Peter Zijlstra wrote:
> > > On Thu, 2011-01-13 at 04:54 +0100, Mike Galbraith wrote:
> > > > sched, autogroup: fix CONFIG_RT_GROUP_SCHED sched_setscheduler() failure.
> > > > 
> > > > If CONFIG_RT_GROUP_SCHED is set, __sched_setscheduler() fails due to autogroup
> > > > not allocating rt_runtime.  Free unused/unusable rt_se and rt_rq, redirect RT
> > > > tasks to the root task group, and tell __sched_setscheduler() that it's ok.
> > > > 
> > > > Signed-off-by: Mike Galbraith <efault@gmx.de>
> > > > Reported-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > > 
> > > Thanks, applied!
> > 
> > while this behaviour is certeinly necessary, i think this is a hack.
> > it fixes the problem for autogroups.
> > But its not fixed for things which want to control the cfs shares via
> > normal cgroups.
> 
> You mean automated control ala systemd?  For a static set of groups, it
> works fine.  I was wondering how systemd would deal with it.

but i can not get the same behaviour as if CONFIG_RT_GROUP_SCHED was
off. iE N cgroups with different cpu.share values, but each with
rt_runtime_us=950000

if the rt_runtime_us was in a different subsystem, its my understanding
that i could leave rt_runtime_us alone, and have all tasks in the root
group in the rt_runtime subsystem.

> 
> > why isnt rt_runtime_us residing in a separate (new) subsystem ?
> 
> The allocation problem was shamelessly punted back to the user, where I
> think it truly belongs.

sure it belongs to the user. but what if user wants to have different
cpu.shares, but full rt_runtime_us for all tasks ?

i can not have 2 sibling cgroups, whose rt_runtime_us adds up to over
the rt_period ...



> 
> 	-Mike
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
torben Hohn

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Mike Galbraith]

On Fri, 2011-02-18 at 12:09 +0100, torbenh wrote:
> On Tue, Feb 15, 2011 at 05:43:30PM +0100, Mike Galbraith wrote:
> > On Tue, 2011-02-15 at 16:46 +0100, torbenh wrote:
> > > On Mon, Jan 17, 2011 at 02:16:00PM +0100, Peter Zijlstra wrote:
> > > > On Thu, 2011-01-13 at 04:54 +0100, Mike Galbraith wrote:
> > > > > sched, autogroup: fix CONFIG_RT_GROUP_SCHED sched_setscheduler() failure.
> > > > > 
> > > > > If CONFIG_RT_GROUP_SCHED is set, __sched_setscheduler() fails due to autogroup
> > > > > not allocating rt_runtime.  Free unused/unusable rt_se and rt_rq, redirect RT
> > > > > tasks to the root task group, and tell __sched_setscheduler() that it's ok.
> > > > > 
> > > > > Signed-off-by: Mike Galbraith <efault@gmx.de>
> > > > > Reported-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > > > 
> > > > Thanks, applied!
> > > 
> > > while this behaviour is certeinly necessary, i think this is a hack.
> > > it fixes the problem for autogroups.
> > > But its not fixed for things which want to control the cfs shares via
> > > normal cgroups.
> > 
> > You mean automated control ala systemd?  For a static set of groups, it
> > works fine.  I was wondering how systemd would deal with it.
> 
> but i can not get the same behaviour as if CONFIG_RT_GROUP_SCHED was
> off. iE N cgroups with different cpu.share values, but each with
> rt_runtime_us=950000

? if CONFIG_RT_GROUP_SCHED was a noop, it wouldn't exist.

> if the rt_runtime_us was in a different subsystem, its my understanding
> that i could leave rt_runtime_us alone, and have all tasks in the root
> group in the rt_runtime subsystem.
 
Sounds like you just want to turn CONFIG_RT_GROUP_SCHED off.

> > The allocation problem was shamelessly punted back to the user, where I
> > think it truly belongs.
> 
> sure it belongs to the user. but what if user wants to have different
> cpu.shares, but full rt_runtime_us for all tasks ?

Then the user doesn't want CONFIG_RT_GROUP_SCHED set.

	-Mike

Re: [patch] Re: autogroup: sched_setscheduler() fails

[torbenh]

On Fri, Feb 18, 2011 at 01:50:12PM +0100, Mike Galbraith wrote:
> On Fri, 2011-02-18 at 12:09 +0100, torbenh wrote:
> > On Tue, Feb 15, 2011 at 05:43:30PM +0100, Mike Galbraith wrote:
> > > On Tue, 2011-02-15 at 16:46 +0100, torbenh wrote:
> > > > On Mon, Jan 17, 2011 at 02:16:00PM +0100, Peter Zijlstra wrote:
> > > > > On Thu, 2011-01-13 at 04:54 +0100, Mike Galbraith wrote:
> > > > > > sched, autogroup: fix CONFIG_RT_GROUP_SCHED sched_setscheduler() failure.
> > > > > > 
> > > > > > If CONFIG_RT_GROUP_SCHED is set, __sched_setscheduler() fails due to autogroup
> > > > > > not allocating rt_runtime.  Free unused/unusable rt_se and rt_rq, redirect RT
> > > > > > tasks to the root task group, and tell __sched_setscheduler() that it's ok.
> > > > > > 
> > > > > > Signed-off-by: Mike Galbraith <efault@gmx.de>
> > > > > > Reported-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > > > > 
> > > > > Thanks, applied!
> > > > 
> > > > while this behaviour is certeinly necessary, i think this is a hack.
> > > > it fixes the problem for autogroups.
> > > > But its not fixed for things which want to control the cfs shares via
> > > > normal cgroups.
> > > 
> > > You mean automated control ala systemd?  For a static set of groups, it
> > > works fine.  I was wondering how systemd would deal with it.
> > 
> > but i can not get the same behaviour as if CONFIG_RT_GROUP_SCHED was
> > off. iE N cgroups with different cpu.share values, but each with
> > rt_runtime_us=950000
> 
> ? if CONFIG_RT_GROUP_SCHED was a noop, it wouldn't exist.
> 
> > if the rt_runtime_us was in a different subsystem, its my understanding
> > that i could leave rt_runtime_us alone, and have all tasks in the root
> > group in the rt_runtime subsystem.
>  
> Sounds like you just want to turn CONFIG_RT_GROUP_SCHED off.
> 
> > > The allocation problem was shamelessly punted back to the user, where I
> > > think it truly belongs.
> > 
> > sure it belongs to the user. but what if user wants to have different
> > cpu.shares, but full rt_runtime_us for all tasks ?
> 
> Then the user doesn't want CONFIG_RT_GROUP_SCHED set.

many users dont configure their kernels themselves.
and this setting is not dynamic.

i guess its highly unlikely any normal user wants CONFIG_RT_GROUP_SCHED.
but maybe distros will turn it on. 

-- 
torben Hohn

Re: [patch] Re: autogroup: sched_setscheduler() fails

[torbenh]

On Fri, Feb 18, 2011 at 01:50:12PM +0100, Mike Galbraith wrote:
> On Fri, 2011-02-18 at 12:09 +0100, torbenh wrote:
> > On Tue, Feb 15, 2011 at 05:43:30PM +0100, Mike Galbraith wrote:
> > > On Tue, 2011-02-15 at 16:46 +0100, torbenh wrote:
> > > > On Mon, Jan 17, 2011 at 02:16:00PM +0100, Peter Zijlstra wrote:
> > > > > On Thu, 2011-01-13 at 04:54 +0100, Mike Galbraith wrote:
> > > > > > sched, autogroup: fix CONFIG_RT_GROUP_SCHED sched_setscheduler() failure.
> > > > > > 
> > > > > > If CONFIG_RT_GROUP_SCHED is set, __sched_setscheduler() fails due to autogroup
> > > > > > not allocating rt_runtime.  Free unused/unusable rt_se and rt_rq, redirect RT
> > > > > > tasks to the root task group, and tell __sched_setscheduler() that it's ok.
> > > > > > 
> > > > > > Signed-off-by: Mike Galbraith <efault@gmx.de>
> > > > > > Reported-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > > > > 
> > > > > Thanks, applied!
> > > > 
> > > > while this behaviour is certeinly necessary, i think this is a hack.
> > > > it fixes the problem for autogroups.
> > > > But its not fixed for things which want to control the cfs shares via
> > > > normal cgroups.
> > > 
> > > You mean automated control ala systemd?  For a static set of groups, it
> > > works fine.  I was wondering how systemd would deal with it.
> > 
> > but i can not get the same behaviour as if CONFIG_RT_GROUP_SCHED was
> > off. iE N cgroups with different cpu.share values, but each with
> > rt_runtime_us=950000
> 
> ? if CONFIG_RT_GROUP_SCHED was a noop, it wouldn't exist.
> 
> > if the rt_runtime_us was in a different subsystem, its my understanding
> > that i could leave rt_runtime_us alone, and have all tasks in the root
> > group in the rt_runtime subsystem.
>  
> Sounds like you just want to turn CONFIG_RT_GROUP_SCHED off.

but distros turn it on.
we could prevent debian from turning it on. 
now opensuse 11.4 has turned it on.


> 
> > > The allocation problem was shamelessly punted back to the user, where I
> > > think it truly belongs.
> > 
> > sure it belongs to the user. but what if user wants to have different
> > cpu.shares, but full rt_runtime_us for all tasks ?
> 
> Then the user doesn't want CONFIG_RT_GROUP_SCHED set.

but distros force it onto the user.
if systemd shows up, and puts things into different cgroups, 
we end up with a pretty fragmented rt_runtime_us.

> 
> 	-Mike
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
torben Hohn

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Mike Galbraith]

On Tue, 2011-02-22 at 13:24 +0100, torbenh wrote:
> On Fri, Feb 18, 2011 at 01:50:12PM +0100, Mike Galbraith wrote:
> > On Fri, 2011-02-18 at 12:09 +0100, torbenh wrote:
> > > On Tue, Feb 15, 2011 at 05:43:30PM +0100, Mike Galbraith wrote:
> > > > On Tue, 2011-02-15 at 16:46 +0100, torbenh wrote:
> > > > > On Mon, Jan 17, 2011 at 02:16:00PM +0100, Peter Zijlstra wrote:
> > > > > > On Thu, 2011-01-13 at 04:54 +0100, Mike Galbraith wrote:
> > > > > > > sched, autogroup: fix CONFIG_RT_GROUP_SCHED sched_setscheduler() failure.
> > > > > > > 
> > > > > > > If CONFIG_RT_GROUP_SCHED is set, __sched_setscheduler() fails due to autogroup
> > > > > > > not allocating rt_runtime.  Free unused/unusable rt_se and rt_rq, redirect RT
> > > > > > > tasks to the root task group, and tell __sched_setscheduler() that it's ok.
> > > > > > > 
> > > > > > > Signed-off-by: Mike Galbraith <efault@gmx.de>
> > > > > > > Reported-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > > > > > 
> > > > > > Thanks, applied!
> > > > > 
> > > > > while this behaviour is certeinly necessary, i think this is a hack.
> > > > > it fixes the problem for autogroups.
> > > > > But its not fixed for things which want to control the cfs shares via
> > > > > normal cgroups.
> > > > 
> > > > You mean automated control ala systemd?  For a static set of groups, it
> > > > works fine.  I was wondering how systemd would deal with it.
> > > 
> > > but i can not get the same behaviour as if CONFIG_RT_GROUP_SCHED was
> > > off. iE N cgroups with different cpu.share values, but each with
> > > rt_runtime_us=950000
> > 
> > ? if CONFIG_RT_GROUP_SCHED was a noop, it wouldn't exist.
> > 
> > > if the rt_runtime_us was in a different subsystem, its my understanding
> > > that i could leave rt_runtime_us alone, and have all tasks in the root
> > > group in the rt_runtime subsystem.
> >  
> > Sounds like you just want to turn CONFIG_RT_GROUP_SCHED off.
> 
> but distros turn it on.
> we could prevent debian from turning it on. 
> now opensuse 11.4 has turned it on.

If you or anyone else turns on RT_GROUP_SCHED, you will count your
beans, and pay up front, or you will not play.  That's a very sensible
policy for realtime.

> > > > The allocation problem was shamelessly punted back to the user, where I
> > > > think it truly belongs.
> > > 
> > > sure it belongs to the user. but what if user wants to have different
> > > cpu.shares, but full rt_runtime_us for all tasks ?
> > 
> > Then the user doesn't want CONFIG_RT_GROUP_SCHED set.
> 
> but distros force it onto the user.
> if systemd shows up, and puts things into different cgroups, 
> we end up with a pretty fragmented rt_runtime_us.

If systemd deals with it at all, seems to me it can only make a mess of
it.  But who knows, maybe they made a clever allocator.  If they didn't,
they'll need an escape hatch methinks.

	-Mike

Re: [patch] Re: autogroup: sched_setscheduler() fails

[torbenh]

On Tue, Feb 22, 2011 at 03:47:53PM +0100, Mike Galbraith wrote:
> On Tue, 2011-02-22 at 13:24 +0100, torbenh wrote:
> > On Fri, Feb 18, 2011 at 01:50:12PM +0100, Mike Galbraith wrote:
> > > On Fri, 2011-02-18 at 12:09 +0100, torbenh wrote:
> > > > On Tue, Feb 15, 2011 at 05:43:30PM +0100, Mike Galbraith wrote:
> > > > > On Tue, 2011-02-15 at 16:46 +0100, torbenh wrote:
> > > > > > On Mon, Jan 17, 2011 at 02:16:00PM +0100, Peter Zijlstra wrote:
> > > > > > > On Thu, 2011-01-13 at 04:54 +0100, Mike Galbraith wrote:
> > > > > > > > sched, autogroup: fix CONFIG_RT_GROUP_SCHED sched_setscheduler() failure.
> > > > > > > > 
> > > > > > > > If CONFIG_RT_GROUP_SCHED is set, __sched_setscheduler() fails due to autogroup
> > > > > > > > not allocating rt_runtime.  Free unused/unusable rt_se and rt_rq, redirect RT
> > > > > > > > tasks to the root task group, and tell __sched_setscheduler() that it's ok.
> > > > > > > > 
> > > > > > > > Signed-off-by: Mike Galbraith <efault@gmx.de>
> > > > > > > > Reported-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > > > > > > 
> > > > > > > Thanks, applied!
> > > > > > 
> > > > > > while this behaviour is certeinly necessary, i think this is a hack.
> > > > > > it fixes the problem for autogroups.
> > > > > > But its not fixed for things which want to control the cfs shares via
> > > > > > normal cgroups.
> > > > > 
> > > > > You mean automated control ala systemd?  For a static set of groups, it
> > > > > works fine.  I was wondering how systemd would deal with it.
> > > > 
> > > > but i can not get the same behaviour as if CONFIG_RT_GROUP_SCHED was
> > > > off. iE N cgroups with different cpu.share values, but each with
> > > > rt_runtime_us=950000
> > > 
> > > ? if CONFIG_RT_GROUP_SCHED was a noop, it wouldn't exist.
> > > 
> > > > if the rt_runtime_us was in a different subsystem, its my understanding
> > > > that i could leave rt_runtime_us alone, and have all tasks in the root
> > > > group in the rt_runtime subsystem.
> > >  
> > > Sounds like you just want to turn CONFIG_RT_GROUP_SCHED off.
> > 
> > but distros turn it on.
> > we could prevent debian from turning it on. 
> > now opensuse 11.4 has turned it on.
> 
> If you or anyone else turns on RT_GROUP_SCHED, you will count your
> beans, and pay up front, or you will not play.  That's a very sensible
> policy for realtime.

this probably means that generic computer distros should not turn this
option on ?

> 
> > > > > The allocation problem was shamelessly punted back to the user, where I
> > > > > think it truly belongs.
> > > > 
> > > > sure it belongs to the user. but what if user wants to have different
> > > > cpu.shares, but full rt_runtime_us for all tasks ?
> > > 
> > > Then the user doesn't want CONFIG_RT_GROUP_SCHED set.
> > 
> > but distros force it onto the user.
> > if systemd shows up, and puts things into different cgroups, 
> > we end up with a pretty fragmented rt_runtime_us.
> 
> If systemd deals with it at all, seems to me it can only make a mess of
> it.  But who knows, maybe they made a clever allocator.  If they didn't,
> they'll need an escape hatch methinks.

the problem is that audio applications can not really pre allocate their
cpu needs. user can add processing plugins until he pushes his machine
to the limit. (or the cgroup where his process is running in)

we dont really have a mechanism for plugins to publish their needed
cycles. 


-- 
torben Hohn

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Mike Galbraith]

On Mon, 2011-02-28 at 18:53 +0100, torbenh wrote:
> 
> On Tue, Feb 22, 2011 at 03:47:53PM +0100, Mike Galbraith wrote:
> > On Tue, 2011-02-22 at 13:24 +0100, torbenh wrote:
> > > On Fri, Feb 18, 2011 at 01:50:12PM +0100, Mike Galbraith wrote:
>  
> > > > Sounds like you just want to turn CONFIG_RT_GROUP_SCHED off.
> > > 
> > > but distros turn it on.
> > > we could prevent debian from turning it on. 
> > > now opensuse 11.4 has turned it on.
> > 
> > If you or anyone else turns on RT_GROUP_SCHED, you will count your
> > beans, and pay up front, or you will not play.  That's a very sensible
> > policy for realtime.
> 
> this probably means that generic computer distros should not turn this
> option on ?

Yeah, agreed, not for a great default config, but only because
newfangled automation thingies can't (possibly?) deal with it sanely.
 
> > If systemd deals with it at all, seems to me it can only make a mess of
> > it.  But who knows, maybe they made a clever allocator.  If they didn't,
> > they'll need an escape hatch methinks.
> 
> the problem is that audio applications can not really pre allocate their
> cpu needs. user can add processing plugins until he pushes his machine
> to the limit. (or the cgroup where his process is running in)
> 
> we dont really have a mechanism for plugins to publish their needed
> cycles.

I can't see how it could matter what any individual group of arbitrary
groups N (who can appear/disappear in the blink of an eye) advertises as
it's wish of the instant.  "Hard" + "Arbitrary" doesn't compute for me.

	-Mike

Re: [patch] Re: autogroup: sched_setscheduler() fails

[torbenh]

sorry... i cced the old ML addresses :S
fixing the CC now.


On Mon, Feb 28, 2011 at 07:29:54PM +0100, Mike Galbraith wrote:
> On Mon, 2011-02-28 at 18:53 +0100, torbenh wrote:
> > 
> > On Tue, Feb 22, 2011 at 03:47:53PM +0100, Mike Galbraith wrote:
> > > On Tue, 2011-02-22 at 13:24 +0100, torbenh wrote:
> > > > On Fri, Feb 18, 2011 at 01:50:12PM +0100, Mike Galbraith wrote:
> >  
> > > > > Sounds like you just want to turn CONFIG_RT_GROUP_SCHED off.
> > > > 
> > > > but distros turn it on.
> > > > we could prevent debian from turning it on. 
> > > > now opensuse 11.4 has turned it on.
> > > 
> > > If you or anyone else turns on RT_GROUP_SCHED, you will count your
> > > beans, and pay up front, or you will not play.  That's a very sensible
> > > policy for realtime.
> > 
> > this probably means that generic computer distros should not turn this
> > option on ?
> 
> Yeah, agreed, not for a great default config, but only because
> newfangled automation thingies can't (possibly?) deal with it sanely.

but this is excactly the reason, why i would advocate rt_runtime to be
in a separate cgroups system.
any admin who wants to limit RT runtime could still do it.
people who dont care, and just want their cfs slices configured, can
still do it.

>  
> > > If systemd deals with it at all, seems to me it can only make a mess of
> > > it.  But who knows, maybe they made a clever allocator.  If they didn't,
> > > they'll need an escape hatch methinks.
> > 
> > the problem is that audio applications can not really pre allocate their
> > cpu needs. user can add processing plugins until he pushes his machine
> > to the limit. (or the cgroup where his process is running in)
> > 
> > we dont really have a mechanism for plugins to publish their needed
> > cycles.
> 
> I can't see how it could matter what any individual group of arbitrary
> groups N (who can appear/disappear in the blink of an eye) advertises as
> it's wish of the instant.  "Hard" + "Arbitrary" doesn't compute for me.

i dont really understnad this statement.

> 
> 	-Mike
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
torben Hohn

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Mike Galbraith]

On Mon, 2011-02-28 at 20:10 +0100, torbenh wrote: 

> i dont really understnad this statement.

Doesn't matter, we're thinking about different subjects.  You're
thinking of an unborn mechanism, I'm thinking of the living one.

	-Mike

Re: [patch] Re: autogroup: sched_setscheduler() fails

[Mike Galbraith]

P.S.  Kindly do not CC closed lists.  Closed list readers only see your
part of any conversation, and anyone replying to you is rewarded with
bounce spam for their trouble.

Re: [patch] Re: autogroup: sched_setscheduler() fails

[torbenh]

On Tue, Mar 01, 2011 at 05:21:30AM +0100, Mike Galbraith wrote:
> P.S.  Kindly do not CC closed lists.  Closed list readers only see your
> part of any conversation, and anyone replying to you is rewarded with
> bounce spam for their trouble.

yeah. sorry. i thought the lists were open. 
there doesnt seem to be interest in it anyways.

i guess i rest my case, and wait for things to fall apart.


-- 
torben Hohn