Re: [PATCH 5/5] perf: Make perf aware of tracefs

[Steven Rostedt]

On Sun, 25 Jan 2015 18:34:33 +0100
Jiri Olsa <jolsa@xxxxxxxxxx> wrote:

> On Sat, Jan 24, 2015 at 01:13:35PM -0500, Steven Rostedt wrote:
> 
> SNIP
> 
> > @@ -351,11 +385,15 @@ const char *find_tracing_dir(void)
> >  	if (tracing_found)
> >  		return tracing;
> >  
> > -	debugfs = find_debugfs();
> > -	if (!debugfs)
> > -		return NULL;
> > +	debugfs = find_tracefs();
> > +	if (!debugfs) {
> > +		tracing_dir = "/tracing";
> > +		debugfs = find_debugfs();
> > +		if (!debugfs)
> > +			return NULL;
> 
> so this pattern 'try tracefs, if that does not work try debugfs'
> is all over the patch.. how about we add new new 'virtual' fs to
> encapsulate that, like:
> 
>   with followign interface:
>     bool tpfs_configured(void);
>     const char *tpfs_find_mountpoint(void);
>     int tpfs_valid_mountpoint(const char *debugfs);
>     char *tpfs_mount(const char *mountpoint);
>     extern char tpfs_mountpoint[];
> 
> It does not neceserily needs to reside in tools/lib/api/fs/tpfs.[ch],
> but I believe we need some form of encapsulation for this.

I did it this way because perf had no encapsulation for finding debugfs
paths. I made the change in trace-cmd in one location. Perf does it a
little different in each place. I have no idea why there was more than
one location slapping on "/tracing".

I'd like to get tracefs into perf before doing any redesign of perf's
infrastructure. That could come later as a cleanup.

This is not a fast path, it wont hurt performance. I agree it should be
encapsulated, but that's going outside of the scope of this patch
series, which is only to add awareness to tracefs, not to clean up
perf's access to the debugfs/tracing directory.

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