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

From: Arnaldo Carvalho de Melo
Date: Wed Sep 02 2015 - 10:16:20 EST


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@xxxxxxxxxxxxxx
> > > Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> > > ---
> > > 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
--
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/