Re: [PATCH v4] sched: automated per session task groups

Mike Galbraith
2010-11-30T16:41:53Z

On Tue, 2010-11-30 at 15:13 +0100, Ingo Molnar wrote:
> * Mike Galbraith <efault@gmx.de> wrote:
>
> > On Mon, 2010-11-29 at 20:23 -0800, Paul Turner wrote:
> >
> > > I've left some machines running tip + fix above + autogroup to see if
> > > anything else emerges. Hasn't crashed yet, I'll leave it going
> > > overnight.
> >
> > Thanks. Below is the hopefully final version against tip. The last I
> > sent contained a couple remnants.
>
> Note, I removed this chunk:
>
> > kernel/sched_debug.c | 29 ++--
>
> > Index: linux-2.6/kernel/sched_debug.c
> > ===================================================================
> > --- linux-2.6.orig/kernel/sched_debug.c
> > +++ linux-2.6/kernel/sched_debug.c
> > @@ -87,6 +87,20 @@ static void print_cfs_group_stats(struct
> > }
> > #endif
> >
> > +#if defined(CONFIG_CGROUP_SCHED) && \
> > + (defined(CONFIG_FAIR_GROUP_SCHED) || defined(CONFIG_RT_GROUP_SCHED))
> > +static void task_group_path(struct task_group *tg, char *buf, int buflen)
> > +{
> > + /* may be NULL if the underlying cgroup isn't fully-created yet */
> > + if (!tg->css.cgroup) {
> > + if (!autogroup_path(tg, buf, buflen))
> > + buf[0] = '\0';
> > + return;
> > + }
> > + cgroup_path(tg->css.cgroup, buf, buflen);
> > +}
> > +#endif
> > +
> > static void
> > print_task(struct seq_file *m, struct rq *rq, struct task_struct *p)
> > {
> > @@ -115,7 +129,7 @@ print_task(struct seq_file *m, struct rq
> > char path[64];
> >
> > rcu_read_lock();
> > - cgroup_path(task_group(p)->css.cgroup, path, sizeof(path));
> > + task_group_path(task_group(p), path, sizeof(path));
> > rcu_read_unlock();
> > SEQ_printf(m, " %s", path);
> > }
> > @@ -147,19 +161,6 @@ static void print_rq(struct seq_file *m,
> > read_unlock_irqrestore(&tasklist_lock, flags);
> > }
> >
> > -#if defined(CONFIG_CGROUP_SCHED) && \
> > - (defined(CONFIG_FAIR_GROUP_SCHED) || defined(CONFIG_RT_GROUP_SCHED))
> > -static void task_group_path(struct task_group *tg, char *buf, int buflen)
> > -{
> > - /* may be NULL if the underlying cgroup isn't fully-created yet */
> > - if (!tg->css.cgroup) {
> > - buf[0] = '\0';
> > - return;
> > - }
> > - cgroup_path(tg->css.cgroup, buf, buflen);
> > -}
> > -#endif
> > -
> > void print_cfs_rq(struct seq_file *m, int cpu, struct cfs_rq *cfs_rq)
> > {
> > s64 MIN_vruntime = -1, min_vruntime, max_vruntime = -1,
>
> Because it didn't build (for obvious reasons - the CONFIG conditions dont match up),
> but more importantly it's quite ugly. Some existing 'path' variables are 64 byte,
> some are 128 byte - so there's pre-existing damage - i removed it all.

Won't removing that hunk bring back oops if you cat /proc/sched_debug?

cfs_rq[0]:/autogroup-88
.exec_clock : 0.228697
.MIN_vruntime : 0.000001
.min_vruntime : 0.819879
.max_vruntime : 0.000001
.spread : 0.000000
.spread0 : -22925903.100800

> Could we do this debugging code in a bit saner way please? (as a delta patch on top
> of the -tip that i'll push out in the next hour or so.)

Guess I'll try.

-Mike