Re: [PATCH 06/15] tools lib api: Make tracing_path_strerror_open message generic

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Wed, Sep 02, 2015 at 03:44:50PM +0200, Jiri Olsa escreveu:
> On Wed, Sep 02, 2015 at 10:18:44AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Sep 02, 2015 at 09:56:36AM +0200, Jiri Olsa escreveu:
> > > Making tracing_path__strerror_open_tp message generic by mentioning
> > 
> > What means "making message generic"? What is the current behaviour you
> > think is problematic. what is the new behaviour ad why do you think it
> > is better?
> > 
> > The test for ENOENT became confusing, i.e. since this was a test for
> > "tracefs", if debugfs_configured() returned true, i.e. debugfs _was_
> > found in the system, then, the message makes sense, even if probably
> > could be made better, i.e. isn't true that if CONFIG_DEBUGFS is
> > configured and furthermore, debugfs_configure() returns true, then it
> > should be something like CONFIG_TRACEFS that needs enabling?
> > 
> > I applied all patches before this one, BTW.
> > 
> > - Arnaldo
> > 
> > > both debugfs/tracefs words in error message plus the tracing_path
> > > instead of debugfs_mountpoint.
> > > 
> > > Link: http://lkml.kernel.org/n/tip-5y7nboe2xe619hp649ry58z6@git.kernel.org
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > ---
> > >  tools/lib/api/fs/tracing_path.c | 20 ++++++++++----------
> > >  1 file changed, 10 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/tools/lib/api/fs/tracing_path.c b/tools/lib/api/fs/tracing_path.c
> > > index 3b3e4f5fc50b..b0ee3b3acef0 100644
> > > --- a/tools/lib/api/fs/tracing_path.c
> > > +++ b/tools/lib/api/fs/tracing_path.c
> > > @@ -90,33 +90,33 @@ static int strerror_open(int err, char *buf, size_t size, const char *filename)
> > >  
> > >  	switch (err) {
> > >  	case ENOENT:
> > > -		if (debugfs_configured()) {
> > > +		if (debugfs_configured() || tracefs_configured()) {
> > >  			snprintf(buf, size,
> > >  				 "Error:\tFile %s/%s not found.\n"
> > >  				 "Hint:\tPerhaps this kernel misses some CONFIG_ setting to enable this feature?.\n",
> > > -				 debugfs_mountpoint, filename);
> > > +				 tracing_events_path, filename);
> > 
> > Humm
> 
> we will get here if we can't find the tracepoint, but one of
> debugfs or tracefs is configured, which means you probably
> want some tracepoint which wasn't compiled in your kernel
> 
> before it did not take into account we could have tracefs configured
> thats what other changes in here are about, to consider tracefs mount

Ok, that helps, will add the above as an comment.

Somehow I was seeing this as not finding the mountpoints :-\

- Arnaldo