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

[Mike Galbraith <efault@gmx.de>]

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