Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1213895 > unrolled thread

[RFC 00/11] perf tools: Enhance parsing events tracepoint error output

Started byJiri Olsa <jolsa@kernel.org>
First post2015-08-26 15:50 +0200
Last post2015-08-31 10:40 +0200
Articles 7 — 5 participants

Back to article view | Back to linux.kernel

Contents

  [RFC 00/11] perf tools: Enhance parsing events tracepoint error output Jiri Olsa <jolsa@kernel.org> - 2015-08-26 15:50 +0200
    [PATCH 03/11] perf tools: Do not change lib/api/fs/debugfs directly Jiri Olsa <jolsa@kernel.org> - 2015-08-26 15:50 +0200
      Re: [PATCH 03/11] perf tools: Do not change lib/api/fs/debugfs  directly Jiri Olsa <jolsa@redhat.com> - 2015-08-26 16:20 +0200
      Re: [PATCH 03/11] perf tools: Do not change lib/api/fs/debugfs  directly Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-08-26 16:50 +0200
        Re: [PATCH 03/11] perf tools: Do not change lib/api/fs/debugfs  directly Matt Fleming <matt@codeblueprint.co.uk> - 2015-08-28 14:30 +0200
      Re: [PATCH 03/11] perf tools: Do not change lib/api/fs/debugfs  directly Matt Fleming <matt@codeblueprint.co.uk> - 2015-08-28 14:30 +0200
      [tip:perf/core] perf tools: Do not change lib/api/fs/  debugfs directly tip-bot for Jiri Olsa <tipbot@zytor.com> - 2015-08-31 10:40 +0200

#1213895 — [RFC 00/11] perf tools: Enhance parsing events tracepoint error output

FromJiri Olsa <jolsa@kernel.org>
Date2015-08-26 15:50 +0200
Subject[RFC 00/11] perf tools: Enhance parsing events tracepoint error output
Message-ID<q1Jfs-7l6-5@gated-at.bofh.it>
hi,
enhancing parsing events tracepoint error output. Adding
more verbose output when the tracepoint is not found or
the tracing event path cannot be access.

  $ sudo perf record -e sched:sched_krava ls
  event syntax error: 'sched:sched_krava'
                       \___ unknown tracepoint

  Error:  File /sys/kernel/debug/tracing//tracing/events/sched/sched_krava not found.
  Hint:   Perhaps this kernel misses some CONFIG_ setting to enable this feature?.

  Run 'perf list' for a list of valid events
  ...

  $ perf record -e sched:sched_krava ls
  event syntax error: 'sched:sched_krava'
                       \___ can't access trace events

  Error:  No permissions to read /sys/kernel/debug/tracing//tracing/events/sched/sched_krava
  Hint:   Try 'sudo mount -o remount,mode=755 /sys/kernel/debug'

  Run 'perf list' for a list of valid events
  ...

I changed api/fs tracefs/debugfs related code and I'm not completely
sure what were the long term intentions with this code, so please
comment ;-)

jirka


---
Jiri Olsa (11):
      tools: Add err.h with ERR_PTR PTR_ERR interface
      perf tools: Add tracing_path and remove unneeded functions
      perf tools: Do not change lib/api/fs/debugfs directly
      perf tools: Move debugfs__strerror_open into util.c object
      perf tools: Move tracing_path stuff under same namespace
      perf tools: Move tracing_path interface into trace-event-path.c
      perf tools: Make tracing_path_strerror_open message generic
      perf tools: Do not export debugfs_mountpoint and tracefs_mountpoint
      perf tools: Propagate error info for the tracepoint parsing
      perf tools: Propagate error info from tp_format
      perf tools: Enhance parsing events tracepoint error output

 tools/include/linux/err.h                  |  28 +++++++++++++++++++
 tools/lib/api/fs/debugfs.c                 |  53 +-----------------------------------
 tools/lib/api/fs/debugfs.h                 |   5 ----
 tools/lib/api/fs/tracefs.c                 |   2 +-
 tools/lib/api/fs/tracefs.h                 |   2 --
 tools/perf/builtin-trace.c                 |   4 +--
 tools/perf/perf.c                          |  11 ++++----
 tools/perf/tests/openat-syscall-all-cpus.c |   2 ++
 tools/perf/tests/openat-syscall.c          |   2 ++
 tools/perf/util/Build                      |   1 +
 tools/perf/util/evsel.c                    |   8 ++++--
 tools/perf/util/parse-events.c             |  58 ++++++++++++++++++++++++++++++----------
 tools/perf/util/parse-events.h             |   3 ++-
 tools/perf/util/parse-events.y             |  16 ++++++-----
 tools/perf/util/trace-event-path.c         | 129 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/trace-event.c              |   7 +++--
 tools/perf/util/trace-event.h              |  12 +++++++++
 tools/perf/util/util.c                     | 119 ---------------------------------------------------------------------------------
 tools/perf/util/util.h                     |   8 ------
 19 files changed, 250 insertions(+), 220 deletions(-)
 create mode 100644 tools/include/linux/err.h
 create mode 100644 tools/perf/util/trace-event-path.c
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1213896 — [PATCH 03/11] perf tools: Do not change lib/api/fs/debugfs directly

FromJiri Olsa <jolsa@kernel.org>
Date2015-08-26 15:50 +0200
Subject[PATCH 03/11] perf tools: Do not change lib/api/fs/debugfs directly
Message-ID<q1Jft-7l6-43@gated-at.bofh.it>
In reply to#1213895
The tracing_events_path is the variable we want to change
via --debugfs-dir option, not the debugfs_mountpoint.

Link: http://lkml.kernel.org/n/tip-pj9htv4foum6f6uk7wzi9etx@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/perf.c      | 2 +-
 tools/perf/util/util.c | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index b857fcbd00cf..07dbff5c0e60 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -231,7 +231,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 			(*argc)--;
 		} else if (!prefixcmp(cmd, CMD_DEBUGFS_DIR)) {
 			perf_debugfs_set_path(cmd + strlen(CMD_DEBUGFS_DIR));
-			fprintf(stderr, "dir: %s\n", debugfs_mountpoint);
+			fprintf(stderr, "dir: %s\n", tracing_path);
 			if (envchanged)
 				*envchanged = 1;
 		} else if (!strcmp(cmd, "--list-cmds")) {
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index d33c34196a5a..7acafb3c5592 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -439,7 +439,6 @@ const char *perf_debugfs_mount(const char *mountpoint)
 
 void perf_debugfs_set_path(const char *mntpt)
 {
-	snprintf(debugfs_mountpoint, strlen(debugfs_mountpoint), "%s", mntpt);
 	set_tracing_events_path("tracing/", mntpt);
 }
 
-- 
2.4.3

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

[toc] | [prev] | [next] | [standalone]

#1213910 — Re: [PATCH 03/11] perf tools: Do not change lib/api/fs/debugfs directly

FromJiri Olsa <jolsa@redhat.com>
Date2015-08-26 16:20 +0200
SubjectRe: [PATCH 03/11] perf tools: Do not change lib/api/fs/debugfs directly
Message-ID<q1JIu-88h-11@gated-at.bofh.it>
In reply to#1213896
On Wed, Aug 26, 2015 at 10:06:45AM -0400, Raphaël Beamonte wrote:
> On Aug 26, 2015 9:47 AM, "Jiri Olsa" <jolsa@kernel.org> wrote:
> >
> > The tracing_events_path is the variable we want to change
> > via --debugfs-dir option, not the debugfs_mountpoint.
> 
> <SNIP>
> 
> >                         perf_debugfs_set_path(cmd +
> strlen(CMD_DEBUGFS_DIR));
> > -                       fprintf(stderr, "dir: %s\n", debugfs_mountpoint);
> > +                       fprintf(stderr, "dir: %s\n", tracing_path);
> >                         if (envchanged)
> >                                 *envchanged = 1;
> >                 } else if (!strcmp(cmd, "--list-cmds")) {
> > diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
> > index d33c34196a5a..7acafb3c5592 100644
> > --- a/tools/perf/util/util.c
> > +++ b/tools/perf/util/util.c
> > @@ -439,7 +439,6 @@ const char *perf_debugfs_mount(const char *mountpoint)
> >
> >  void perf_debugfs_set_path(const char *mntpt)
> >  {
> > -       snprintf(debugfs_mountpoint, strlen(debugfs_mountpoint), "%s",
> mntpt);
> >         set_tracing_events_path("tracing/", mntpt) ;
> >  }
> 
> Why keep a function name with debugfs in it here if we're not touching
> debugfs anymore? Shouldn't we call directly set_tracing_events_path if

please check patch 05/11

> that's what we want to do? Also, the option name --debugfs-dir is not
> entirely relevant anymore?

probably, but it's out there and someone could be using it

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

[toc] | [prev] | [next] | [standalone]

#1213932 — Re: [PATCH 03/11] perf tools: Do not change lib/api/fs/debugfs directly

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2015-08-26 16:50 +0200
SubjectRe: [PATCH 03/11] perf tools: Do not change lib/api/fs/debugfs directly
Message-ID<q1Kbv-fe-5@gated-at.bofh.it>
In reply to#1213896
Em Wed, Aug 26, 2015 at 10:06:45AM -0400, Raphaël Beamonte escreveu:
> On Aug 26, 2015 9:47 AM, "Jiri Olsa" <jolsa@kernel.org> wrote:
> >
> > The tracing_events_path is the variable we want to change
> > via --debugfs-dir option, not the debugfs_mountpoint.
> 
> <SNIP>
> 
> >                         perf_debugfs_set_path(cmd +
> strlen(CMD_DEBUGFS_DIR));
> > -                       fprintf(stderr, "dir: %s\n", debugfs_mountpoint);
> > +                       fprintf(stderr, "dir: %s\n", tracing_path);
> >                         if (envchanged)
> >                                 *envchanged = 1;
> >                 } else if (!strcmp(cmd, "--list-cmds")) {
> > diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
> > index d33c34196a5a..7acafb3c5592 100644
> > --- a/tools/perf/util/util.c
> > +++ b/tools/perf/util/util.c
> > @@ -439,7 +439,6 @@ const char *perf_debugfs_mount(const char *mountpoint)
> >
> >  void perf_debugfs_set_path(const char *mntpt)
> >  {
> > -       snprintf(debugfs_mountpoint, strlen(debugfs_mountpoint), "%s",
> mntpt);
> >         set_tracing_events_path("tracing/", mntpt) ;
> >  }
> 
> Why keep a function name with debugfs in it here if we're not touching
> debugfs anymore?

Right

> Shouldn't we call directly set_tracing_events_path if that's what we
> want to do? Also, the option name --debugfs-dir is not entirely
> relevant anymore?

Yeah, probably that was a really bad name to start with, i.e. it
should've been named perhaps --event-definitions-dir or something to
that degree.

"debugfs", "tracefs" and the next thing to come in this area are just
implementation details :-)

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

[toc] | [prev] | [next] | [standalone]

#1215306 — Re: [PATCH 03/11] perf tools: Do not change lib/api/fs/debugfs directly

FromMatt Fleming <matt@codeblueprint.co.uk>
Date2015-08-28 14:30 +0200
SubjectRe: [PATCH 03/11] perf tools: Do not change lib/api/fs/debugfs directly
Message-ID<q2qXa-3ay-37@gated-at.bofh.it>
In reply to#1213932
On Wed, 26 Aug, at 11:27:58AM, Arnaldo Carvalho de Melo wrote:
> 
> Yeah, probably that was a really bad name to start with, i.e. it
> should've been named perhaps --event-definitions-dir or something to
> that degree.
> 
> "debugfs", "tracefs" and the next thing to come in this area are just
> implementation details :-)

Perhaps there should be an alias for --debugfs-dir that more
accurately reflects the "events definition dir" use?

-- 
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1215304 — Re: [PATCH 03/11] perf tools: Do not change lib/api/fs/debugfs directly

FromMatt Fleming <matt@codeblueprint.co.uk>
Date2015-08-28 14:30 +0200
SubjectRe: [PATCH 03/11] perf tools: Do not change lib/api/fs/debugfs directly
Message-ID<q2qX9-3ay-31@gated-at.bofh.it>
In reply to#1213896
On Wed, 26 Aug, at 03:46:45PM, Jiri Olsa wrote:
> The tracing_events_path is the variable we want to change
> via --debugfs-dir option, not the debugfs_mountpoint.
> 
> Link: http://lkml.kernel.org/n/tip-pj9htv4foum6f6uk7wzi9etx@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/perf.c      | 2 +-
>  tools/perf/util/util.c | 1 -
>  2 files changed, 1 insertion(+), 2 deletions(-)

Reviewed-by: Matt Fleming <matt.fleming@intel.com>

-- 
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1216107 — [tip:perf/core] perf tools: Do not change lib/api/fs/ debugfs directly

Fromtip-bot for Jiri Olsa <tipbot@zytor.com>
Date2015-08-31 10:40 +0200
Subject[tip:perf/core] perf tools: Do not change lib/api/fs/ debugfs directly
Message-ID<q3sNd-2pJ-43@gated-at.bofh.it>
In reply to#1213896
Commit-ID:  9f30fffc78ca35c862f74f34cc597c7fdddc8793
Gitweb:     http://git.kernel.org/tip/9f30fffc78ca35c862f74f34cc597c7fdddc8793
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Wed, 26 Aug 2015 15:46:45 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 28 Aug 2015 14:53:53 -0300

perf tools: Do not change lib/api/fs/debugfs directly

The tracing_events_path is the variable we want to change via
--debugfs-dir option, not the debugfs_mountpoint.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Reviewed-by: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Raphael Beamonte <raphael.beamonte@gmail.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1440596813-12844-4-git-send-email-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/perf.c      | 2 +-
 tools/perf/util/util.c | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index b857fcb..07dbff5 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -231,7 +231,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 			(*argc)--;
 		} else if (!prefixcmp(cmd, CMD_DEBUGFS_DIR)) {
 			perf_debugfs_set_path(cmd + strlen(CMD_DEBUGFS_DIR));
-			fprintf(stderr, "dir: %s\n", debugfs_mountpoint);
+			fprintf(stderr, "dir: %s\n", tracing_path);
 			if (envchanged)
 				*envchanged = 1;
 		} else if (!strcmp(cmd, "--list-cmds")) {
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index d33c341..7acafb3 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -439,7 +439,6 @@ const char *perf_debugfs_mount(const char *mountpoint)
 
 void perf_debugfs_set_path(const char *mntpt)
 {
-	snprintf(debugfs_mountpoint, strlen(debugfs_mountpoint), "%s", mntpt);
 	set_tracing_events_path("tracing/", mntpt);
 }
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web