Re: [PATCH 11/20] blkio: Some CFQ debugging Aid

[Vivek Goyal <vgoyal@redhat.com>]

On Wed, Nov 04, 2009 at 01:52:51PM -0500, Jeff Moyer wrote:
> Vivek Goyal <vgoyal@redhat.com> writes:
> 
> > o Some CFQ debugging Aid.
> 
> Some CFQ debugging aids.  Sorry, I couldn't help myself.
> 

It thought this was all about IO controller and not english grammar. :-)

Will fix it.

> > +config DEBUG_BLK_CGROUP
> > +	bool
> > +	depends on BLK_CGROUP
> > +	default n
> > +	---help---
> > +	Enable some debugging help. Currently it stores the cgroup path
> > +	in the blk group which can be used by cfq for tracing various
> > +	group related activity.
> > +
> >  endif # BLOCK
> >  
> 
> > +config DEBUG_CFQ_IOSCHED
> > +	bool "Debug CFQ Scheduling"
> > +	depends on CFQ_GROUP_IOSCHED
> > +	select DEBUG_BLK_CGROUP
> 
> This seems wrong.  DEBUG_CFQ_IOSCHED sounds like it enables debugging of
> CFQ.  In your implementation, though, it only enables this in tandem
> with the blkio cgroup infrastructure.  Please decouple these things.
> 

What's wrong with this? We are emitting more debugging information for
CFQ like cgroup name and paths in blktrace output. That's a different
thing that internally that information is also dependent debugging being
enabled in blk io controller.

Important thing here is that blk io controller and its debugging option is
automatically selected depending on what user wants from end policies.

So I really can't see that what's wrong if CFQ debugging option internally
selects some other config option also.
 

> > +#ifdef CONFIG_DEBUG_BLK_CGROUP
> > +	/* Store cgroup path */
> > +	char path[128];
> > +#endif
> 
> Where does 128 come from?  What's wrong with PATH_MAX?

CFS influence (kernel/sched_debug.c)

Actually PATH_MAX will be 4096 bytes. Too long to display in blktrace
output? I have not check what's the blktrace limit but we don't want too
long a lines in output. 

Thanks
Vivek