[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150902141606.GJ12722@kernel.org>
Date: Wed, 2 Sep 2015 11:16:06 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Jiri Olsa <jolsa@...hat.com>
Cc: Jiri Olsa <jolsa@...nel.org>, lkml <linux-kernel@...r.kernel.org>,
David Ahern <dsahern@...il.com>,
Ingo Molnar <mingo@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>,
Matt Fleming <matt@...eblueprint.co.uk>,
Raphaƫl Beamonte <raphael.beamonte@...il.com>,
Steven Rostedt <rostedt@...dmis.org>
Subject: Re: [PATCH 06/15] tools lib api: Make tracing_path_strerror_open
message generic
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@...nel.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists